Daniel P. Berrangé <[email protected]> writes:
> The ability to set the thread name needs to be used in a number
> of places, so expose the current impls as public methods.
>
> Reviewed-by: Richard Henderson <[email protected]>
> Reviewed-by: Dr. David Alan Gilbert <[email protected]>
> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> include/qemu/thread.h | 1 +
> util/qemu-thread-posix.c | 26 ++++++++++++++++----------
> util/qemu-thread-win32.c | 13 ++++++++-----
> 3 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 3a286bb3ef..27b888ab0a 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -215,6 +215,7 @@ void *qemu_thread_join(QemuThread *thread);
> void qemu_thread_get_self(QemuThread *thread);
> bool qemu_thread_is_self(QemuThread *thread);
> G_NORETURN void qemu_thread_exit(void *retval);
> +void qemu_thread_set_name(const char *name);
>
> struct Notifier;
> /**
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 7c985b5d38..b1c127dbe3 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -329,6 +329,21 @@ static void qemu_thread_atexit_notify(void *arg)
> notifier_list_notify(&thread_exit, NULL);
> }
>
> +void qemu_thread_set_name(const char *name)
> +{
> + /*
> + * Attempt to set the threads name; note that this is for debug, so
> + * we're not going to fail if we can't set it.
> + */
> +# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> + pthread_setname_np(pthread_self(), name);
> +# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
> + pthread_setname_np(name);
> +# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
> + pthread_set_name_np(pthread_self(), name);
> +# endif
> +}
> +
> typedef struct {
> void *(*start_routine)(void *);
> void *arg;
> @@ -342,17 +357,8 @@ static void *qemu_thread_start(void *args)
> void *arg = qemu_thread_args->arg;
> void *r;
>
> - /* Attempt to set the threads name; note that this is for debug, so
> - * we're not going to fail if we can't set it.
> - */
> if (qemu_thread_args->name) {
> -# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> - pthread_setname_np(pthread_self(), qemu_thread_args->name);
> -# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
> - pthread_setname_np(qemu_thread_args->name);
> -# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
> - pthread_set_name_np(pthread_self(), qemu_thread_args->name);
> -# endif
> + qemu_thread_set_name(qemu_thread_args->name);
> }
> QEMU_TSAN_ANNOTATE_THREAD_NAME(qemu_thread_args->name);
> g_free(qemu_thread_args->name);
qemu_thread_set_name() is factored out. No change in behavior.
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 9595a5b090..4d2d663a9a 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -225,6 +225,7 @@ struct QemuThreadData {
> void *arg;
> short mode;
> NotifierList exit;
> + char *name;
>
> /* Only used for joinable threads. */
> bool exited;
> @@ -266,6 +267,10 @@ static unsigned __stdcall win32_start_routine(void *arg)
> void *(*start_routine)(void *) = data->start_routine;
> void *thread_arg = data->arg;
>
> + if (data->name) {
> + qemu_thread_set_name(data->name);
> + g_clear_pointer(&data->name, g_free);
> + }
> qemu_thread_data = data;
> qemu_thread_exit(start_routine(thread_arg));
> abort();
> @@ -316,7 +321,7 @@ void *qemu_thread_join(QemuThread *thread)
> return ret;
> }
>
> -static void set_thread_description(HANDLE h, const char *name)
> +void qemu_thread_set_name(const char *name)
> {
> g_autofree wchar_t *namew = NULL;
>
> @@ -329,7 +334,7 @@ static void set_thread_description(HANDLE h, const char
> *name)
> return;
> }
>
> - SetThreadDescriptionFunc(h, namew);
> + SetThreadDescriptionFunc(GetCurrentThread(), namew);
> }
>
> void qemu_thread_create(QemuThread *thread, const char *name,
> @@ -344,6 +349,7 @@ void qemu_thread_create(QemuThread *thread, const char
> *name,
> data->arg = arg;
> data->mode = mode;
> data->exited = false;
> + data->name = g_strdup(name);
> notifier_list_init(&data->exit);
>
> if (data->mode != QEMU_THREAD_DETACHED) {
> @@ -355,9 +361,6 @@ void qemu_thread_create(QemuThread *thread, const char
> *name,
> if (!hThread) {
> error_exit(GetLastError(), __func__);
> }
> - if (name) {
> - set_thread_description(hThread, name);
> - }
> CloseHandle(hThread);
>
> thread->data = data;
This delays setting the thread name until the thread runs. Fine, I
guess, but I'd mention it in the commit message.
The new QemuThreadData member @name is non-null only between thread
creation and thread name setting, unlike the other members under
/* Passed to win32_start_routine. */ Worth a comment? Keep it alive
until the thread dies? Entirely up to you.
Reviewed-by: Markus Armbruster <[email protected]>
