Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Thu, 23 Nov 2023 07:28:50 +0100
From:   Andrea Righi <andrea.righi@canonical.com>
To:     Qais Yousef <qyousef@layalina.io>
Cc:     "Paul E. McKenney" <paulmck@kernel.org>,
        Joel Fernandes <joel@joelfernandes.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Zqiang <qiang.zhang1211@gmail.com>,
        John Stultz <jstultz@google.com>, linux-kernel@vger.kernel.org,
        rcu@vger.kernel.org
Subject: Re: [PATCH] rcu: Provide a boot time parameter to enable lazy RCU
Message-ID: <ZV7xIjSq1l1q7gRI@gpd>
References: <20231121205304.315146-1-qyousef@layalina.io>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231121205304.315146-1-qyousef@layalina.io>
Precedence: bulk

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