From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MJAQZ-0003OJ-2g for qemu-devel@nongnu.org; Tue, 23 Jun 2009 14:09:03 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MJAQU-0003Ju-Cf for qemu-devel@nongnu.org; Tue, 23 Jun 2009 14:09:02 -0400 Received: from [199.232.76.173] (port=57835 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MJAQU-0003JL-3z for qemu-devel@nongnu.org; Tue, 23 Jun 2009 14:08:58 -0400 Received: from mail-ew0-f211.google.com ([209.85.219.211]:48763) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MJAQT-0003bm-CY for qemu-devel@nongnu.org; Tue, 23 Jun 2009 14:08:57 -0400 Received: by ewy7 with SMTP id 7so376502ewy.34 for ; Tue, 23 Jun 2009 11:08:55 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4A41095D.7070608@redhat.com> References: <1245763236-23464-1-git-send-email-avi@redhat.com> <20090623165006.GC27211@lst.de> <4A41095D.7070608@redhat.com> Date: Tue, 23 Jun 2009 20:08:55 +0200 Message-ID: <5b31733c0906231108y28995333s83124722195faf16@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs From: Filip Navara Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Christoph Hellwig , qemu-devel@nongnu.org On Tue, Jun 23, 2009 at 6:57 PM, Avi Kivity wrote: > On 06/23/2009 07:50 PM, Christoph Hellwig wrote: >> >> On Tue, Jun 23, 2009 at 04:20:36PM +0300, Avi Kivity wrote: >> >>> >>> Commit 6a7ad299 ("Call qemu_bh_delete at bdrv_aio_bh_cb") deletes >>> emulated >>> aio bottom halves to prevent endless accumulation. =A0However, it leave= s a >>> stale ->bh pointer, which is then waited on when the aio is reused. >>> >>> Zeroing the pointer fixes the issue, allowing vmdk format images to be >>> used. >>> >> >> What operations on vmdk images does this cause to fail? =A0qemu-iotests >> seems to do fine on vmdk so it's nothing yet exercised by it. >> > > Just starting qemu with a vmdk image hangs. =A0I think the very first rea= d > triggers it. Actually I think it's the second read ;-) > >>> --- a/block.c >>> +++ b/block.c >>> @@ -1374,6 +1374,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB >>> *blockacb) >>> =A0{ >>> =A0 =A0 =A0BlockDriverAIOCBSync *acb =3D (BlockDriverAIOCBSync *)blocka= cb; >>> =A0 =A0 =A0qemu_bh_delete(acb->bh); >>> + =A0 =A0acb->bh =3D NULL; >>> =A0 =A0 =A0qemu_aio_release(acb); >>> =A0} >>> >>> @@ -1391,6 +1392,7 @@ static void bdrv_aio_bh_cb(void *opaque) >>> =A0 =A0 =A0qemu_vfree(acb->bounce); >>> =A0 =A0 =A0acb->common.cb(acb->common.opaque, acb->ret); >>> =A0 =A0 =A0qemu_bh_delete(acb->bh); >>> + =A0 =A0acb->bh =3D NULL; >>> =A0 =A0 =A0qemu_aio_release(acb); >>> =A0} >>> >> >> I think not having the state of the private acb area cleared over a >> free/realloc cycle is pretty dangerous. =A0Wouldn't it be better to alwa= ys >> clear that space in qemu_aio_get? >> > > Maybe, but that's a bigger change. =A0Let's start with this (in stable- t= oo) > and rework aio later. > Agreed, let's get this in, the win32 builds are seriously affected by the bug due to the absence of AIO on the platform. Best regards, Filip Navara