kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).