Re: AIO_WAIT_WHILE questions

[Stefan Hajnoczi]

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 <address@hidden> 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(&wait_->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