kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Venkatesh Srinivas <venkateshs@chromium.org>
Cc: kvm@vger.kernel.org, dmatlack@google.com, pbonzini@redhat.com
Subject: Re: [PATCH] kvm: Cap halt polling at kvm->max_halt_poll_ns
Date: Thu, 6 May 2021 16:13:03 +0000	[thread overview]
Message-ID: <YJQVj3GaVp9tvWog@google.com> (raw)
In-Reply-To: <20210506152442.4010298-1-venkateshs@chromium.org>

Prefer capitalizing KVM in the shortlog, if only because I'm lazy with grep :-)

On Thu, May 06, 2021, Venkatesh Srinivas wrote:
> From: David Matlack <dmatlack@google.com>
> 
> When growing halt-polling, there is no check that the poll time exceeds
> the per-VM limit. It's possible for vcpu->halt_poll_ns to grow past
> kvm->max_halt_poll_ns and stay there until a halt which takes longer
> than kvm->halt_poll_ns.
> 

Fixes: acd05785e48c ("kvm: add capability for halt polling")

and probably Cc: stable@ too.


> Signed-off-by: David Matlack <dmatlack@google.com>
> Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>
> ---
>  virt/kvm/kvm_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 2799c6660cce..120817c5f271 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2893,8 +2893,8 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
>  	if (val < grow_start)
>  		val = grow_start;
>  
> -	if (val > halt_poll_ns)
> -		val = halt_poll_ns;
> +	if (val > vcpu->kvm->max_halt_poll_ns)
> +		val = vcpu->kvm->max_halt_poll_ns;

Hmm, I would argue that the introduction of the capability broke halt_poll_ns.
The halt_poll_ns module param is writable after KVM is loaded.  Prior to the
capability, that meant the admin could adjust the param on the fly and all vCPUs
would honor the new value as it was changed.

By snapshotting the module param at VM creation, those semantics were lost.
That's not necessarily wrong/bad, but I don't see anything in the changelog for
the capability that suggests killing the old behavior was intentional/desirable.

>  
>  	vcpu->halt_poll_ns = val;
>  out:
> -- 
> 2.31.1.607.g51e8a6a459-goog
> 

  reply	other threads:[~2021-05-06 16:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06 15:24 Venkatesh Srinivas
2021-05-06 16:13 ` Sean Christopherson [this message]
2021-05-06 16:30   ` David Matlack
2021-05-06 17:41     ` Sean Christopherson

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=YJQVj3GaVp9tvWog@google.com \
    --to=seanjc@google.com \
    --cc=dmatlack@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=venkateshs@chromium.org \
    --subject='Re: [PATCH] kvm: Cap halt polling at kvm->max_halt_poll_ns' \
    /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

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).