From: Paolo Bonzini <pbonzini@redhat.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
Yuki Shibuya <shibuya.yk@ncos.nec.co.jp>,
Rik van Riel <riel@redhat.com>, Peter Krempa <pkrempa@redhat.com>
Subject: Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy
Date: Thu, 25 Feb 2016 20:11:36 +0100 [thread overview]
Message-ID: <56CF51E8.3040502@redhat.com> (raw)
In-Reply-To: <20160225173615.GD18319@potion.redhat.com>
> 2016-02-25 14:38+0100, Paolo Bonzini:
>> On 19/02/2016 15:44, Radim Krčmář wrote:
>>>> The resulting injections are:
>>>>> - for catchup, which QEMU calls slew: 0, 42, 51, 60, 80.
>>
>> I think we agree that 0, 42, 43, 60, 80 is also a "catchup"/"slew"
>> policy.
>
> We do. (Libvirt "catchup" is also QEMU "delay".)
>
>> So we can change QEMU's kvm-i8254 to accept "slew" and warn if
>> "delay" is given.
> **
> QEMU 4e4fa398db69 ("qdev: Introduce lost tick policy property") defines:
>
> delay - replay all lost ticks in a row once the guest accepts them
> again
> slew - lost ticks are gradually replayed at a higher frequency than
> the original tick
>
> "delay" is exactly how kvm-i8254 behaves (in its "reinject" mode), so I
> think we shouldn't change it.
Ooh, I missed this commit message indeed. Then libvirt delay != QEMU
delay, isn't it?
>> In fact "slew" means "a large number or quantity of something" and
>> indeed that's a good word to characterize kvm-i8254's reinjection behavior.
>
> (Isn't it a verb, with a similar meaning as "drift"? ;])
It's a noun too, like "I've just gotten a whole slew of bugs assigned to
me".
>>> Few examples of "delay" that I find easier to accept:
>>> 0, 60, 80.
>>
>> This is "discard".
>
> At 80, the guest thinks that the time is 40, so every action it does
> will still be delayed. I don't see why it isn't libvirt "delay":
> - ticks are delivered at the normal rate
> - guest time is delayed
I can buy this. :)
> I don't think it is libvirt "discard":
> - missed ticks were thrown away
> - future injection continues normally
>
> which is fine, but
> - the guest time is delayed, because there isn't a way to handle lost
> ticks
>
> and this is incompatible with libvirt's definition of "discard"
>
> The guest time may be delayed, unless the OS has explicit handling of
> lost ticks.
>
> "may" doesn't fit. You can only say
> - the guest time is delayed.
>
> which is best described by "delay".
I think we can safely ignore the "may be" -- you cannot say for sure
that the guest time "will" be delayed since you could always have a very
enlightened guest.
... but then, by removing the handwavy "may be" would you say that
libvirt delay and libvirt discard are the same? Would 0, 42, 62, 82 be
a valid implementation of the libvirt "delay" policy?
>> Therefore, it _also_ happens that thanks to IRR and NMI latching you can
>> implement "merge" without having that kind of relationship between the
>> timer device and the interrupt controller.
>
> I disagree. IRR can catch at most one interrupt, so it is insufficient
> to implement libvirt's merge. (libvirt's merge also has the conditional
> "The guest time may be delayed".)
Hmm... is your point that the i8254 _alone_ is implementing discard, and
the tick delivery time is _actually_ 0, 20, 60, 80 (and the t=20 tick is
delivered late but not lost due to the i8259 buffer)? And hence the
QEMU device model should see it as discard. I can definitely agree with
that.
There is still the matter of:
- improving the documentation
- clarifying the meaning of libvirt delay
- deciding whether it's worth changing the meaning of QEMU delay to
match libvirt's (and the default kvm-pit policy from delay to slew)
But if we can agree on this, I can apply patch 1 as is, even for 4.5.
Paolo
next prev parent reply other threads:[~2016-02-25 19:11 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-17 19:14 [PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Radim Krčmář
2016-02-18 16:13 ` Paolo Bonzini
2016-02-18 16:56 ` Radim Krčmář
2016-02-18 17:33 ` Paolo Bonzini
2016-02-18 17:55 ` Paolo Bonzini
2016-02-19 14:44 ` Radim Krčmář
2016-02-25 12:34 ` Peter Krempa
2016-02-25 13:38 ` Paolo Bonzini
2016-02-25 17:36 ` Radim Krčmář
2016-02-25 19:11 ` Paolo Bonzini [this message]
2016-02-26 13:44 ` Radim Krčmář
2016-02-18 18:49 ` Paolo Bonzini
2016-02-17 19:14 ` [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář
2016-02-18 18:04 ` Paolo Bonzini
2016-02-19 15:51 ` Radim Krčmář
2016-02-19 15:56 ` Paolo Bonzini
2016-02-17 19:14 ` [PATCH v2 03/14] KVM: x86: add kvm_pit_reset_reinject Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 04/14] KVM: x86: use atomic_t instead of pit.inject_lock Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 05/14] KVM: x86: tone down WARN_ON pit.state_lock Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 06/14] KVM: x86: pass struct kvm_pit instead of kvm in PIT Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state lock Radim Krčmář
2016-02-18 18:03 ` Paolo Bonzini
2016-02-19 14:45 ` Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy Radim Krčmář
2016-02-18 18:05 ` Paolo Bonzini
2016-02-18 18:08 ` Paolo Bonzini
2016-02-19 15:04 ` Radim Krčmář
2016-02-19 15:06 ` Paolo Bonzini
2016-02-19 15:09 ` Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 09/14] KVM: x86: refactor kvm_create_pit Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 10/14] KVM: x86: refactor kvm_free_pit Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 11/14] KVM: x86: remove pit and kvm from kvm_kpit_state Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 12/14] KVM: x86: remove pointless dereference of PIT Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 13/14] KVM: x86: don't assume layout of kvm_kpit_state Radim Krčmář
2016-02-17 19:14 ` [PATCH v2 14/14] KVM: x86: move PIT timer function initialization Radim Krčmář
2016-02-18 18:11 ` [PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle related code Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56CF51E8.3040502@redhat.com \
--to=pbonzini@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pkrempa@redhat.com \
--cc=riel@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=shibuya.yk@ncos.nec.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.