Patchwork [V3,1/3] qemu: Add qemu-wrappers for pthread_attr_t

login
register
mail settings
Submitter Gautham R Shenoy
Date June 3, 2010, 8:56 a.m.
Message ID <20100603085618.25546.99966.stgit@localhost.localdomain>
Download mbox | patch
Permalink /patch/54463/
State New
Headers show

Comments

Gautham R Shenoy - June 3, 2010, 8:56 a.m.
Add qemu wrappers for pthread_attr_t handling.


Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---
 qemu-thread.c |   34 ++++++++++++++++++++++++++++++++++
 qemu-thread.h |   11 +++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)
Paolo Bonzini - June 3, 2010, 12:31 p.m.
On 06/03/2010 10:56 AM, Gautham R Shenoy wrote:
> Add qemu wrappers for pthread_attr_t handling.

The point of these wrappers AFAIU is not only to add error_exit, but 
also to be portable to Windows in the future.  Is it necessary to create 
the threads as detached?  If you set queue->min_threads to zero all 
threads should exit as soon as they finish their work (which is better 
than exiting immediately).

Paolo
Anthony Liguori - June 4, 2010, 1:07 p.m.
On 06/03/2010 07:31 AM, Paolo Bonzini wrote:
> On 06/03/2010 10:56 AM, Gautham R Shenoy wrote:
>> Add qemu wrappers for pthread_attr_t handling.
>
> The point of these wrappers AFAIU is not only to add error_exit, but 
> also to be portable to Windows in the future.  Is it necessary to 
> create the threads as detached?  If you set queue->min_threads to zero 
> all threads should exit as soon as they finish their work (which is 
> better than exiting immediately).

This is historical because the code was largely inspired by glibc's 
implementation of posix-aio.  It doesn't need to be detached and since 
Corentin wants to be able to join a worker, it makes sense to just avoid 
detaching and pay the overhead of making the threads joinable.

Regards,

Anthony Liguori

> Paolo
>
Corentin Chary - June 4, 2010, 1:19 p.m.
On Fri, Jun 4, 2010 at 3:07 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/03/2010 07:31 AM, Paolo Bonzini wrote:
>>
>> On 06/03/2010 10:56 AM, Gautham R Shenoy wrote:
>>>
>>> Add qemu wrappers for pthread_attr_t handling.
>>
>> The point of these wrappers AFAIU is not only to add error_exit, but also
>> to be portable to Windows in the future.  Is it necessary to create the
>> threads as detached?  If you set queue->min_threads to zero all threads
>> should exit as soon as they finish their work (which is better than exiting
>> immediately).
>
> This is historical because the code was largely inspired by glibc's
> implementation of posix-aio.  It doesn't need to be detached and since
> Corentin wants to be able to join a worker, it makes sense to just avoid
> detaching and pay the overhead of making the threads joinable.
>
> Regards,
>
> Anthony Liguori

Actually, I want to know if the queue is empty and if no job are
currently being processed: all worker are idle or stopped. I don't
really need pthread_join() for that, since worker can be idle (we
don't want to always start and stop the thread :) ).
Paolo Bonzini - June 4, 2010, 1:27 p.m.
On 06/04/2010 03:19 PM, Corentin Chary wrote:
>>> The point of these wrappers AFAIU is not only to add error_exit, but also
>>> to be portable to Windows in the future.
>>
>> This is historical because the code was largely inspired by glibc's
>> implementation of posix-aio.  It doesn't need to be detached and since
>> Corentin wants to be able to join a worker, it makes sense to just avoid
>> detaching and pay the overhead of making the threads joinable.
>
> Actually, I want to know if the queue is empty and if no job are
> currently being processed: all worker are idle or stopped. I don't
> really need pthread_join() for that, since worker can be idle (we
> don't want to always start and stop the thread :) ).

Then it's also fine to have all qemu_threads detached (like in my patch 
to create all qemu_threads with blocked signals).  I just want to avoid 
implementing pthreads one day for qemu-threads-win32.c.

Paolo
Gautham R Shenoy - June 10, 2010, 11:12 a.m.
On Fri, Jun 04, 2010 at 03:19:34PM +0200, Corentin Chary wrote:
> On Fri, Jun 4, 2010 at 3:07 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > On 06/03/2010 07:31 AM, Paolo Bonzini wrote:
> >>
> >> On 06/03/2010 10:56 AM, Gautham R Shenoy wrote:
> >>>
> >>> Add qemu wrappers for pthread_attr_t handling.
> >>
> >> The point of these wrappers AFAIU is not only to add error_exit, but also
> >> to be portable to Windows in the future.  Is it necessary to create the
> >> threads as detached?  If you set queue->min_threads to zero all threads
> >> should exit as soon as they finish their work (which is better than exiting
> >> immediately).
> >
> > This is historical because the code was largely inspired by glibc's
> > implementation of posix-aio.  It doesn't need to be detached and since
> > Corentin wants to be able to join a worker, it makes sense to just avoid
> > detaching and pay the overhead of making the threads joinable.
> >
> > Regards,
> >
> > Anthony Liguori
>

Sorry for the late response. I had been away for the last few days.

> Actually, I want to know if the queue is empty and if no job are
> currently being processed: all worker are idle or stopped.

In other words, you just want to know if the already submitted jobs
have gotten over or not so that you could probably destroy the queue.

The subsystem using the queueing infrastructure is responsible for providing
the guarantee that no new job would be submitted while/after making a call
query the idleness of the queue.

If this understanding is correct, we might want to have a
qemu_async_barrier() implementation which waits for all the threads
finish processing the queued jobs, and destroy themselves.

>I don't really need pthread_join() for that, since worker can be idle (we
> don't want to always start and stop the thread :) ).

Patch

diff --git a/qemu-thread.c b/qemu-thread.c
index 3923db7..524860c 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -142,6 +142,40 @@  void qemu_thread_create(QemuThread *thread,
         error_exit(err, __func__);
 }
 
+void qemu_thread_create_attr(QemuThread *thread, QemuThreadAttr *attr1,
+                       void *(*start_routine)(void*),
+                       void *arg)
+{
+    int err;
+
+    err = pthread_create(&thread->thread, &(attr1->attr), start_routine,
+                            arg);
+    if (err)
+        error_exit(err, __func__);
+}
+
+void qemu_thread_attr_init(QemuThreadAttr *attr1)
+{
+    int err;
+
+    err = pthread_attr_init(&(attr1->attr));
+
+    if (err) {
+        error_exit(err, __func__);
+    }
+}
+
+void qemu_thread_attr_setdetachstate(QemuThreadAttr *attr1, int detachstate)
+{
+    int err;
+
+    err = pthread_attr_setdetachstate(&(attr1->attr), detachstate);
+
+    if (err) {
+        error_exit(err, __func__);
+    }
+}
+
 void qemu_thread_signal(QemuThread *thread, int sig)
 {
     int err;
diff --git a/qemu-thread.h b/qemu-thread.h
index 5ef4a3a..361ef19 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -15,9 +15,14 @@  struct QemuThread {
     pthread_t thread;
 };
 
+struct QemuThreadAttr {
+    pthread_attr_t attr;
+};
+
 typedef struct QemuMutex QemuMutex;
 typedef struct QemuCond QemuCond;
 typedef struct QemuThread QemuThread;
+typedef struct QemuThreadAttr QemuThreadAttr;
 
 void qemu_mutex_init(QemuMutex *mutex);
 void qemu_mutex_lock(QemuMutex *mutex);
@@ -34,7 +39,13 @@  int qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, uint64_t msecs);
 void qemu_thread_create(QemuThread *thread,
                        void *(*start_routine)(void*),
                        void *arg);
+void qemu_thread_create_attr(QemuThread *thread, QemuThreadAttr *attr1,
+                       void *(*start_routine)(void*),
+                       void *arg);
 void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
 int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+
+void qemu_thread_attr_init(QemuThreadAttr *attr1);
+void qemu_thread_attr_setdetachstate(QemuThreadAttr *attr1, int detachstate);
 #endif