From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41217) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWNV3-0006Cb-G4 for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:11:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YWNSq-00085w-CG for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:09:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWNSq-00085s-3U for qemu-devel@nongnu.org; Fri, 13 Mar 2015 07:09:12 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2DB9AT5007833 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 13 Mar 2015 07:09:11 -0400 Message-ID: <5502C552.2050006@redhat.com> Date: Fri, 13 Mar 2015 12:09:06 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1426228529-15969-1-git-send-email-famz@redhat.com> <55029B1A.6000802@redhat.com> <20150313082734.GA3527@ad.nay.redhat.com> In-Reply-To: <20150313082734.GA3527@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block/throttle: Use host clock type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-devel 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 >>> --- >>> 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