kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint
@ 2020-03-04 11:32 Maciej S. Szmigiero
  2020-03-12 16:17 ` Marcelo Tosatti
  0 siblings, 1 reply; 4+ messages in thread
From: Maciej S. Szmigiero @ 2020-03-04 11:32 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano
  Cc: Joao Martins, Boris Ostrovsky, Marcelo Tosatti, kvm, linux-pm,
	linux-kernel

From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

Before commit 1328edca4a14 ("cpuidle-haltpoll: Enable kvm guest polling
when dedicated physical CPUs are available") the cpuidle-haltpoll driver
could also be used in scenarios when the host does not advertise the
KVM_HINTS_REALTIME hint.

While the behavior introduced by the aforementioned commit makes sense as
the default there are cases where the old behavior is desired, for example,
when other kernel changes triggered by presence by this hint are unwanted,
for some workloads where the latency benefit from polling overweights the
loss from idle CPU capacity that otherwise would be available, or just when
running under older Qemu versions that lack this hint.

Let's provide a typical "force" module parameter that allows restoring the
old behavior.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 drivers/cpuidle/cpuidle-haltpoll.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Changes from v1:
Make the module parameter description more general, don't unnecessarily
break a line in haltpoll_init().

diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
index b0ce9bc78113..db124bc1ca2c 100644
--- a/drivers/cpuidle/cpuidle-haltpoll.c
+++ b/drivers/cpuidle/cpuidle-haltpoll.c
@@ -18,6 +18,10 @@
 #include <linux/kvm_para.h>
 #include <linux/cpuidle_haltpoll.h>
 
+static bool force __read_mostly;
+module_param(force, bool, 0444);
+MODULE_PARM_DESC(force, "Load unconditionally");
+
 static struct cpuidle_device __percpu *haltpoll_cpuidle_devices;
 static enum cpuhp_state haltpoll_hp_state;
 
@@ -90,6 +94,11 @@ static void haltpoll_uninit(void)
 	haltpoll_cpuidle_devices = NULL;
 }
 
+static bool haltpool_want(void)
+{
+	return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
+}
+
 static int __init haltpoll_init(void)
 {
 	int ret;
@@ -101,8 +110,7 @@ static int __init haltpoll_init(void)
 
 	cpuidle_poll_state_init(drv);
 
-	if (!kvm_para_available() ||
-		!kvm_para_has_hint(KVM_HINTS_REALTIME))
+	if (!kvm_para_available() || !haltpool_want())
 		return -ENODEV;
 
 	ret = cpuidle_register_driver(drv);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint
  2020-03-04 11:32 [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint Maciej S. Szmigiero
@ 2020-03-12 16:17 ` Marcelo Tosatti
  2020-03-13 17:49   ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Marcelo Tosatti @ 2020-03-12 16:17 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: Rafael J. Wysocki, Daniel Lezcano, Joao Martins, Boris Ostrovsky,
	kvm, linux-pm, linux-kernel

On Wed, Mar 04, 2020 at 12:32:48PM +0100, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> Before commit 1328edca4a14 ("cpuidle-haltpoll: Enable kvm guest polling
> when dedicated physical CPUs are available") the cpuidle-haltpoll driver
> could also be used in scenarios when the host does not advertise the
> KVM_HINTS_REALTIME hint.
> 
> While the behavior introduced by the aforementioned commit makes sense as
> the default there are cases where the old behavior is desired, for example,
> when other kernel changes triggered by presence by this hint are unwanted,
> for some workloads where the latency benefit from polling overweights the
> loss from idle CPU capacity that otherwise would be available, or just when
> running under older Qemu versions that lack this hint.
> 
> Let's provide a typical "force" module parameter that allows restoring the
> old behavior.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>  drivers/cpuidle/cpuidle-haltpoll.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> Changes from v1:
> Make the module parameter description more general, don't unnecessarily
> break a line in haltpoll_init().
> 
> diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> index b0ce9bc78113..db124bc1ca2c 100644
> --- a/drivers/cpuidle/cpuidle-haltpoll.c
> +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> @@ -18,6 +18,10 @@
>  #include <linux/kvm_para.h>
>  #include <linux/cpuidle_haltpoll.h>
>  
> +static bool force __read_mostly;
> +module_param(force, bool, 0444);
> +MODULE_PARM_DESC(force, "Load unconditionally");
> +
>  static struct cpuidle_device __percpu *haltpoll_cpuidle_devices;
>  static enum cpuhp_state haltpoll_hp_state;
>  
> @@ -90,6 +94,11 @@ static void haltpoll_uninit(void)
>  	haltpoll_cpuidle_devices = NULL;
>  }
>  
> +static bool haltpool_want(void)
> +{
> +	return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
> +}
> +
>  static int __init haltpoll_init(void)
>  {
>  	int ret;
> @@ -101,8 +110,7 @@ static int __init haltpoll_init(void)
>  
>  	cpuidle_poll_state_init(drv);
>  
> -	if (!kvm_para_available() ||
> -		!kvm_para_has_hint(KVM_HINTS_REALTIME))
> +	if (!kvm_para_available() || !haltpool_want())
>  		return -ENODEV;
>  
>  	ret = cpuidle_register_driver(drv);

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint
  2020-03-12 16:17 ` Marcelo Tosatti
@ 2020-03-13 17:49   ` Rafael J. Wysocki
  2020-03-14 10:40     ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-03-13 17:49 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Maciej S. Szmigiero, Rafael J. Wysocki, Daniel Lezcano,
	Joao Martins, Boris Ostrovsky, kvm-devel, Linux PM,
	Linux Kernel Mailing List

On Thu, Mar 12, 2020 at 5:36 PM Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> On Wed, Mar 04, 2020 at 12:32:48PM +0100, Maciej S. Szmigiero wrote:
> > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> >
> > Before commit 1328edca4a14 ("cpuidle-haltpoll: Enable kvm guest polling
> > when dedicated physical CPUs are available") the cpuidle-haltpoll driver
> > could also be used in scenarios when the host does not advertise the
> > KVM_HINTS_REALTIME hint.
> >
> > While the behavior introduced by the aforementioned commit makes sense as
> > the default there are cases where the old behavior is desired, for example,
> > when other kernel changes triggered by presence by this hint are unwanted,
> > for some workloads where the latency benefit from polling overweights the
> > loss from idle CPU capacity that otherwise would be available, or just when
> > running under older Qemu versions that lack this hint.
> >
> > Let's provide a typical "force" module parameter that allows restoring the
> > old behavior.
> >
> > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> > ---
> >  drivers/cpuidle/cpuidle-haltpoll.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > Changes from v1:
> > Make the module parameter description more general, don't unnecessarily
> > break a line in haltpoll_init().
> >
> > diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> > index b0ce9bc78113..db124bc1ca2c 100644
> > --- a/drivers/cpuidle/cpuidle-haltpoll.c
> > +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> > @@ -18,6 +18,10 @@
> >  #include <linux/kvm_para.h>
> >  #include <linux/cpuidle_haltpoll.h>
> >
> > +static bool force __read_mostly;
> > +module_param(force, bool, 0444);
> > +MODULE_PARM_DESC(force, "Load unconditionally");
> > +
> >  static struct cpuidle_device __percpu *haltpoll_cpuidle_devices;
> >  static enum cpuhp_state haltpoll_hp_state;
> >
> > @@ -90,6 +94,11 @@ static void haltpoll_uninit(void)
> >       haltpoll_cpuidle_devices = NULL;
> >  }
> >
> > +static bool haltpool_want(void)
> > +{
> > +     return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
> > +}
> > +
> >  static int __init haltpoll_init(void)
> >  {
> >       int ret;
> > @@ -101,8 +110,7 @@ static int __init haltpoll_init(void)
> >
> >       cpuidle_poll_state_init(drv);
> >
> > -     if (!kvm_para_available() ||
> > -             !kvm_para_has_hint(KVM_HINTS_REALTIME))
> > +     if (!kvm_para_available() || !haltpool_want())
> >               return -ENODEV;
> >
> >       ret = cpuidle_register_driver(drv);
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

I'm taking this as a Reviewed-by, thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint
  2020-03-13 17:49   ` Rafael J. Wysocki
@ 2020-03-14 10:40     ` Rafael J. Wysocki
  0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-03-14 10:40 UTC (permalink / raw)
  To: Maciej S. Szmigiero, Marcelo Tosatti
  Cc: Rafael J. Wysocki, Daniel Lezcano, Joao Martins, Boris Ostrovsky,
	kvm-devel, Linux PM, Linux Kernel Mailing List

On Fri, Mar 13, 2020 at 6:49 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Mar 12, 2020 at 5:36 PM Marcelo Tosatti <mtosatti@redhat.com> wrote:
> >
> > On Wed, Mar 04, 2020 at 12:32:48PM +0100, Maciej S. Szmigiero wrote:
> > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > >
> > > Before commit 1328edca4a14 ("cpuidle-haltpoll: Enable kvm guest polling
> > > when dedicated physical CPUs are available") the cpuidle-haltpoll driver
> > > could also be used in scenarios when the host does not advertise the
> > > KVM_HINTS_REALTIME hint.
> > >
> > > While the behavior introduced by the aforementioned commit makes sense as
> > > the default there are cases where the old behavior is desired, for example,
> > > when other kernel changes triggered by presence by this hint are unwanted,
> > > for some workloads where the latency benefit from polling overweights the
> > > loss from idle CPU capacity that otherwise would be available, or just when
> > > running under older Qemu versions that lack this hint.
> > >
> > > Let's provide a typical "force" module parameter that allows restoring the
> > > old behavior.
> > >
> > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> > > ---
> > >  drivers/cpuidle/cpuidle-haltpoll.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > Changes from v1:
> > > Make the module parameter description more general, don't unnecessarily
> > > break a line in haltpoll_init().
> > >
> > > diff --git a/drivers/cpuidle/cpuidle-haltpoll.c b/drivers/cpuidle/cpuidle-haltpoll.c
> > > index b0ce9bc78113..db124bc1ca2c 100644
> > > --- a/drivers/cpuidle/cpuidle-haltpoll.c
> > > +++ b/drivers/cpuidle/cpuidle-haltpoll.c
> > > @@ -18,6 +18,10 @@
> > >  #include <linux/kvm_para.h>
> > >  #include <linux/cpuidle_haltpoll.h>
> > >
> > > +static bool force __read_mostly;
> > > +module_param(force, bool, 0444);
> > > +MODULE_PARM_DESC(force, "Load unconditionally");
> > > +
> > >  static struct cpuidle_device __percpu *haltpoll_cpuidle_devices;
> > >  static enum cpuhp_state haltpoll_hp_state;
> > >
> > > @@ -90,6 +94,11 @@ static void haltpoll_uninit(void)
> > >       haltpoll_cpuidle_devices = NULL;
> > >  }
> > >
> > > +static bool haltpool_want(void)
> > > +{
> > > +     return kvm_para_has_hint(KVM_HINTS_REALTIME) || force;
> > > +}
> > > +
> > >  static int __init haltpoll_init(void)
> > >  {
> > >       int ret;
> > > @@ -101,8 +110,7 @@ static int __init haltpoll_init(void)
> > >
> > >       cpuidle_poll_state_init(drv);
> > >
> > > -     if (!kvm_para_available() ||
> > > -             !kvm_para_has_hint(KVM_HINTS_REALTIME))
> > > +     if (!kvm_para_available() || !haltpool_want())
> > >               return -ENODEV;
> > >
> > >       ret = cpuidle_register_driver(drv);
> >
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> I'm taking this as a Reviewed-by, thanks!

Patch applied as 5.7 material, thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-15  3:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 11:32 [PATCH v2] cpuidle-haltpoll: allow force loading on hosts without the REALTIME hint Maciej S. Szmigiero
2020-03-12 16:17 ` Marcelo Tosatti
2020-03-13 17:49   ` Rafael J. Wysocki
2020-03-14 10:40     ` Rafael J. Wysocki

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