Re: [PATCH 01/10] util/qemu-thread: track bql_locked across the timed condvar wait

[Fabiano Rosas <farosas@suse.de>]

"Denis V. Lunev" <den@openvz.org> writes:

> From: Denis V. Lunev <den@openvz.org>
>
> qemu_cond_timedwait_ts() releases and retakes the mutex around
> pthread_cond_timedwait(), but calls the two trace points by hand
> instead of going through the mutex hooks, so it never reaches
> bql_update_status(). A thread waiting there still reports bql_locked
> true while the mutex is released, which is the confusion the hooks were
> given that job to end. migration/cpu-throttle.c reaches this through
> qemu_cond_timedwait_bql(). The Windows qemu_cond_timedwait_impl() has
> the same two trace calls, next to a qemu_cond_wait_impl() that already
> uses the hooks.
>
> Call the hooks, which emit the same two trace points.
>
> Fixes: c89d1c879a ("bql: Fix bql_locked status with condvar APIs")
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> Cc: Zhao Liu <zhao1.liu@intel.com>
> Cc: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  util/qemu-thread-posix.c | 4 ++--
>  util/qemu-thread-win32.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 089606c93f..580cdf8079 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -250,9 +250,9 @@ qemu_cond_timedwait_ts(QemuCond *cond, QemuMutex *mutex, struct timespec *ts,
>      int err;
>  
>      assert(cond->initialized);
> -    trace_qemu_mutex_unlock(mutex, file, line);
> +    qemu_mutex_pre_unlock(mutex, file, line);
>      err = pthread_cond_timedwait(&cond->cond, &mutex->lock, ts);
> -    trace_qemu_mutex_locked(mutex, file, line);
> +    qemu_mutex_post_lock(mutex, file, line);
>      if (err && err != ETIMEDOUT) {
>          error_exit(err, __func__);
>      }
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 3037732a6f..afdacdb5f2 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -177,11 +177,11 @@ bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms,
>      int rc = 0;
>  
>      assert(cond->initialized);
> -    trace_qemu_mutex_unlock(mutex, file, line);
> +    qemu_mutex_pre_unlock(mutex, file, line);
>      if (!SleepConditionVariableSRW(&cond->var, &mutex->lock, ms, 0)) {
>          rc = GetLastError();
>      }
> -    trace_qemu_mutex_locked(mutex, file, line);
> +    qemu_mutex_post_lock(mutex, file, line);
>      if (rc && rc != ERROR_TIMEOUT) {
>          error_exit(rc, __func__);
>      }

Reviewed-by: Fabiano Rosas <farosas@suse.de>