Re: AIO_WAIT_WHILE questions
On Mon, Mar 30, 2020 at 10:09:45AM +0200, Markus Armbruster wrote:
> Cc'ing people based on output of "scripts/get_maintainer.pl -f
> include/block/aio-wait.h".
>
> Dietmar Maurer writes:
>
> > Hi all,
> >
> > I have a question about AIO_WAIT_WHILE. The docs inside the code say:
> >
> > * The caller's thread must be the IOThread that owns @ctx or the main loop
> > * thread (with @ctx acquired exactly once).
> >
> > I wonder if that "with @ctx acquired exactly once" is always required?
> >
> > I have done a quick test (see code below) and this reveals that the
> > condition is not
> > always met.
> >
> > Or is my test wrong (or the docs)?
> >
> > ---debug helper---
> > diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
> > index afeeb18f95..cf78dca9f9 100644
> > --- a/include/block/aio-wait.h
> > +++ b/include/block/aio-wait.h
> > @@ -82,6 +82,8 @@ extern AioWait global_aio_wait;
> > atomic_inc(_->num_waiters); \
> > if (ctx_ && in_aio_context_home_thread(ctx_)) {\
> > while ((cond)) { \
> > +printf("AIO_WAIT_WHILE %p %d\n", ctx, ctx_->lock_count); \
> > +assert(ctx_->lock_count == 1); \
> > aio_poll(ctx_, true); \
> > waited_ = true;\
> > } \
In this case it doesn't matter. Handlers invoked by aio_poll() that
acquire ctx's recursive mutex will succeed.
The "exactly once" requirement is there because nested locking is not
supported when waiting for an AioContext that runs in a different
thread:
} else { \
assert(qemu_get_current_aio_context() == \
qemu_get_aio_context());\
while ((cond)) { \
if (ctx_) {\
aio_context_release(ctx_); \
^--- doesn't work if we have acquired it multiple times
I think it would be okay to update the documentation to make this clear.
signature.asc
Description: PGP signature
Re: AIO_WAIT_WHILE questions
Cc'ing people based on output of "scripts/get_maintainer.pl -f
include/block/aio-wait.h".
Dietmar Maurer writes:
> Hi all,
>
> I have a question about AIO_WAIT_WHILE. The docs inside the code say:
>
> * The caller's thread must be the IOThread that owns @ctx or the main loop
> * thread (with @ctx acquired exactly once).
>
> I wonder if that "with @ctx acquired exactly once" is always required?
>
> I have done a quick test (see code below) and this reveals that the condition
> is not
> always met.
>
> Or is my test wrong (or the docs)?
>
> ---debug helper---
> diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
> index afeeb18f95..cf78dca9f9 100644
> --- a/include/block/aio-wait.h
> +++ b/include/block/aio-wait.h
> @@ -82,6 +82,8 @@ extern AioWait global_aio_wait;
> atomic_inc(_->num_waiters); \
> if (ctx_ && in_aio_context_home_thread(ctx_)) {\
> while ((cond)) { \
> +printf("AIO_WAIT_WHILE %p %d\n", ctx, ctx_->lock_count); \
> +assert(ctx_->lock_count == 1); \
> aio_poll(ctx_, true); \
> waited_ = true;\
> } \
> diff --git a/include/block/aio.h b/include/block/aio.h
> index cb1989105a..51ef20e2f0 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -125,6 +125,7 @@ struct AioContext {
>
> /* Used by AioContext users to protect from multi-threaded access. */
> QemuRecMutex lock;
> +int lock_count;
>
> /* The list of registered AIO handlers. Protected by ctx->list_lock. */
> AioHandlerList aio_handlers;
> diff --git a/util/async.c b/util/async.c
> index b94518b948..9804c6c64f 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -594,9 +594,11 @@ void aio_context_unref(AioContext *ctx)
> void aio_context_acquire(AioContext *ctx)
> {
> qemu_rec_mutex_lock(>lock);
> +ctx->lock_count++;
> }
>
> void aio_context_release(AioContext *ctx)
> {
> +ctx->lock_count--;
> qemu_rec_mutex_unlock(>lock);
> }
