On Wed, Jan 26, 2011 at 05:40:27PM +0200, Avi Kivity wrote:
> 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);
> }
> }
I think this approach makes sense as the next step towards a
fine-grained strategy.
Remember that qcow2 is not just:
qcow2_aio_write(...)
{
sync_io(...);
aio_write(...);
complete_request(...); /* in callback */
}
It is actually:
qcow2_aio_write(...)
{
sync_io(...);
aio_write(...);
more_sync_io(...); /* in callback */
complete_request(...);
}
We need to make sure the synchronous I/O after aio write completion is
also guarded.
Stefan
