util/qemu-thread-win32.c | 54 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)
From: Marc-André Lureau <marcandre.lureau@redhat.com>
As described in:
https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-set-a-thread-name-in-native-code?view=vs-2022
SetThreadDescription() is available since Windows 10, version 1607 and
in some versions only by "Run Time Dynamic Linking". Its declaration is
not yet in mingw, so we lookup the function the same way glib does.
Tested with Visual Studio Community 2022 debugger.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
util/qemu-thread-win32.c | 54 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index a2d5a6e825..9861ec5b89 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -19,12 +19,40 @@
static bool name_threads;
+typedef HRESULT (WINAPI *pSetThreadDescription) (HANDLE hThread,
+ PCWSTR lpThreadDescription);
+static pSetThreadDescription SetThreadDescriptionFunc = NULL;
+static HMODULE kernel32_module = NULL;
+
+static bool
+load_set_thread_description(void)
+{
+ static gsize _init_once = 0;
+
+ if (g_once_init_enter(&_init_once)) {
+ kernel32_module = LoadLibraryW(L"kernel32.dll");
+ if (kernel32_module) {
+ SetThreadDescriptionFunc =
+ (pSetThreadDescription)GetProcAddress(kernel32_module,
+ "SetThreadDescription");
+ if (!SetThreadDescriptionFunc) {
+ FreeLibrary(kernel32_module);
+ }
+ }
+ g_once_init_leave(&_init_once, 1);
+ }
+
+ return !!SetThreadDescriptionFunc;
+}
+
void qemu_thread_naming(bool enable)
{
/* But note we don't actually name them on Windows yet */
name_threads = enable;
- fprintf(stderr, "qemu: thread naming not supported on this host\n");
+ if (enable && !load_set_thread_description()) {
+ fprintf(stderr, "qemu: thread naming not supported on this host\n");
+ }
}
static void error_exit(int err, const char *msg)
@@ -400,6 +428,26 @@ void *qemu_thread_join(QemuThread *thread)
return ret;
}
+static bool
+set_thread_description(HANDLE h, const char *name)
+{
+ HRESULT hr;
+ g_autofree wchar_t *namew = NULL;
+
+ if (!load_set_thread_description() || !name) {
+ return false;
+ }
+
+ namew = g_utf8_to_utf16(name, -1, NULL, NULL, NULL);
+ if (!namew) {
+ return false;
+ }
+
+ hr = SetThreadDescriptionFunc(h, namew);
+
+ return SUCCEEDED(hr);
+}
+
void qemu_thread_create(QemuThread *thread, const char *name,
void *(*start_routine)(void *),
void *arg, int mode)
@@ -423,7 +471,11 @@ void qemu_thread_create(QemuThread *thread, const char *name,
if (!hThread) {
error_exit(GetLastError(), __func__);
}
+ if (name_threads) {
+ set_thread_description(hThread, name);
+ }
CloseHandle(hThread);
+
thread->data = data;
}
--
2.37.3
On 9/29/22 06:41, marcandre.lureau@redhat.com wrote:
> void qemu_thread_naming(bool enable)
> {
> /* But note we don't actually name them on Windows yet */
> name_threads = enable;
>
> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + if (enable && !load_set_thread_description()) {
> + fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + }
> }
Comment is out of date, and I think it would be better to *not* set name_threads if not
supported, rather than...
> +static bool
> +set_thread_description(HANDLE h, const char *name)
> +{
> + HRESULT hr;
> + g_autofree wchar_t *namew = NULL;
> +
> + if (!load_set_thread_description() || !name) {
> + return false;
> + }
... have to re-query load_set_thread_description later.
Also, unused return value; might as well be void.
r~
Hi
On Thu, Sep 29, 2022 at 9:53 PM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 9/29/22 06:41, marcandre.lureau@redhat.com wrote:
> > void qemu_thread_naming(bool enable)
> > {
> > /* But note we don't actually name them on Windows yet */
> > name_threads = enable;
> >
> > - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> > + if (enable && !load_set_thread_description()) {
> > + fprintf(stderr, "qemu: thread naming not supported on this
> host\n");
> > + }
> > }
>
> Comment is out of date, and I think it would be better to *not* set
> name_threads if not
> supported, rather than...
>
Comment removed.
>
>
> > +static bool
> > +set_thread_description(HANDLE h, const char *name)
> > +{
> > + HRESULT hr;
> > + g_autofree wchar_t *namew = NULL;
> > +
> > + if (!load_set_thread_description() || !name) {
> > + return false;
> > + }
>
> ... have to re-query load_set_thread_description later.
>
The load_set_thread_description() function is actually a "one-time"
function, it doesn't re-load.
>
> Also, unused return value; might as well be void.
>
Right, maybe it should warn if it failed to set the name?
--
Marc-André Lureau
On 9/30/22 01:08, Marc-André Lureau wrote:
> Hi
>
> On Thu, Sep 29, 2022 at 9:53 PM Richard Henderson <richard.henderson@linaro.org
> <mailto:richard.henderson@linaro.org>> wrote:
>
> On 9/29/22 06:41, marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com> wrote:
> > void qemu_thread_naming(bool enable)
> > {
> > /* But note we don't actually name them on Windows yet */
> > name_threads = enable;
> >
> > - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> > + if (enable && !load_set_thread_description()) {
> > + fprintf(stderr, "qemu: thread naming not supported on this host\n");
> > + }
> > }
>
> Comment is out of date, and I think it would be better to *not* set name_threads if not
> supported, rather than...
>
>
> Comment removed.
>
>
>
> > +static bool
> > +set_thread_description(HANDLE h, const char *name)
> > +{
> > + HRESULT hr;
> > + g_autofree wchar_t *namew = NULL;
> > +
> > + if (!load_set_thread_description() || !name) {
> > + return false;
> > + }
>
> ... have to re-query load_set_thread_description later.
>
>
> The load_set_thread_description() function is actually a "one-time" function, it doesn't
> re-load.
You're calling it again. That has some cost in the mutex/spinlock that's behind that
one-time operation, when you're already making the call to set_thread_description conditional.
> Right, maybe it should warn if it failed to set the name?
After you've already printed an error in qemu_thread_naming()? Doesn't seem useful. Or
did you mean in the case we think it should work, but didn't?
r~
Hi
On Fri, Sep 30, 2022 at 5:35 PM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 9/30/22 01:08, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Sep 29, 2022 at 9:53 PM Richard Henderson <
> richard.henderson@linaro.org
> > <mailto:richard.henderson@linaro.org>> wrote:
> >
> > On 9/29/22 06:41, marcandre.lureau@redhat.com <mailto:
> marcandre.lureau@redhat.com> wrote:
> > > void qemu_thread_naming(bool enable)
> > > {
> > > /* But note we don't actually name them on Windows yet */
> > > name_threads = enable;
> > >
> > > - fprintf(stderr, "qemu: thread naming not supported on this
> host\n");
> > > + if (enable && !load_set_thread_description()) {
> > > + fprintf(stderr, "qemu: thread naming not supported on
> this host\n");
> > > + }
> > > }
> >
> > Comment is out of date, and I think it would be better to *not* set
> name_threads if not
> > supported, rather than...
> >
> >
> > Comment removed.
> >
> >
> >
> > > +static bool
> > > +set_thread_description(HANDLE h, const char *name)
> > > +{
> > > + HRESULT hr;
> > > + g_autofree wchar_t *namew = NULL;
> > > +
> > > + if (!load_set_thread_description() || !name) {
> > > + return false;
> > > + }
> >
> > ... have to re-query load_set_thread_description later.
> >
> >
> > The load_set_thread_description() function is actually a "one-time"
> function, it doesn't
> > re-load.
>
> You're calling it again. That has some cost in the mutex/spinlock that's
> behind that
> one-time operation, when you're already making the call to
> set_thread_description conditional.
>
That seems pretty marginal given the frequencies of this call, when we are
creating threads.
So you suggest simply setting "name_threads" to false when loading the
function failed?
>
> > Right, maybe it should warn if it failed to set the name?
>
> After you've already printed an error in qemu_thread_naming()? Doesn't
> seem useful. Or
> did you mean in the case we think it should work, but didn't?
>
During qemu_thread_create(), if set_thread_description() failed.
--
Marc-André Lureau
© 2016 - 2024 Red Hat, Inc.