[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block/throttle: Use host clock type
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] block/throttle: Use host clock type |
Date: |
Fri, 13 Mar 2015 12:09:06 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 13/03/2015 09:27, Fam Zheng wrote:
> On Fri, 03/13 09:08, Paolo Bonzini wrote:
>>
>>
>> On 13/03/2015 07:35, Fam Zheng wrote:
>>> Throttle timers won't make any progress when VCPU is not running, which
>>> is prone to stall the request queue in cases like utils, qtest,
>>> suspending, and live migration, unless carefully handled. What we do now
>>> is crude. For example in bdrv_drain_all, requests are resumed
>>> immediately without consulting throttling timer. Unfortunately
>>> bdrv_drain_all is so widely used that there may be too many holes that
>>> guest could bypass throttling.
>>>
>>> If we use the host clock, we can just trust the nested poll when waiting
>>> for requests.
>>>
>>> Signed-off-by: Fam Zheng <address@hidden>
>>> ---
>>> block.c | 2 +-
>>> tests/test-throttle.c | 14 +++++++-------
>>
>> I think test-throttle.c should use the vm_clock. At some point it was
>> managing the clock manually (by overriding cpu_get_clock from
>> libqemustub.a), and that's only possible with QEMU_CLOCK_VIRTUAL.
>
> Ah! That is in iotests 093 (hint: authord by Fam Zheng :-/), which WILL be
> complicated if block.c switches away from QEMU_CLOCK_VIRTUAL. But I'll do the
> work if we decide to make this change.
>
> As to tests/test-throttle.c, I don't see its dependency on clock type, so
> either way should work and I don't mind keeping it as-is at all.
If there's another way to do the same thing, I'd prefer it.
For example, can we call bdrv_drain_all() at the beginning of
do_vm_stop, before pausing the VCPUs?
>> As to block.c, I'll leave the review to the block folks. But I think
>> QEMU_CLOCK_REALTIME is preferrable.
>
> Real time clock should be fine, but we should review that the code handles
> clock reversing.
QEMU_CLOCK_HOST is the one that follows the wall clock;
QEMU_CLOCK_REALTIME is monotonic. :)
Paolo