On Tue, Mar 27, 2012 at 12:29:09PM +0800, Zhi Yong Wu wrote: > On Mon, Mar 26, 2012 at 10:21 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote: > > On Tue, Mar 20, 2012 at 11:44 AM, Stefan Hajnoczi > > <stefa...@linux.vnet.ibm.com> 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. > IDE is using both sync API and async API when guest boot.
Yes, a warning patch must handle the synchronous I/O case during startup (guessing disk geometry), which I think happens even for virtio-blk. Stefan