All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: lower default for halt_poll_ns
Date: Tue, 16 May 2017 22:41:10 +0200	[thread overview]
Message-ID: <20170516204109.GA15344@potion> (raw)
In-Reply-To: <4385bc02-2456-e8a7-a22b-cc2783ad6822@redhat.com>

2017-05-16 18:58+0200, Paolo Bonzini:
> On 18/04/2017 12:41, Paolo Bonzini wrote:
>> In some fio benchmarks, halt_poll_ns=400000 caused CPU utilization to
>> increase heavily even in cases where the performance improvement was
>> small.  In particular, bandwidth divided by CPU usage was as much as
>> 60% lower.
>> 
>> To some extent this is the expected effect of the patch, and the
>> additional CPU utilization is only visible when running the
>> benchmarks.  However, halving the threshold also halves the extra
>> CPU utilization (from +30-130% to +20-70%) and has no negative
>> effect on performance.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Ping?

I didn't see any regression in crude benchmarks either and 200 us seems
better anyway (just under 1/2 of Windows' timer frequency).

Queued for rc2 as it is simple enough, thanks.

---
Still, I think we have dynamic polling to mitigate this overhead;
how was it behaving?

I noticed a questionable decision in growing the window:
we know how long the polling should have been (block_ns), but we do not
use that information to set the next halt_poll_ns.

Has something like this been tried?

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f0fe9d02f6bb..d8dbf50957fc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2193,7 +2193,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 		/* we had a short halt and our poll time is too small */
 		else if (vcpu->halt_poll_ns < halt_poll_ns &&
 			block_ns < halt_poll_ns)
-			grow_halt_poll_ns(vcpu);
+			vcpu->halt_poll_ns = block_ns /* + x ? */;
 	} else
 		vcpu->halt_poll_ns = 0;
 

It would avoid a case where several halts in a row were interrupted
after 300 us, but on the first one we'd schedule out after 10 us, then
after 20, 40, 80, 160, and finally have the successful poll at 320 us,
but we have just wasted time if the window is reset at any point before
that.

(I really don't like benchmarking ...)

Thanks.

  reply	other threads:[~2017-05-16 20:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-18 10:41 [PATCH] KVM: x86: lower default for halt_poll_ns Paolo Bonzini
2017-05-16 16:58 ` Paolo Bonzini
2017-05-16 20:41   ` Radim Krčmář [this message]
2017-05-17  6:54     ` 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=20170516204109.GA15344@potion \
    --to=rkrcmar@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /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.