Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Thu, 2 Nov 2023 19:54:17 -0700
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Zqiang <qiang.zhang1211@gmail.com>
Cc:     frederic@kernel.org, joel@joelfernandes.org, boqun.feng@gmail.com,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rcu: Force quiescent states only for ongoing grace period
Message-ID: <9f15280f-45a3-4e3f-832d-cea5c77eb449@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20231101033507.21651-1-qiang.zhang1211@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231101033507.21651-1-qiang.zhang1211@gmail.com>
Precedence: bulk

On Wed, Nov 01, 2023 at 11:35:07AM +0800, Zqiang wrote:
> Currently, when running the rcutorture testing, if the fqs_task
> kthread was created, the periodic fqs operations will be performed,
> regardless of whether the grace-period is ongoing. however, if there
> is no ongoing grace-period, invoke the rcu_force_quiescent_state() has
> no effect, because when the new grace-period starting, will clear all
> flags int rcu_state.gp_flags in rcu_gp_init(). this commit therefore add
> rcu_gp_in_progress() check in rcu_force_quiescent_state(), if there is
> no ongoing grace-period, return directly.
> 
> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>

Nice optimization, but one question below.

						Thanx, Paul

> ---
>  kernel/rcu/tree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index aa4c808978b8..5b4279ef66da 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2338,6 +2338,8 @@ void rcu_force_quiescent_state(void)
>  	struct rcu_node *rnp;
>  	struct rcu_node *rnp_old = NULL;
>  
> +	if (!rcu_gp_in_progress())
> +		return;

Suppose that the grace period that was in progress above ends right
at this point in the code.  We will still do the useless grace
forcing of quiescent states.  Which means that this code path
does need to be tested.

So, when you run rcutorture with this change, how often has the
grace period ended before this function returns?  If that happens
reasonably often, say more than once per minute or so, then this
works nicely.  If not, we do need to do something to make sure
that that code path gets tested.

Thoughts?

>  	/* Funnel through hierarchy to reduce memory contention. */
>  	rnp = raw_cpu_read(rcu_data.mynode);
>  	for (; rnp != NULL; rnp = rnp->parent) {
> -- 
> 2.17.1
>