Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 Jan 2023 11:27:24 -0800
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     "Joel Fernandes (Google)" <joel@joelfernandes.org>
Cc:     linux-kernel@vger.kernel.org,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>,
        fweisbec@gmail.com, urezki@gmail.com
Subject: Re: [PATCH v2 rcu/dev 1/2] rcu: Track laziness during boot and
 suspend
Message-ID: <20230112192724.GA3610129@paulmck-ThinkPad-P17-Gen-1>
Reply-To: paulmck@kernel.org
References: <20230112005223.2329802-1-joel@joelfernandes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230112005223.2329802-1-joel@joelfernandes.org>
Precedence: bulk

On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote:
> During boot and suspend/resume, it is desired to prevent RCU laziness from
> effecting performance and in some cases failures like with suspend.
> 
> Track whether RCU laziness is to be ignored or not, in kernels with
> CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however
> since Android currently expedites synchronous_rcu() always, we cannot rely on
> that. The next patch ignores laziness hints based on this tracking.
> 
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> Paul, could we take this and the next one for 6.2 -rc cycle?
> 
> I also booted debian Linux and verified the flag is reset correctly after boot
> completes. Thanks.

After going back and forth a bit, I took these two as-is (aside from
the usual commit-log wordsmithing).

At some point, the state-change-notification from things like boot,
suspend/resume, sysrq-t, and panic() should probably be refactored.
But I do not believe that we are yet at that point, and there is not
much point in half-refactorings in cases such as this one.

I added the Fixes tags, and, if testing goes well, I plan to submit
them into the upcoming merge window.  I have no reason to believe that
anyone is hitting this, it is only a few weeks away anyhow, and a good
chunk of that would be consumed by testing and review.

And if someone does hit it, we do have your fixes to send to them, so
thank you for that!

(These won't become visible on -rcu until I complete today's rebase,
in case you were wondering.)

						Thanx, Paul

>  kernel/rcu/rcu.h    |  6 ++++++
>  kernel/rcu/tree.c   |  2 ++
>  kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  5 files changed, 55 insertions(+), 1 deletion(-)
>  create mode 100644 cc_list
>  create mode 100644 to_list
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index 5c8013f7085f..115616ac3bfa 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -449,14 +449,20 @@ do {									\
>  /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */
>  static inline bool rcu_gp_is_normal(void) { return true; }
>  static inline bool rcu_gp_is_expedited(void) { return false; }
> +static inline bool rcu_async_should_hurry(void) { return false; }
>  static inline void rcu_expedite_gp(void) { }
>  static inline void rcu_unexpedite_gp(void) { }
> +static inline void rcu_async_hurry(void) { }
> +static inline void rcu_async_relax(void) { }
>  static inline void rcu_request_urgent_qs_task(struct task_struct *t) { }
>  #else /* #ifdef CONFIG_TINY_RCU */
>  bool rcu_gp_is_normal(void);     /* Internal RCU use. */
>  bool rcu_gp_is_expedited(void);  /* Internal RCU use. */
> +bool rcu_async_should_hurry(void);  /* Internal RCU use. */
>  void rcu_expedite_gp(void);
>  void rcu_unexpedite_gp(void);
> +void rcu_async_hurry(void);
> +void rcu_async_relax(void);
>  void rcupdate_announce_bootup_oddness(void);
>  #ifdef CONFIG_TASKS_RCU_GENERIC
>  void show_rcu_tasks_gp_kthreads(void);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 63545d79da51..78b2e999c904 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self,
>  	switch (action) {
>  	case PM_HIBERNATION_PREPARE:
>  	case PM_SUSPEND_PREPARE:
> +		rcu_async_hurry();
>  		rcu_expedite_gp();
>  		break;
>  	case PM_POST_HIBERNATION:
>  	case PM_POST_SUSPEND:
>  		rcu_unexpedite_gp();
> +		rcu_async_relax();
>  		break;
>  	default:
>  		break;
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 3893022f8ed8..19bf6fa3ee6a 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void)
>  }
>  EXPORT_SYMBOL_GPL(rcu_gp_is_normal);
>  
> -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1);
> +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1);
> +/*
> + * Should call_rcu() callbacks be processed with urgency or are
> + * they OK being executed with arbitrary delays?
> + */
> +bool rcu_async_should_hurry(void)
> +{
> +	return !IS_ENABLED(CONFIG_RCU_LAZY) ||
> +	       atomic_read(&rcu_async_hurry_nesting);
> +}
> +EXPORT_SYMBOL_GPL(rcu_async_should_hurry);
> +
> +/**
> + * rcu_async_hurry - Make future async RCU callbacks not lazy.
> + *
> + * After a call to this function, future calls to call_rcu()
> + * will be processed in a timely fashion.
> + */
> +void rcu_async_hurry(void)
> +{
> +	if (IS_ENABLED(CONFIG_RCU_LAZY))
> +		atomic_inc(&rcu_async_hurry_nesting);
> +}
> +EXPORT_SYMBOL_GPL(rcu_async_hurry);
>  
> +/**
> + * rcu_async_relax - Make future async RCU callbacks lazy.
> + *
> + * After a call to this function, future calls to call_rcu()
> + * will be processed in a lazy fashion.
> + */
> +void rcu_async_relax(void)
> +{
> +	if (IS_ENABLED(CONFIG_RCU_LAZY))
> +		atomic_dec(&rcu_async_hurry_nesting);
> +}
> +EXPORT_SYMBOL_GPL(rcu_async_relax);
> +
> +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1);
>  /*
>   * Should normal grace-period primitives be expedited?  Intended for
>   * use within RCU.  Note that this function takes the rcu_expedited
> @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly;
>  void rcu_end_inkernel_boot(void)
>  {
>  	rcu_unexpedite_gp();
> +	rcu_async_relax();
>  	if (rcu_normal_after_boot)
>  		WRITE_ONCE(rcu_normal, 1);
>  	rcu_boot_ended = true;
> -- 
> 2.39.0.314.g84b9a713c41-goog