[PATCH v3 05/20] audio: make jackaudio use qemu_thread_set_name

Daniel P. Berrangé posted 20 patches 2 weeks, 3 days ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Weil <sw@weilnetz.de>
There is a newer version of this series
[PATCH v3 05/20] audio: make jackaudio use qemu_thread_set_name
Posted by Daniel P. Berrangé 2 weeks, 3 days ago
This has greater portability than directly call pthread_setname_np,
which is only 1 out of 3 possible functions for pthreads that can
set the name.

The new API requires a trampoline function, since it can only set
the name of the current thread.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/jackaudio.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 974a3caad3..69dce3f302 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -629,18 +629,36 @@ static void qjack_enable_in(HWVoiceIn *hw, bool enable)
     ji->c.enabled = enable;
 }
 
-#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+#if !defined(WIN32)
+struct QJackThreadData {
+    void *(*function)(void *);
+    void *arg;
+};
+
+static void *qjack_thread_trampoline(void *targ)
+{
+    struct QJackThreadData *data = targ;
+    void *(*function)(void *) = data->function;
+    void *arg = data->arg;
+
+    g_free(data);
+    qemu_thread_set_name("jack-client");
+
+    return function(arg);
+}
+
 static int qjack_thread_creator(jack_native_thread_t *thread,
     const pthread_attr_t *attr, void *(*function)(void *), void *arg)
 {
-    int ret = pthread_create(thread, attr, function, arg);
+    struct QJackThreadData *data = g_new0(struct QJackThreadData, 1);
+    data->function = function;
+    data->arg = arg;
+    int ret = pthread_create(thread, attr, qjack_thread_trampoline, data);
     if (ret != 0) {
+        g_free(data);
         return ret;
     }
 
-    /* set the name of the thread */
-    pthread_setname_np(*thread, "jack-client");
-
     return ret;
 }
 #endif
@@ -696,7 +714,7 @@ static void register_audio_jack(void)
 {
     qemu_mutex_init(&qjack_shutdown_lock);
     audio_driver_register(&jack_driver);
-#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+#if !defined(WIN32)
     jack_set_thread_creator(qjack_thread_creator);
 #endif
     jack_set_error_function(qjack_error);
-- 
2.50.1


Re: [PATCH v3 05/20] audio: make jackaudio use qemu_thread_set_name
Posted by Christian Schoenebeck 2 weeks, 3 days ago
On Wednesday, September 10, 2025 8:03:42 PM CEST Daniel P. Berrangé wrote:
> This has greater portability than directly call pthread_setname_np,
> which is only 1 out of 3 possible functions for pthreads that can
> set the name.
> 
> The new API requires a trampoline function, since it can only set
> the name of the current thread.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  audio/jackaudio.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/audio/jackaudio.c b/audio/jackaudio.c
> index 974a3caad3..69dce3f302 100644
> --- a/audio/jackaudio.c
> +++ b/audio/jackaudio.c
> @@ -629,18 +629,36 @@ static void qjack_enable_in(HWVoiceIn *hw, bool
> enable) ji->c.enabled = enable;
>  }
> 
> -#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> +#if !defined(WIN32)
> +struct QJackThreadData {
> +    void *(*function)(void *);
> +    void *arg;
> +};
> +
> +static void *qjack_thread_trampoline(void *targ)
> +{
> +    struct QJackThreadData *data = targ;
> +    void *(*function)(void *) = data->function;
> +    void *arg = data->arg;
> +
> +    g_free(data);
> +    qemu_thread_set_name("jack-client");
> +
> +    return function(arg);
> +}
> +
>  static int qjack_thread_creator(jack_native_thread_t *thread,
>      const pthread_attr_t *attr, void *(*function)(void *), void *arg)
>  {
> -    int ret = pthread_create(thread, attr, function, arg);
> +    struct QJackThreadData *data = g_new0(struct QJackThreadData, 1);
> +    data->function = function;
> +    data->arg = arg;
> +    int ret = pthread_create(thread, attr, qjack_thread_trampoline, data);
>      if (ret != 0) {
> +        g_free(data);
>          return ret;
>      }
> 
> -    /* set the name of the thread */
> -    pthread_setname_np(*thread, "jack-client");
> -
>      return ret;
>  }
>  #endif
> @@ -696,7 +714,7 @@ static void register_audio_jack(void)
>  {
>      qemu_mutex_init(&qjack_shutdown_lock);
>      audio_driver_register(&jack_driver);
> -#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> +#if !defined(WIN32)
>      jack_set_thread_creator(qjack_thread_creator);
>  #endif
>      jack_set_error_function(qjack_error);

Well, it does what you said, so:

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

I just wonder whether this thread naming code couldn't be much more simplified
now by dropping jack_set_thread_creator() entirely, which is very seldomly
used at all and had another user case [1]:

"This function can be used in very very specialized cases where it is
necessary that client threads created by JACK are created by something other
than pthread_create(). After it is used, any threads that JACK needs for the
client will will be created by calling the function passed to this function.

No normal application/client should consider calling this. The specific case
for which it was created involves running win32/x86 plugins under Wine on
Linux, where it is necessary that all threads that might call win32 functions
are known to Wine."

[1] https://jackaudio.org/api/group__ClientThreads.html#ga157ab0ade60e266ffd26ddffdb5545af

However there doesn't seem to be a thread creation callback in the JACK API,
so the jack_set_thread_creator() bypass is still the best we can get, right?

/Christian
Re: [PATCH v3 05/20] audio: make jackaudio use qemu_thread_set_name
Posted by Daniel P. Berrangé 2 weeks, 2 days ago
On Thu, Sep 11, 2025 at 03:21:04PM +0200, Christian Schoenebeck wrote:
> On Wednesday, September 10, 2025 8:03:42 PM CEST Daniel P. Berrangé wrote:
> > This has greater portability than directly call pthread_setname_np,
> > which is only 1 out of 3 possible functions for pthreads that can
> > set the name.
> > 
> > The new API requires a trampoline function, since it can only set
> > the name of the current thread.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  audio/jackaudio.c | 30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> > 
> > diff --git a/audio/jackaudio.c b/audio/jackaudio.c
> > index 974a3caad3..69dce3f302 100644
> > --- a/audio/jackaudio.c
> > +++ b/audio/jackaudio.c
> > @@ -629,18 +629,36 @@ static void qjack_enable_in(HWVoiceIn *hw, bool
> > enable) ji->c.enabled = enable;
> >  }
> > 
> > -#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> > +#if !defined(WIN32)
> > +struct QJackThreadData {
> > +    void *(*function)(void *);
> > +    void *arg;
> > +};
> > +
> > +static void *qjack_thread_trampoline(void *targ)
> > +{
> > +    struct QJackThreadData *data = targ;
> > +    void *(*function)(void *) = data->function;
> > +    void *arg = data->arg;
> > +
> > +    g_free(data);
> > +    qemu_thread_set_name("jack-client");
> > +
> > +    return function(arg);
> > +}
> > +
> >  static int qjack_thread_creator(jack_native_thread_t *thread,
> >      const pthread_attr_t *attr, void *(*function)(void *), void *arg)
> >  {
> > -    int ret = pthread_create(thread, attr, function, arg);
> > +    struct QJackThreadData *data = g_new0(struct QJackThreadData, 1);
> > +    data->function = function;
> > +    data->arg = arg;
> > +    int ret = pthread_create(thread, attr, qjack_thread_trampoline, data);
> >      if (ret != 0) {
> > +        g_free(data);
> >          return ret;
> >      }
> > 
> > -    /* set the name of the thread */
> > -    pthread_setname_np(*thread, "jack-client");
> > -
> >      return ret;
> >  }
> >  #endif
> > @@ -696,7 +714,7 @@ static void register_audio_jack(void)
> >  {
> >      qemu_mutex_init(&qjack_shutdown_lock);
> >      audio_driver_register(&jack_driver);
> > -#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> > +#if !defined(WIN32)
> >      jack_set_thread_creator(qjack_thread_creator);
> >  #endif
> >      jack_set_error_function(qjack_error);
> 
> Well, it does what you said, so:
> 
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> 
> I just wonder whether this thread naming code couldn't be much more simplified
> now by dropping jack_set_thread_creator() entirely, which is very seldomly
> used at all and had another user case [1]:
> 
> "This function can be used in very very specialized cases where it is
> necessary that client threads created by JACK are created by something other
> than pthread_create(). After it is used, any threads that JACK needs for the
> client will will be created by calling the function passed to this function.
> 
> No normal application/client should consider calling this. The specific case
> for which it was created involves running win32/x86 plugins under Wine on
> Linux, where it is necessary that all threads that might call win32 functions
> are known to Wine."
> 
> [1] https://jackaudio.org/api/group__ClientThreads.html#ga157ab0ade60e266ffd26ddffdb5545af
> 
> However there doesn't seem to be a thread creation callback in the JACK API,
> so the jack_set_thread_creator() bypass is still the best we can get, right?

From QEMU's POV the only value we get from using the thread callback
is that we can set the thread name so the jack thrread can be easily
identified when debugging.  I don't think there's another easy way
to get that set, as we can't portability set thread names from outside
the thread.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v3 05/20] audio: make jackaudio use qemu_thread_set_name
Posted by Markus Armbruster 1 week, 2 days ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Sep 11, 2025 at 03:21:04PM +0200, Christian Schoenebeck wrote:
>> On Wednesday, September 10, 2025 8:03:42 PM CEST Daniel P. Berrangé wrote:
>> > This has greater portability than directly call pthread_setname_np,
>> > which is only 1 out of 3 possible functions for pthreads that can
>> > set the name.
>> > 
>> > The new API requires a trampoline function, since it can only set
>> > the name of the current thread.
>> > 
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[...]

>> Well, it does what you said, so:
>> 
>> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>> 
>> I just wonder whether this thread naming code couldn't be much more simplified
>> now by dropping jack_set_thread_creator() entirely, which is very seldomly
>> used at all and had another user case [1]:
>> 
>> "This function can be used in very very specialized cases where it is
>> necessary that client threads created by JACK are created by something other
>> than pthread_create(). After it is used, any threads that JACK needs for the
>> client will will be created by calling the function passed to this function.
>> 
>> No normal application/client should consider calling this. The specific case
>> for which it was created involves running win32/x86 plugins under Wine on
>> Linux, where it is necessary that all threads that might call win32 functions
>> are known to Wine."
>> 
>> [1] https://jackaudio.org/api/group__ClientThreads.html#ga157ab0ade60e266ffd26ddffdb5545af
>> 
>> However there doesn't seem to be a thread creation callback in the JACK API,
>> so the jack_set_thread_creator() bypass is still the best we can get, right?
>
> From QEMU's POV the only value we get from using the thread callback
> is that we can set the thread name so the jack thrread can be easily
> identified when debugging.  I don't think there's another easy way
> to get that set, as we can't portability set thread names from outside
> the thread.

As far as I can tell, this is due to Mac OS X.  Personally, I wouldn't
bother complicating things for that.  But whoever wrote the code in
qemu-thread-posix.c did, so ...