[PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Qais Yousef]

To allow more flexible opt-in arrangements while still provide a single
kernel for distros, provide a boot time parameter to enable lazy RCU.

Specify:

	rcutree.enable_rcu_lazy

Which also requires

	rcu_nocbs=all

at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
parameter will be ignored if CONFIG_RCU_LAZY is not set.

With this change now lazy RCU is disabled by default if the boot
parameter is not set even when CONFIG_RCU_LAZY is enabled.

Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
---

Makes sense to remove the CONFIG_RCU_LAZY now we have a boot time param?

We can make it a static key too if it *really* matters.

Thanks to Joel for helping initially in reviewing this patch which was intended
originally for Android.

I got some requests to make this a runtime modifiable for init scripts; but
Paul suggested there shall be dragons. So RO it is.


 .../admin-guide/kernel-parameters.txt         |  5 ++++
 kernel/rcu/tree.c                             | 26 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 65731b060e3f..2f0386a12aa7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5021,6 +5021,11 @@
 			this kernel boot parameter, forcibly setting it
 			to zero.
 
+	rcutree.enable_rcu_lazy= [KNL]
+			To save power, batch RCU callbacks and flush after
+			delay, memory pressure or callback list growing too
+			big.
+
 	rcuscale.gp_async= [KNL]
 			Measure performance of asynchronous
 			grace-period primitives such as call_rcu().
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 3ac3c846105f..e0885905b3f6 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2718,7 +2718,30 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
 	}
 }
 
+static bool enable_rcu_lazy;
 #ifdef CONFIG_RCU_LAZY
+/* Enable lazy rcu at boot time */
+static int param_set_rcu_lazy(const char *val, const struct kernel_param *kp)
+{
+	int ret;
+
+	/*
+	 * Make sure a grace period has passed before and after flipping the
+	 * switch.
+	 */
+	rcu_barrier();
+	ret = param_set_bool(val, kp);
+	rcu_barrier();
+
+	return ret;
+}
+static const struct kernel_param_ops rcu_lazy_ops = {
+	.flags = KERNEL_PARAM_OPS_FL_NOARG,
+	.set = param_set_rcu_lazy,
+	.get = param_get_bool,
+};
+module_param_cb(enable_rcu_lazy, &rcu_lazy_ops, &enable_rcu_lazy, 0444);
+
 /**
  * call_rcu_hurry() - Queue RCU callback for invocation after grace period, and
  * flush all lazy callbacks (including the new one) to the main ->cblist while
@@ -2792,7 +2815,8 @@ EXPORT_SYMBOL_GPL(call_rcu_hurry);
  */
 void call_rcu(struct rcu_head *head, rcu_callback_t func)
 {
-	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY));
+	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY) &&
+			  READ_ONCE(enable_rcu_lazy));
 }
 EXPORT_SYMBOL_GPL(call_rcu);
 
-- 
2.34.1

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Paul E. McKenney]

On Tue, Nov 21, 2023 at 08:53:04PM +0000, Qais Yousef wrote:
> To allow more flexible opt-in arrangements while still provide a single
> kernel for distros, provide a boot time parameter to enable lazy RCU.
> 
> Specify:
> 
> 	rcutree.enable_rcu_lazy
> 
> Which also requires
> 
> 	rcu_nocbs=all
> 
> at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
> parameter will be ignored if CONFIG_RCU_LAZY is not set.
> 
> With this change now lazy RCU is disabled by default if the boot
> parameter is not set even when CONFIG_RCU_LAZY is enabled.
> 
> Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
> ---
> 
> Makes sense to remove the CONFIG_RCU_LAZY now we have a boot time param?
> 
> We can make it a static key too if it *really* matters.
> 
> Thanks to Joel for helping initially in reviewing this patch which was intended
> originally for Android.
> 
> I got some requests to make this a runtime modifiable for init scripts; but
> Paul suggested there shall be dragons. So RO it is.

I must defer to the people using this, but my experience is that kernel
boot parameters work for some people but not others.  For example,
I tried making rcu_nocbs be the only way to say that all CPUs were
going to be offloaded, but popular demand resulted in my adding a
CONFIG_RCU_NOCB_CPU_DEFAULT_ALL.

If we cannot be sure that we know everyone using CONFIG_RCU_LAZY=y
and expecting full laziness, the safe approach is to make another
Kconfig option that defaults to off, but with either setting allowing
rcutree.enable_rcu_lazy to override at boot time.

If you can be sure that you know everyone using CONFIG_RCU_LAZY=y
is OK with this change, I must confess that I am curious as to how
you found them all.

Thoughts?

							Thanx, Paul

>  .../admin-guide/kernel-parameters.txt         |  5 ++++
>  kernel/rcu/tree.c                             | 26 ++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 65731b060e3f..2f0386a12aa7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5021,6 +5021,11 @@
>  			this kernel boot parameter, forcibly setting it
>  			to zero.
>  
> +	rcutree.enable_rcu_lazy= [KNL]
> +			To save power, batch RCU callbacks and flush after
> +			delay, memory pressure or callback list growing too
> +			big.
> +
>  	rcuscale.gp_async= [KNL]
>  			Measure performance of asynchronous
>  			grace-period primitives such as call_rcu().
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 3ac3c846105f..e0885905b3f6 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2718,7 +2718,30 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
>  	}
>  }
>  
> +static bool enable_rcu_lazy;
>  #ifdef CONFIG_RCU_LAZY
> +/* Enable lazy rcu at boot time */
> +static int param_set_rcu_lazy(const char *val, const struct kernel_param *kp)
> +{
> +	int ret;
> +
> +	/*
> +	 * Make sure a grace period has passed before and after flipping the
> +	 * switch.
> +	 */
> +	rcu_barrier();
> +	ret = param_set_bool(val, kp);
> +	rcu_barrier();
> +
> +	return ret;
> +}
> +static const struct kernel_param_ops rcu_lazy_ops = {
> +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> +	.set = param_set_rcu_lazy,
> +	.get = param_get_bool,
> +};
> +module_param_cb(enable_rcu_lazy, &rcu_lazy_ops, &enable_rcu_lazy, 0444);

OK, I will bite...

Given that this is to be set only at boot time, why not replace everything
from "#ifdef CONFIG_RCU_LAZY" to here with this?

module_param(enable_rcu_lazy, bool, 0444);

And then maybe also a __read_mostly on the definition of enable_rcu_lazy?

> +
>  /**
>   * call_rcu_hurry() - Queue RCU callback for invocation after grace period, and
>   * flush all lazy callbacks (including the new one) to the main ->cblist while
> @@ -2792,7 +2815,8 @@ EXPORT_SYMBOL_GPL(call_rcu_hurry);
>   */
>  void call_rcu(struct rcu_head *head, rcu_callback_t func)
>  {
> -	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY));
> +	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY) &&
> +			  READ_ONCE(enable_rcu_lazy));
>  }
>  EXPORT_SYMBOL_GPL(call_rcu);
>  
> -- 
> 2.34.1
> 

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Qais Yousef]

On 11/22/23 14:00, Paul E. McKenney wrote:
> On Tue, Nov 21, 2023 at 08:53:04PM +0000, Qais Yousef wrote:
> > To allow more flexible opt-in arrangements while still provide a single
> > kernel for distros, provide a boot time parameter to enable lazy RCU.
> > 
> > Specify:
> > 
> > 	rcutree.enable_rcu_lazy
> > 
> > Which also requires
> > 
> > 	rcu_nocbs=all
> > 
> > at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
> > parameter will be ignored if CONFIG_RCU_LAZY is not set.
> > 
> > With this change now lazy RCU is disabled by default if the boot
> > parameter is not set even when CONFIG_RCU_LAZY is enabled.
> > 
> > Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
> > ---
> > 
> > Makes sense to remove the CONFIG_RCU_LAZY now we have a boot time param?
> > 
> > We can make it a static key too if it *really* matters.
> > 
> > Thanks to Joel for helping initially in reviewing this patch which was intended
> > originally for Android.
> > 
> > I got some requests to make this a runtime modifiable for init scripts; but
> > Paul suggested there shall be dragons. So RO it is.
> 
> I must defer to the people using this, but my experience is that kernel
> boot parameters work for some people but not others.  For example,
> I tried making rcu_nocbs be the only way to say that all CPUs were
> going to be offloaded, but popular demand resulted in my adding a
> CONFIG_RCU_NOCB_CPU_DEFAULT_ALL.

Speak of pleasing a crowd.. There's always someone who wants something else :-)

I imagine the difficulty is in some environments it is easier to switch a sysfs
knob than add a new boot time parameter. And in the absence of a writable sysfs
node, I can imagine some folks think having a Kconfig to force a default at
compile time is the 2nd best compared to modifying their boot time parameters..

Either way; I'll follow what the crowd wants too :-)

> 
> If we cannot be sure that we know everyone using CONFIG_RCU_LAZY=y
> and expecting full laziness, the safe approach is to make another
> Kconfig option that defaults to off, but with either setting allowing
> rcutree.enable_rcu_lazy to override at boot time.
> 
> If you can be sure that you know everyone using CONFIG_RCU_LAZY=y
> is OK with this change, I must confess that I am curious as to how
> you found them all.

If you let it break and no one shouts..

/me hides

Jokes aside, all options work for me. I'll wait to hear from the other rcu
gurus what they'd like.

> 
> Thoughts?
> 
> 							Thanx, Paul
> 
> >  .../admin-guide/kernel-parameters.txt         |  5 ++++
> >  kernel/rcu/tree.c                             | 26 ++++++++++++++++++-
> >  2 files changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 65731b060e3f..2f0386a12aa7 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -5021,6 +5021,11 @@
> >  			this kernel boot parameter, forcibly setting it
> >  			to zero.
> >  
> > +	rcutree.enable_rcu_lazy= [KNL]
> > +			To save power, batch RCU callbacks and flush after
> > +			delay, memory pressure or callback list growing too
> > +			big.
> > +
> >  	rcuscale.gp_async= [KNL]
> >  			Measure performance of asynchronous
> >  			grace-period primitives such as call_rcu().
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 3ac3c846105f..e0885905b3f6 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2718,7 +2718,30 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> >  	}
> >  }
> >  
> > +static bool enable_rcu_lazy;
> >  #ifdef CONFIG_RCU_LAZY
> > +/* Enable lazy rcu at boot time */
> > +static int param_set_rcu_lazy(const char *val, const struct kernel_param *kp)
> > +{
> > +	int ret;
> > +
> > +	/*
> > +	 * Make sure a grace period has passed before and after flipping the
> > +	 * switch.
> > +	 */
> > +	rcu_barrier();
> > +	ret = param_set_bool(val, kp);
> > +	rcu_barrier();
> > +
> > +	return ret;
> > +}
> > +static const struct kernel_param_ops rcu_lazy_ops = {
> > +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> > +	.set = param_set_rcu_lazy,
> > +	.get = param_get_bool,
> > +};
> > +module_param_cb(enable_rcu_lazy, &rcu_lazy_ops, &enable_rcu_lazy, 0444);
> 
> OK, I will bite...
> 
> Given that this is to be set only at boot time, why not replace everything
> from "#ifdef CONFIG_RCU_LAZY" to here with this?
> 
> module_param(enable_rcu_lazy, bool, 0444);

No need for the rcu_barrier() then? Only reason why we use the _cb flavour

> And then maybe also a __read_mostly on the definition of enable_rcu_lazy?

+1

I think the READ_ONCE() was unnecessary too.


Thanks!

--
Qais Yousef

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Paul E. McKenney]

On Tue, Nov 21, 2023 at 09:44:15PM +0000, Qais Yousef wrote:
> On 11/22/23 14:00, Paul E. McKenney wrote:
> > On Tue, Nov 21, 2023 at 08:53:04PM +0000, Qais Yousef wrote:
> > > To allow more flexible opt-in arrangements while still provide a single
> > > kernel for distros, provide a boot time parameter to enable lazy RCU.
> > > 
> > > Specify:
> > > 
> > > 	rcutree.enable_rcu_lazy
> > > 
> > > Which also requires
> > > 
> > > 	rcu_nocbs=all
> > > 
> > > at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
> > > parameter will be ignored if CONFIG_RCU_LAZY is not set.
> > > 
> > > With this change now lazy RCU is disabled by default if the boot
> > > parameter is not set even when CONFIG_RCU_LAZY is enabled.
> > > 
> > > Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
> > > ---
> > > 
> > > Makes sense to remove the CONFIG_RCU_LAZY now we have a boot time param?
> > > 
> > > We can make it a static key too if it *really* matters.
> > > 
> > > Thanks to Joel for helping initially in reviewing this patch which was intended
> > > originally for Android.
> > > 
> > > I got some requests to make this a runtime modifiable for init scripts; but
> > > Paul suggested there shall be dragons. So RO it is.
> > 
> > I must defer to the people using this, but my experience is that kernel
> > boot parameters work for some people but not others.  For example,
> > I tried making rcu_nocbs be the only way to say that all CPUs were
> > going to be offloaded, but popular demand resulted in my adding a
> > CONFIG_RCU_NOCB_CPU_DEFAULT_ALL.
> 
> Speak of pleasing a crowd.. There's always someone who wants something else :-)
> 
> I imagine the difficulty is in some environments it is easier to switch a sysfs
> knob than add a new boot time parameter. And in the absence of a writable sysfs
> node, I can imagine some folks think having a Kconfig to force a default at
> compile time is the 2nd best compared to modifying their boot time parameters..
> 
> Either way; I'll follow what the crowd wants too :-)

Usually a wise choice.  ;-)

But I must defer to the people using it.

> > If we cannot be sure that we know everyone using CONFIG_RCU_LAZY=y
> > and expecting full laziness, the safe approach is to make another
> > Kconfig option that defaults to off, but with either setting allowing
> > rcutree.enable_rcu_lazy to override at boot time.
> > 
> > If you can be sure that you know everyone using CONFIG_RCU_LAZY=y
> > is OK with this change, I must confess that I am curious as to how
> > you found them all.
> 
> If you let it break and no one shouts..
> 
> /me hides

Indeed, sometimes you get lucky.  Sometimes.  ;-)

> Jokes aside, all options work for me. I'll wait to hear from the other rcu
> gurus what they'd like.

Fair enough!

> > Thoughts?
> > 
> > 							Thanx, Paul
> > 
> > >  .../admin-guide/kernel-parameters.txt         |  5 ++++
> > >  kernel/rcu/tree.c                             | 26 ++++++++++++++++++-
> > >  2 files changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > index 65731b060e3f..2f0386a12aa7 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -5021,6 +5021,11 @@
> > >  			this kernel boot parameter, forcibly setting it
> > >  			to zero.
> > >  
> > > +	rcutree.enable_rcu_lazy= [KNL]
> > > +			To save power, batch RCU callbacks and flush after
> > > +			delay, memory pressure or callback list growing too
> > > +			big.
> > > +
> > >  	rcuscale.gp_async= [KNL]
> > >  			Measure performance of asynchronous
> > >  			grace-period primitives such as call_rcu().
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 3ac3c846105f..e0885905b3f6 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -2718,7 +2718,30 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> > >  	}
> > >  }
> > >  
> > > +static bool enable_rcu_lazy;
> > >  #ifdef CONFIG_RCU_LAZY
> > > +/* Enable lazy rcu at boot time */
> > > +static int param_set_rcu_lazy(const char *val, const struct kernel_param *kp)
> > > +{
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Make sure a grace period has passed before and after flipping the
> > > +	 * switch.
> > > +	 */
> > > +	rcu_barrier();
> > > +	ret = param_set_bool(val, kp);
> > > +	rcu_barrier();
> > > +
> > > +	return ret;
> > > +}
> > > +static const struct kernel_param_ops rcu_lazy_ops = {
> > > +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> > > +	.set = param_set_rcu_lazy,
> > > +	.get = param_get_bool,
> > > +};
> > > +module_param_cb(enable_rcu_lazy, &rcu_lazy_ops, &enable_rcu_lazy, 0444);
> > 
> > OK, I will bite...
> > 
> > Given that this is to be set only at boot time, why not replace everything
> > from "#ifdef CONFIG_RCU_LAZY" to here with this?
> > 
> > module_param(enable_rcu_lazy, bool, 0444);
> 
> No need for the rcu_barrier() then? Only reason why we use the _cb flavour

The module_param() parameters are processed during early boot, before
the boot CPU has enabled interrupts.  In fact, before rcu_init()
is invoked, which is in turn long before the scheduler has started.
Calling rcu_barrier() that early is not so good for your kernel's
actuarial statistics.

I am guessing that the module_param_cb() processing happens somewhat
later in the kernel's lifetime.

> > And then maybe also a __read_mostly on the definition of enable_rcu_lazy?
> 
> +1
> 
> I think the READ_ONCE() was unnecessary too.

Good point, and yes, I did miss that one.

							Thanx, Paul

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Qais Yousef]

On 11/22/23 15:26, Paul E. McKenney wrote:

> > Either way; I'll follow what the crowd wants too :-)
> 
> Usually a wise choice.  ;-)
> 
> But I must defer to the people using it.

From Android PoV I'd like to be able to boot with default disabled and allow
people to opt-in. At least for the time being until we have confidence no one
is caught with surprise if this caused unexpected problems.

In the future it might default to on once it gets wider usage and testing.

So having a new Kconfig to DEFAULT_OFF sounds good to me to enable a compile
time switch to pick the default with a boot time to further control.

Which would be my plan for v2 unless I hear another suggestion in the coming
week (where I hope people would have had a chance to look and think about it).

> > No need for the rcu_barrier() then? Only reason why we use the _cb flavour
> 
> The module_param() parameters are processed during early boot, before
> the boot CPU has enabled interrupts.  In fact, before rcu_init()
> is invoked, which is in turn long before the scheduler has started.
> Calling rcu_barrier() that early is not so good for your kernel's
> actuarial statistics.

Ah, I missed that it is done that early.

> 
> I am guessing that the module_param_cb() processing happens somewhat
> later in the kernel's lifetime.


Thanks!

--
Qais Yousef

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Paul E. McKenney]

On Tue, Nov 21, 2023 at 10:15:56PM +0000, Qais Yousef wrote:
> On 11/22/23 15:26, Paul E. McKenney wrote:
> 
> > > Either way; I'll follow what the crowd wants too :-)
> > 
> > Usually a wise choice.  ;-)
> > 
> > But I must defer to the people using it.
> 
> >From Android PoV I'd like to be able to boot with default disabled and allow
> people to opt-in. At least for the time being until we have confidence no one
> is caught with surprise if this caused unexpected problems.
> 
> In the future it might default to on once it gets wider usage and testing.
> 
> So having a new Kconfig to DEFAULT_OFF sounds good to me to enable a compile
> time switch to pick the default with a boot time to further control.
> 
> Which would be my plan for v2 unless I hear another suggestion in the coming
> week (where I hope people would have had a chance to look and think about it).

Sounds good to me!  Silence will be interpreted as assent.  ;-)

							Thanx, Paul

> > > No need for the rcu_barrier() then? Only reason why we use the _cb flavour
> > 
> > The module_param() parameters are processed during early boot, before
> > the boot CPU has enabled interrupts.  In fact, before rcu_init()
> > is invoked, which is in turn long before the scheduler has started.
> > Calling rcu_barrier() that early is not so good for your kernel's
> > actuarial statistics.
> 
> Ah, I missed that it is done that early.
> 
> > 
> > I am guessing that the module_param_cb() processing happens somewhat
> > later in the kernel's lifetime.
> 
> 
> Thanks!
> 
> --
> Qais Yousef

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Frederic Weisbecker]

Le Thu, Nov 23, 2023 at 09:42:12AM -0800, Paul E. McKenney a écrit :
> On Tue, Nov 21, 2023 at 10:15:56PM +0000, Qais Yousef wrote:
> > On 11/22/23 15:26, Paul E. McKenney wrote:
> > 
> > > > Either way; I'll follow what the crowd wants too :-)
> > > 
> > > Usually a wise choice.  ;-)
> > > 
> > > But I must defer to the people using it.
> > 
> > >From Android PoV I'd like to be able to boot with default disabled and allow
> > people to opt-in. At least for the time being until we have confidence no one
> > is caught with surprise if this caused unexpected problems.
> > 
> > In the future it might default to on once it gets wider usage and testing.
> > 
> > So having a new Kconfig to DEFAULT_OFF sounds good to me to enable a compile
> > time switch to pick the default with a boot time to further control.
> > 
> > Which would be my plan for v2 unless I hear another suggestion in the coming
> > week (where I hope people would have had a chance to look and think about it).
> 
> Sounds good to me!  Silence will be interpreted as assent.  ;-)

Silence sounds good to me :-)

Thanks.

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Paul E. McKenney]

On Thu, Nov 23, 2023 at 07:04:02PM +0100, Frederic Weisbecker wrote:
> Le Thu, Nov 23, 2023 at 09:42:12AM -0800, Paul E. McKenney a écrit :
> > On Tue, Nov 21, 2023 at 10:15:56PM +0000, Qais Yousef wrote:
> > > On 11/22/23 15:26, Paul E. McKenney wrote:
> > > 
> > > > > Either way; I'll follow what the crowd wants too :-)
> > > > 
> > > > Usually a wise choice.  ;-)
> > > > 
> > > > But I must defer to the people using it.
> > > 
> > > >From Android PoV I'd like to be able to boot with default disabled and allow
> > > people to opt-in. At least for the time being until we have confidence no one
> > > is caught with surprise if this caused unexpected problems.
> > > 
> > > In the future it might default to on once it gets wider usage and testing.
> > > 
> > > So having a new Kconfig to DEFAULT_OFF sounds good to me to enable a compile
> > > time switch to pick the default with a boot time to further control.
> > > 
> > > Which would be my plan for v2 unless I hear another suggestion in the coming
> > > week (where I hope people would have had a chance to look and think about it).
> > 
> > Sounds good to me!  Silence will be interpreted as assent.  ;-)
> 
> Silence sounds good to me :-)

This reply intentionally left content-free to preserve the silence.  ;-)

							Thanx, Paul

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Andrea Righi]

On Tue, Nov 21, 2023 at 08:53:04PM +0000, Qais Yousef wrote:
> To allow more flexible opt-in arrangements while still provide a single
> kernel for distros, provide a boot time parameter to enable lazy RCU.
> 
> Specify:
> 
> 	rcutree.enable_rcu_lazy
> 
> Which also requires
> 
> 	rcu_nocbs=all
> 
> at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
> parameter will be ignored if CONFIG_RCU_LAZY is not set.
> 
> With this change now lazy RCU is disabled by default if the boot
> parameter is not set even when CONFIG_RCU_LAZY is enabled.

I'm wondering if we should make this enabled by default if
CONFIG_RCU_LAZY=y, so we don't break the previous behavior, and those
who want it disabled (despite having CONFIG_RCU_LAZY=y in their .config)
can add rcutree.enable_rcu_lazy=0 to the boot options.

Thanks for working on this!
-Andrea

> 
> Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
> ---
> 
> Makes sense to remove the CONFIG_RCU_LAZY now we have a boot time param?
> 
> We can make it a static key too if it *really* matters.
> 
> Thanks to Joel for helping initially in reviewing this patch which was intended
> originally for Android.
> 
> I got some requests to make this a runtime modifiable for init scripts; but
> Paul suggested there shall be dragons. So RO it is.
> 
> 
>  .../admin-guide/kernel-parameters.txt         |  5 ++++
>  kernel/rcu/tree.c                             | 26 ++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 65731b060e3f..2f0386a12aa7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5021,6 +5021,11 @@
>  			this kernel boot parameter, forcibly setting it
>  			to zero.
>  
> +	rcutree.enable_rcu_lazy= [KNL]
> +			To save power, batch RCU callbacks and flush after
> +			delay, memory pressure or callback list growing too
> +			big.
> +
>  	rcuscale.gp_async= [KNL]
>  			Measure performance of asynchronous
>  			grace-period primitives such as call_rcu().
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 3ac3c846105f..e0885905b3f6 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2718,7 +2718,30 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
>  	}
>  }
>  
> +static bool enable_rcu_lazy;
>  #ifdef CONFIG_RCU_LAZY
> +/* Enable lazy rcu at boot time */
> +static int param_set_rcu_lazy(const char *val, const struct kernel_param *kp)
> +{
> +	int ret;
> +
> +	/*
> +	 * Make sure a grace period has passed before and after flipping the
> +	 * switch.
> +	 */
> +	rcu_barrier();
> +	ret = param_set_bool(val, kp);
> +	rcu_barrier();
> +
> +	return ret;
> +}
> +static const struct kernel_param_ops rcu_lazy_ops = {
> +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> +	.set = param_set_rcu_lazy,
> +	.get = param_get_bool,
> +};
> +module_param_cb(enable_rcu_lazy, &rcu_lazy_ops, &enable_rcu_lazy, 0444);
> +
>  /**
>   * call_rcu_hurry() - Queue RCU callback for invocation after grace period, and
>   * flush all lazy callbacks (including the new one) to the main ->cblist while
> @@ -2792,7 +2815,8 @@ EXPORT_SYMBOL_GPL(call_rcu_hurry);
>   */
>  void call_rcu(struct rcu_head *head, rcu_callback_t func)
>  {
> -	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY));
> +	__call_rcu_common(head, func, IS_ENABLED(CONFIG_RCU_LAZY) &&
> +			  READ_ONCE(enable_rcu_lazy));
>  }
>  EXPORT_SYMBOL_GPL(call_rcu);
>  
> -- 
> 2.34.1

Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU

[Qais Yousef]

On 11/23/23 07:28, Andrea Righi wrote:
> On Tue, Nov 21, 2023 at 08:53:04PM +0000, Qais Yousef wrote:
> > To allow more flexible opt-in arrangements while still provide a single
> > kernel for distros, provide a boot time parameter to enable lazy RCU.
> > 
> > Specify:
> > 
> > 	rcutree.enable_rcu_lazy
> > 
> > Which also requires
> > 
> > 	rcu_nocbs=all
> > 
> > at boot time to enable lazy RCU assuming CONFIG_RCU_LAZY=y. The
> > parameter will be ignored if CONFIG_RCU_LAZY is not set.
> > 
> > With this change now lazy RCU is disabled by default if the boot
> > parameter is not set even when CONFIG_RCU_LAZY is enabled.
> 
> I'm wondering if we should make this enabled by default if
> CONFIG_RCU_LAZY=y, so we don't break the previous behavior, and those
> who want it disabled (despite having CONFIG_RCU_LAZY=y in their .config)
> can add rcutree.enable_rcu_lazy=0 to the boot options.
> 
> Thanks for working on this!

Sure, thanks for having a look!

Cheers

--
Qais Yousef