Re: [Qemu-devel] [PATCH v4 3/4] qemu-thread: add QemuEvent

[Jan Kiszka]

On 2013-09-22 10:11, Liu Ping Fan wrote:
> This emulates Win32 manual-reset events using futexes or conditional
> variables.  Typical ways to use them are with multi-producer,
> single-consumer data structures, to test for a complex condition whose
> elements come from different threads:
> 
>     for (;;) {
>         qemu_event_reset(ev);
>         ... test complex condition ...
>         if (condition is true) {
>             break;
>         }
>         qemu_event_wait(ev);
>     }
> 
> Or more efficiently (but with some duplication):
> 
>     ... evaluate condition ...
>     while (!condition) {
>         qemu_event_reset(ev);
>         ... evaluate condition ...
>         if (!condition) {
>             qemu_event_wait(ev);
>             ... evaluate condition ...
>         }
>     }
> 
> QemuEvent provides a very fast userspace path in the common case when
> no other thread is waiting, or the event is not changing state.  It
> is used to report RCU quiescent states to the thread calling
> synchronize_rcu (the latter being the single consumer), and to report
> call_rcu invocations to the thread that receives them.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Again, from and signed-off mismatch.

BTW, above is a good template for a commit log of patch 1.

Jan

> ---
>  include/qemu/thread-posix.h |   8 +++
>  include/qemu/thread-win32.h |   4 ++
>  include/qemu/thread.h       |   7 +++
>  util/qemu-thread-posix.c    | 116 
> ++++++++++++++++++++++++++++++++++++++++++++
>  util/qemu-thread-win32.c    |  26 ++++++++++
>  5 files changed, 161 insertions(+)
> 
> diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
> index 361566a..eb5c7a1 100644
> --- a/include/qemu/thread-posix.h
> +++ b/include/qemu/thread-posix.h
> @@ -21,6 +21,14 @@ struct QemuSemaphore {
>  #endif
>  };
>  
> +struct QemuEvent {
> +#ifndef __linux__
> +    pthread_mutex_t lock;
> +    pthread_cond_t cond;
> +#endif
> +    unsigned value;
> +};
> +
>  struct QemuThread {
>      pthread_t thread;
>  };
> diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
> index 13adb95..3d58081 100644
> --- a/include/qemu/thread-win32.h
> +++ b/include/qemu/thread-win32.h
> @@ -17,6 +17,10 @@ struct QemuSemaphore {
>      HANDLE sema;
>  };
>  
> +struct QemuEvent {
> +    HANDLE event;
> +};
> +
>  typedef struct QemuThreadData QemuThreadData;
>  struct QemuThread {
>      QemuThreadData *data;
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index c02404b..3e32c65 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -7,6 +7,7 @@
>  typedef struct QemuMutex QemuMutex;
>  typedef struct QemuCond QemuCond;
>  typedef struct QemuSemaphore QemuSemaphore;
> +typedef struct QemuEvent QemuEvent;
>  typedef struct QemuThread QemuThread;
>  
>  #ifdef _WIN32
> @@ -45,6 +46,12 @@ void qemu_sem_wait(QemuSemaphore *sem);
>  int qemu_sem_timedwait(QemuSemaphore *sem, int ms);
>  void qemu_sem_destroy(QemuSemaphore *sem);
>  
> +void qemu_event_init(QemuEvent *ev, bool init);
> +void qemu_event_set(QemuEvent *ev);
> +void qemu_event_reset(QemuEvent *ev);
> +void qemu_event_wait(QemuEvent *ev);
> +void qemu_event_destroy(QemuEvent *ev);
> +
>  void qemu_thread_create(QemuThread *thread,
>                          void *(*start_routine)(void *),
>                          void *arg, int mode);
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 4de133e..37dd298 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -20,7 +20,12 @@
>  #include <limits.h>
>  #include <unistd.h>
>  #include <sys/time.h>
> +#ifdef __linux__
> +#include <sys/syscall.h>
> +#include <linux/futex.h>
> +#endif
>  #include "qemu/thread.h"
> +#include "qemu/atomic.h"
>  
>  static void error_exit(int err, const char *msg)
>  {
> @@ -272,6 +277,117 @@ void qemu_sem_wait(QemuSemaphore *sem)
>  #endif
>  }
>  
> +#ifdef __linux__
> +#define futex(...)              syscall(__NR_futex, __VA_ARGS__)
> +
> +static inline void futex_wake(QemuEvent *ev, int n)
> +{
> +    futex(ev, FUTEX_WAKE, n, NULL, NULL, 0);
> +}
> +
> +static inline void futex_wait(QemuEvent *ev, unsigned val)
> +{
> +    futex(ev, FUTEX_WAIT, (int) val, NULL, NULL, 0);
> +}
> +#else
> +static inline void futex_wake(QemuEvent *ev, int n)
> +{
> +    if (n == 1) {
> +        pthread_cond_signal(&ev->cond);
> +    } else {
> +        pthread_cond_broadcast(&ev->cond);
> +    }
> +}
> +
> +static inline void futex_wait(QemuEvent *ev, unsigned val)
> +{
> +    pthread_mutex_lock(&ev->lock);
> +    if (ev->value == val) {
> +        pthread_cond_wait(&ev->cond, &ev->lock);
> +    }
> +    pthread_mutex_unlock(&ev->lock);
> +}
> +#endif
> +
> +/* Valid transitions:
> + * - free->set, when setting the event
> + * - busy->set, when setting the event, followed by futex_wake
> + * - set->free, when resetting the event
> + * - free->busy, when waiting
> + *
> + * set->busy does not happen (it can be observed from the outside but
> + * it really is set->free->busy).
> + *
> + * busy->free provably cannot happen; to enforce it, the set->free transition
> + * is done with an OR, which becomes a no-op if the event has concurrently
> + * transitioned to free or busy.
> + */
> +
> +#define EV_SET         0
> +#define EV_FREE        1
> +#define EV_BUSY       -1
> +
> +void qemu_event_init(QemuEvent *ev, bool init)
> +{
> +#ifndef __linux__
> +    pthread_mutex_init(&ev->lock, NULL);
> +    pthread_cond_init(&ev->cond, NULL);
> +#endif
> +
> +    ev->value = (init ? EV_SET : EV_FREE);
> +}
> +
> +void qemu_event_destroy(QemuEvent *ev)
> +{
> +#ifndef __linux__
> +    pthread_mutex_destroy(&ev->lock);
> +    pthread_cond_destroy(&ev->cond);
> +#endif
> +}
> +
> +void qemu_event_set(QemuEvent *ev)
> +{
> +    if (atomic_mb_read(&ev->value) != EV_SET) {
> +        if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
> +            /* There were waiters, wake them up.  */
> +            futex_wake(ev, INT_MAX);
> +        }
> +    }
> +}
> +
> +void qemu_event_reset(QemuEvent *ev)
> +{
> +    if (atomic_mb_read(&ev->value) == EV_SET) {
> +        /*
> +         * If there was a concurrent reset (or even reset+wait),
> +         * do nothing.  Otherwise change EV_SET->EV_FREE.
> +         */
> +        atomic_or(&ev->value, EV_FREE);
> +    }
> +}
> +
> +void qemu_event_wait(QemuEvent *ev)
> +{
> +    unsigned value;
> +
> +    value = atomic_mb_read(&ev->value);
> +    if (value != EV_SET) {
> +        if (value == EV_FREE) {
> +            /*
> +             * Leave the event reset and tell qemu_event_set that there
> +             * are waiters.  No need to retry, because there cannot be
> +             * a concurent busy->free transition.  After the CAS, the
> +             * event will be either set or busy.
> +             */
> +            if (atomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
> +                return;
> +            }
> +        }
> +        futex_wait(ev, EV_BUSY);
> +    }
> +}
> +
> +
>  void qemu_thread_create(QemuThread *thread,
>                         void *(*start_routine)(void*),
>                         void *arg, int mode)
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 517878d..27a5217 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -227,6 +227,32 @@ void qemu_sem_wait(QemuSemaphore *sem)
>      }
>  }
>  
> +void qemu_event_init(QemuEvent *ev, bool init)
> +{
> +    /* Manual reset.  */
> +    ev->event = CreateEvent(NULL, TRUE, init, NULL);
> +}
> +
> +void qemu_event_destroy(QemuEvent *ev)
> +{
> +    CloseHandle(ev->event);
> +}
> +
> +void qemu_event_set(QemuEvent *ev)
> +{
> +    SetEvent(ev->event);
> +}
> +
> +void qemu_event_reset(QemuEvent *ev)
> +{
> +    ResetEvent(ev->event);
> +}
> +
> +void qemu_event_wait(QemuEvent *ev)
> +{
> +    WaitForSingleObject(ev->event, INFINITE);
> +}
> +
>  struct QemuThreadData {
>      /* Passed to win32_start_routine.  */
>      void             *(*start_routine)(void *);
> 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux