Re: [Qemu-devel] fix bdrv_read/write_em and qemu_aio_flush

[Andrea Arcangeli]

Hi Christoph,

On Sat, May 30, 2009 at 12:08:42PM +0200, Christoph Hellwig wrote:
> On Thu, May 28, 2009 at 06:33:10PM +0200, Andrea Arcangeli wrote:
> > Hello,
> > 
> > the debug code in my ide_dma_cancel patch (not yet included upstream)
> > made us notice that when qemu_aio_flush returns, there are still
> > pending aio commands that waits to complete. Auditing the code I found
> > strange stuff like the fact qemu_aio_waits does nothing if there's an
> > unrelated (no aio related) bh executed. And I think I found the reason
> > of why there was still pending aio when qemu_aio_flush because
> > qemu_aio_wait does a lot more than wait, it can start aio, and if the
> > previous ->io_flush returned zero, the loop ends and ->io_flush isn't
> > repeated. The fact an unrelated bh can make qemu_aio_wait a noop seems
> > troublesome for all callers that aren't calling qemu_aio_wait in a
> > loop like qemu_aio_flush, so I preferred to change those callers to a
> > safer qemu_aio_flush in case the bh executed generates more pending
> > I/O. What you think about this patch against qemu git?
> 
> Looks good to me.  In my unsubmitted aio support patches for qemu-io
> I had to call qemu_aio_wait at least twice to get aio requests reliably
> completed, but with this patch and calling qemu_aio_flush it always
> completes all requests.

Exactly! In effect this could be slightly optimized in the future, if
we could track a single aio request, instead of waiting for them all.
This is a bit the equivalent of 'sync' instead of wait_on_page in the
kernel, because we don't have a wait_on_page here, so we've to flush
them all to be safe from bh execution. Given the potential of not
waiting and I/O corruption or misbehavior of ide.c because of
qemu_aio_flush returning too early without my patch, I think it's good
idea to apply.