kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Joao Martins <joao.m.martins@oracle.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	kvm-devel <kvm@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Radim Krcmar <rkrcmar@redhat.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Wanpeng Li <kernellwp@gmail.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Raslan KarimAllah <karahmed@amazon.de>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Ankur Arora <ankur.a.arora@oracle.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: [patch v2 3/3] cpuidle-haltpoll: disable host side polling when kvm virtualized
Date: Thu, 6 Jun 2019 17:01:31 -0400	[thread overview]
Message-ID: <20190606210131.GE20928@redhat.com> (raw)
In-Reply-To: <70bf0678-1ff7-bfc4-1ce2-fe7392ad663a@oracle.com>

On Thu, Jun 06, 2019 at 08:22:40PM +0100, Joao Martins wrote:
> On 6/6/19 7:51 PM, Joao Martins wrote:
> > On 6/6/19 7:36 PM, Andrea Arcangeli wrote:
> >> Hello,
> >>
> >> On Thu, Jun 06, 2019 at 07:25:28PM +0100, Joao Martins wrote:
> >>> But I wonder whether we should fail to load cpuidle-haltpoll when host halt
> >>> polling can't be disabled[*]? That is to avoid polling in both host and guest
> >>> and *possibly* avoid chances for performance regressions when running on older
> >>> hypervisors?
> >>
> >> I don't think it's necessary: that would force an upgrade of the host
> >> KVM version in order to use the guest haltpoll feature with an
> >> upgraded guest kernel that can use the guest haltpoll.
> >>
> > Hence why I was suggesting a *guest* cpuidle-haltpoll module parameter to still
> > allow it to load or otherwise (or allow guest to pick).
> > 
> By 'still allow it to load', I meant specifically to handle the case when host
> polling control is not supported and what to do in that case.

All right, we could add a force=1 parameter to force loading as an
opt-in (and fail load by default with force=0).

> >> The guest haltpoll is self contained in the guest, so there's no
> >> reason to prevent that by design or to force upgrade of the KVM host
> >> version. It'd be more than enough to reload kvm.ko in the host with
> >> the host haltpoll set to zero with the module parameter already
> >> available, to achieve the same runtime without requiring a forced host
> >> upgrade.
> >>
> > It's just with the new driver we unilaterally poll on both sides, just felt I
> > would point it out should this raise unattended performance side effects ;)
> > 
> To be clear: by 'unilaterally' I was trying to refer to hosts KVM without
> polling control (which is safe to say that it is the majority atm?).
> 
> Alternatively, there's always the option that if guest sees any issues on that
> case (with polling on both sides=, that it can always blacklist
> cpuidle-haltpoll. But may this is not an issue and perhaps majority of users
> still observes benefit when polling is enabled on guest and host.

It should be workload dependent if it increases performance to
haltpoll both in both host kernel and guest, the only sure cons is
that it'd burn some more energy..

By default the cpuidle-haltpoll driver shouldn't get loaded if it's
built as a module (the expectation is the default will be =m), and it
can still easily disabled with rmmod if it hurts performance.

So the policy if to activate guest haltpoll or not will still reside
in the guest userland: the guest userland code or guest admin, without
a force parameter has to decide if to load or not to load the module,
with the force=1 parameter it'll have to decide if to load it with =0
or =1 (or load it first with =0 and then decide if to try to load it
again with =1 which would be the benefit the force parameter
provides). To decide if to load or not to load it, the guest userland
could check if there's no support for disabling the host haltpoll,
which can be verified with rdmsr too.

# rdmsr 0x4b564d05
rdmsr: CPU 0 cannot read MSR 0x4b564d05

Adding a force=1 parameter to force loading will just add a few lines
more of kernel code, I'm neutral on that but it looks fine either
ways.

Thanks,
Andrea

  reply	other threads:[~2019-06-06 21:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-03 22:52 [patch 0/3] cpuidle-haltpoll driver (v2) Marcelo Tosatti
2019-06-03 22:52 ` [patch 1/3] drivers/cpuidle: add cpuidle-haltpoll driver Marcelo Tosatti
2019-06-05  8:16   ` Ankur Arora
2019-06-06 17:51   ` Andrea Arcangeli
2019-06-07 20:20     ` Marcelo Tosatti
2019-06-06 19:03   ` Peter Zijlstra
2019-06-07  9:54   ` Rafael J. Wysocki
2019-06-03 22:52 ` [patch 2/3] kvm: x86: add host poll control msrs Marcelo Tosatti
2019-06-06 12:04   ` Paolo Bonzini
2019-06-03 22:52 ` [patch 3/3] cpuidle-haltpoll: disable host side polling when kvm virtualized Marcelo Tosatti
2019-06-04  1:26   ` kbuild test robot
2019-06-04 12:24   ` [patch v2 " Marcelo Tosatti
2019-06-06 18:25     ` Joao Martins
2019-06-06 18:36       ` Andrea Arcangeli
2019-06-06 18:51         ` Joao Martins
2019-06-06 19:22           ` Joao Martins
2019-06-06 21:01             ` Andrea Arcangeli [this message]
2019-06-07 20:38         ` Marcelo Tosatti
2019-06-07 20:25       ` Marcelo Tosatti
2019-06-07  9:49 ` [patch 0/3] cpuidle-haltpoll driver (v2) Rafael J. Wysocki
2019-06-07 17:16   ` Marcelo Tosatti
2019-06-07 18:22     ` Paolo Bonzini
2019-06-07 21:38       ` Marcelo Tosatti
2019-06-10 14:59   ` Marcelo Tosatti
2019-06-10 22:03     ` Rafael J. Wysocki
2019-06-11 14:26       ` Marcelo Tosatti
2019-06-11 21:24         ` Rafael J. Wysocki
2019-06-17 15:57           ` Peter Zijlstra

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=20190606210131.GE20928@redhat.com \
    --to=aarcange@redhat.com \
    --cc=ankur.a.arora@oracle.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=borntraeger@de.ibm.com \
    --cc=joao.m.martins@oracle.com \
    --cc=karahmed@amazon.de \
    --cc=kernellwp@gmail.com \
    --cc=konrad.wilk@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rkrcmar@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 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).