From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MANV3-0001vL-O2 for qemu-devel@nongnu.org; Sat, 30 May 2009 08:17:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MANUy-0001uZ-PQ for qemu-devel@nongnu.org; Sat, 30 May 2009 08:17:21 -0400 Received: from [199.232.76.173] (port=49025 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MANUy-0001uW-MW for qemu-devel@nongnu.org; Sat, 30 May 2009 08:17:16 -0400 Received: from mx2.redhat.com ([66.187.237.31]:56081) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MANUy-0004RG-AN for qemu-devel@nongnu.org; Sat, 30 May 2009 08:17:16 -0400 Date: Sat, 30 May 2009 14:17:10 +0200 From: Andrea Arcangeli Subject: Re: [Qemu-devel] fix bdrv_read/write_em and qemu_aio_flush Message-ID: <20090530121709.GA22104@random.random> References: <20090528163309.GJ20464@random.random> <20090530100842.GA28053@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090530100842.GA28053@lst.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: qemu-devel@nongnu.org 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.