From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Date: Fri, 19 Feb 2016 15:44:23 +0100 Message-ID: <20160219144422.GA2456@potion.brq.redhat.com> References: <1455736496-374-1-git-send-email-rkrcmar@redhat.com> <1455736496-374-2-git-send-email-rkrcmar@redhat.com> <56C5EDA9.9030204@redhat.com> <20160218165608.GC18904@potion.brq.redhat.com> <56C6004F.2050105@redhat.com> <56C60579.5040003@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Yuki Shibuya , Rik van Riel , Peter Krempa To: Paolo Bonzini Return-path: Content-Disposition: inline In-Reply-To: <56C60579.5040003@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org [Cc'd Peter, the last guy that touched timers in libvirt, because he might know what tick policies are supposed to be.] 2016-02-18 18:55+0100, Paolo Bonzini: > On 18/02/2016 18:33, Paolo Bonzini wrote: >> On 18/02/2016 17:56, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >>> 2016-02-18 17:13+0100, Paolo Bonzini: >>>> On 17/02/2016 20:14, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >>>>> Discard policy uses ack_notifiers to prevent injection of PIT int= errupts >>>>> before EOI from the last one. >>>>> >>>>> This patch changes the policy to always try to deliver the interr= upt, >>>>> which makes a difference when its vector is in ISR. >>>>> Old implementation would drop the interrupt, but proposed one inj= ects to >>>>> IRR, like real hardware would. >>>> >>>> This seems like what libvirt calls the "merge" policy: >>> >>> Oops, I never looked beyond QEMU after seeing that the naming in li= bvirt >>> doesn't even match ... >>> >>> I think the policy that KVM implements (which I call discard) is "d= elay" >>> in libvirt. (https://libvirt.org/formatdomain.html#elementsTime) (I looked at libvirt code, but couldn't find any use of merge or discar= d policies, so please bear with me as I disagree wherever it's possible.= ) >> Suppose the scheduled ticks are at times 0, 20, 40, 60, 80. The EOI= for >> time 0 is only delivered at time 42, other EOIs are timely. >>=20 >> The resulting injections are: >> - for catchup, which QEMU calls slew: 0, 42, 51, 60, 80. >>=20 >> - for merge: 0, 20 (in IRR, delivered at 42), 60, 80. >>=20 >> For delay I *think* it would be 0, 42, 62, 82, 102. I could call this "delay". Continue to deliver ticks at the normal rate. The guest time will be delayed due to the late tick At 82 time units, the guest thinks it's 60, so the guest will do everything late. (Leading us to call it delayed?!) =46ew examples of "delay" that I find easier to accept: 0, 60, 80. 0, 42, 60, 80. Because we haven't missed the tick at 20, it just took a while to be delivered. (Semantics ...) > Wrong: for delay it is something like 0, 42, 43, 60, 80. Aargh! One KVM policy does this and QEMU calls it 'delay'. I think that libvirt would call it "catchup". Deliver ticks at a higher rate to catch up with the missed tick. The guest time should not be delayed once catchup is complete. At 80, the guest time is 80; no signs of delay. > Your patch does the right thing, QEMU is wrong in calling the policy > "discard" where it should have been "merge". In fact both i8254 and = RTC > use the same wrong nomenclature. Terminlogy does suck. (Maybe it stems from the fact that QEMU talks about lost ticks, but libvirt about ticks?) Nevertheless, I don't think that libvirt "merge" covers what PIT does i= n KVM or real hardware. Merge the missed tick(s) into one tick and inject. The guest time may be delayed, depending on how the OS reacts to the merging of ticks No merging is happening in KVM or real hardware: every tick is exactly one tick, so the guest cannot tell that we missed some ticks and the time is delayed. If a tick made it into clear IRR, it's not missed. In the example: >> - for merge: 0, 20 (in IRR, delivered at 42), 60, 80. at 80, the guest thinks it's 60. I think that merge might do: 0, 42, 60, 80. But the tick at 42 is counted as two ticks (20, 40) in the guest. The main problem of this interpretation is that discard is a subset of merge: >> - for discard: 0, 60, 80. The tick at 60 has to be counted as three ticks (20, 40, 60). *throws hands into the air and runs in circles*