Re: [Qemu-devel] [RFC] aio: add aio_context_acquire() and aio_context_release()

[Stefan Hajnoczi <stefanha@redhat.com>]

On Thu, Aug 29, 2013 at 10:26:31AM +0200, Paolo Bonzini wrote:
> Il 27/08/2013 16:39, Stefan Hajnoczi ha scritto:
> > +void aio_context_acquire(AioContext *ctx)
> > +{
> > +    qemu_mutex_lock(&ctx->acquire_lock);
> > +    while (ctx->owner) {
> > +        assert(!qemu_thread_is_self(ctx->owner));
> > +        aio_notify(ctx); /* kick current owner */
> > +        qemu_cond_wait(&ctx->acquire_cond, &ctx->acquire_lock);
> > +    }
> > +    qemu_thread_get_self(&ctx->owner_thread);
> > +    ctx->owner = &ctx->owner_thread;
> > +    qemu_mutex_unlock(&ctx->acquire_lock);
> > +}
> > +
> > +void aio_context_release(AioContext *ctx)
> > +{
> > +    qemu_mutex_lock(&ctx->acquire_lock);
> > +    assert(ctx->owner && qemu_thread_is_self(ctx->owner));
> > +    ctx->owner = NULL;
> > +    qemu_cond_signal(&ctx->acquire_cond);
> > +    qemu_mutex_unlock(&ctx->acquire_lock);
> > +}
> 
> Thinking more about it, there is a risk of busy waiting here if one
> thread releases the AioContext and tries to acquire it again (as in the
> common case of one thread doing acquire/poll/release in a loop).  It
> would only work if mutexes guarantee some level of fairness.

You are right.  I wrote a test that showed there is no fairness.  For
some reason I thought the condvar would provide fairness.

> If you implement recursive acquisition, however, you can make aio_poll
> acquire the context up until just before it invokes ppoll, and then
> again after it comes back from the ppoll.  The two acquire/release pair
> will be no-ops if called during "synchronous" I/O such as
> 
>   /* Another thread */
>   aio_context_acquire(ctx);
>   bdrv_read(bs, 0x1000, buf, 1);
>   aio_context_release(ctx);
> 
> Yet they will do the right thing when called from the event loop thread.
> 
> (where the bdrv_read can actually be something more complicated such as
> a live snapshot or, in general, anything involving bdrv_drain_all).

This doesn't guarantee fairness either, right?  If ppoll(2) returns
immediately then the thread might still be scheduled and have enough of
its time slice left to acquire the AioContext again.

With your approach another thread can squeeze in when ppoll(2) is
returning so newer fd activity can be processed before we processed
*before* older activity.  Not sure out-of-order callbacks are a problem
but it can happen since we don't have fairness.

But at least this way other threads can acquire the AioContext while
ppoll(2) is blocked without racing against each other for the acquire
lock.

Stefan