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.