Re: [Qemu-devel] [PATCH 0/2] aio: add aio_context_acquire() and aio_context_release()

[Stefan Hajnoczi]

On Wed, Oct 09, 2013 at 12:16:47PM +0200, Paolo Bonzini wrote:
> Il 09/10/2013 11:55, Stefan Hajnoczi ha scritto:
> > aio_context_acquire() and aio_context_release() make it possible for 
> > multiple
> > threads to safely operate on a shared AioContext.  This is a prerequisite 
> > for
> > using the block layer outside the QEMU global mutex.
> > 
> > Imagine that a dataplane thread is performing I/O on behalf of the guest 
> > when
> > the user issues a monitor command that needs to access the BlockDriverState.
> > The monitor thread needs to acquire the AioContext before calling bdrv_*()
> > functions on it.  This prevents the dataplane thread from interfering with 
> > the
> > monitor thread.
> > 
> > There was a discussion in the RFC email thread about how to implement 
> > fairness:
> > each thread should get a turn to acquire the AioContext so that starvation 
> > is
> > avoided.
> > 
> > Paolo suggested doing what the QEMU main loop does today: drop the lock 
> > before
> > invoking ppoll(2).  It turns out that this is tricky since we want both 
> > threads
> > to be able to call aio_poll() simultaneously.  AioContext->pollfds[] 
> > currently
> > prevents two simultaneous aio_poll() calls since it is a shared resource.  
> > It's
> > also tricky to avoid deadlocks if two threads execute aio_poll() 
> > simulatenously
> > except by waking all threads each time *any* thread makes any progress.
> > 
> > I found the simplest solution is to implement RFifoLock, a recursive lock 
> > with
> > FIFO ordering.  This lock supports the semantics we need for the following
> > pattern:
> > 
> >   /* Event loop thread */
> >   while (running) {
> >       aio_context_acquire(ctx);
> >       aio_poll(ctx, true);
> >       aio_context_release(ctx);
> >   }
> > 
> >   /* Another thread */
> >   aio_context_acquire(ctx);
> >   bdrv_read(bs, 0x1000, buf, 1);
> >   aio_context_release(ctx);
> 
> Should bdrv_read itself call aio_context_acquire?  If not, what's the
> use of recursion?

Recursion is an artifact of our previous discussions.  In a few days I
should have explored enough use cases to know whether we need it or not
- I'm converting code to use acquire/release right now.

Will let you know once I've figured out if we really need it or not.

Stefan