[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs
From: |
Filip Navara |
Subject: |
Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs |
Date: |
Tue, 23 Jun 2009 20:08:55 +0200 |
On Tue, Jun 23, 2009 at 6:57 PM, Avi Kivity<address@hidden> 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. However, it leaves 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? qemu-iotests
>> seems to do fine on vmdk so it's nothing yet exercised by it.
>>
>
> Just starting qemu with a vmdk image hangs. I think the very first read
> 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)
>>> {
>>> BlockDriverAIOCBSync *acb = (BlockDriverAIOCBSync *)blockacb;
>>> qemu_bh_delete(acb->bh);
>>> + acb->bh = NULL;
>>> qemu_aio_release(acb);
>>> }
>>>
>>> @@ -1391,6 +1392,7 @@ static void bdrv_aio_bh_cb(void *opaque)
>>> qemu_vfree(acb->bounce);
>>> acb->common.cb(acb->common.opaque, acb->ret);
>>> qemu_bh_delete(acb->bh);
>>> + acb->bh = NULL;
>>> qemu_aio_release(acb);
>>> }
>>>
>>
>> I think not having the state of the private acb area cleared over a
>> free/realloc cycle is pretty dangerous. Wouldn't it be better to always
>> clear that space in qemu_aio_get?
>>
>
> Maybe, but that's a bigger change. Let's start with this (in stable- too)
> 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
- [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Avi Kivity, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Filip Navara, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Christoph Hellwig, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Avi Kivity, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Christoph Hellwig, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Avi Kivity, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Christoph Hellwig, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Avi Kivity, 2009/06/23
- Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Filip Navara, 2009/06/23
Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs, Stefan Weil, 2009/06/24