Re: [Qemu-devel] [RFC][PATCH 11/12] qcow2: Convert qcow2 to use coroutines for async I/O

[Avi Kivity]

On 01/26/2011 06:08 PM, Anthony Liguori wrote:
> On 01/26/2011 09:50 AM, Kevin Wolf wrote:
> > Am 26.01.2011 16:40, schrieb Avi Kivity:
> > > On 01/22/2011 11:29 AM, Stefan Hajnoczi wrote:
> > > > Converting qcow2 to use coroutines is fairly simple since most of 
> > > > qcow2
> > > > is synchronous.  The synchronous I/O functions likes bdrv_pread() now
> > > > transparently work when called from a coroutine, so all the 
> > > > synchronous
> > > > code just works.
> > > >
> > > > The explicitly asynchronous code is adjusted to repeatedly call
> > > > qcow2_aio_read_cb() or qcow2_aio_write_cb() until the request 
> > > > completes.
> > > > At that point the coroutine will return from its entry function and 
> > > > its
> > > > resources are freed.
> > > >
> > > > The bdrv_aio_readv() and bdrv_aio_writev() user callback is now 
> > > > invoked
> > > > from a BH.  This is necessary since the user callback code does not
> > > > expect to be executed from a coroutine.
> > > >
> > > > This conversion is not completely correct because the safety the
> > > > synchronous code does not carry over to the coroutine version.
> > > > Previously, a synchronous code path could assume that it will never be
> > > > interleaved with another request executing.  This is no longer true
> > > > because bdrv_pread() and bdrv_pwrite() cause the coroutine to yield 
> > > > and
> > > > other requests can be processed during that time.
> > > >
> > > > The solution is to carefully introduce checks so that pending requests
> > > > do not step on each other's toes.  That is left for a future patch...
> > > The way I thought of doing this is:
> > >
> > > qcow_aio_write(...)
> > > {
> > >       execute_in_coroutine {
> > >           co_mutex_lock(&bs->mutex);
> > >           do_qcow_aio_write(...); // original qcow code
> > >           co_mutex_release(&bs->mutex);
>
> The release has to be executed in the call back.
>
> I think it's a bit nicer to not do a mutex, but rather to have a 
> notion of freezing/unfreezing the block queue and instead do:
>
> completion() {
>    bdrv_unfreeze(bs);
> }
>
> coroutine {
>    bdrv_freeze(bs);
>    do_qcow_aio_write(completion);
> }
>
> Freeze/unfreeze is useful in a number of other places too (like 
> snapshotting).

Serializing against a global mutex has the advantage that it can be 
treated as a global lock that is decomposed into fine-grained locks.

For example, we can start the code conversion from an explict async 
model to a threaded sync model by converting the mutex into a 
shared/exclusive lock.  Operations like read and write take the lock for 
shared access (and take a fine-grained mutex on the metadata cache 
entry), while operation like creating a snapshot take the lock for 
exclusive access.  That doesn't work with freeze/thaw.

--
error compiling committee.c: too many arguments to function