qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] block: add the support to drain throttled reque


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH] block: add the support to drain throttled requests
Date: Mon, 26 Mar 2012 15:21:59 +0100

On Tue, Mar 20, 2012 at 11:44 AM, Stefan Hajnoczi
<address@hidden> wrote:
> On Tue, Mar 20, 2012 at 10:58:10AM +0100, Kevin Wolf wrote:
>> Am 20.03.2012 10:47, schrieb Paolo Bonzini:
>> > Il 20/03/2012 10:40, Zhi Yong Wu ha scritto:
>> >> HI, Kevin,
>> >>
>> >> We hope that I/O throttling can be shipped without known issue in QEMU
>> >> 1.1, so if you are available, can you give this patch some love?
>> >
>> > I'm sorry to say this, but I think I/O throttling is impossible to save.
>> >  As it is implemented now, it just cannot work in the presence of
>> > synchronous I/O, except at the cost of busy waiting with the global
>> > mutex taken.  See the message from Stefan yesterday.
>>
>> qemu_aio_flush() is busy waiting with the global mutex taken anyway, so
>> it doesn't change that much.
>
> Yesterday I only posted an analysis of the bug but here are some
> thoughts on how to move forward.  Throttling itself is not the problem.
> We've known that synchronous operations in the vcpu thread are a problem
> long before throttling.  This is just another reason to convert device
> emulation to use asynchronous interfaces.
>
> Here is the list of device models that perform synchronous block I/O:
> hw/fdc.c
> hw/ide/atapi.c
> hw/ide/core.c
> hw/nand.c
> hw/onenand.c
> hw/pflash_cfi01.c
> hw/pflash_cfi02.c
> hw/sd.c
>
> Zhi Hui Li is working on hw/fdc.c and recently sent a patch.
>
> I think it's too close to QEMU 1.1 to convert all the remaining devices
> and test them properly before the soft-freeze.  But it's probably
> possible to convert IDE before the soft-freeze.
>
> In the meantime we could add this to bdrv_rw_co():
>
> if (bs->io_limits_enabled) {
>    fprintf(stderr, "Disabling I/O throttling on '%s' due "
>            "to synchronous I/O\n", bdrv_get_device_name(bs));
>    bdrv_io_limits_disable(bs);
> }
>
> It's not pretty but tells the user there is an issue and avoids
> deadlocking.

No one has commented on this suggestion.  I think leaving a known hang
in QEMU 1.1 is undesirable.  Better to have this warning and disable
throttling in the case we cannot support right now.

Kevin: Would you accept a patch like this?  Or do you have another
solution in mind?

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]