Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 5 Oct 2020 10:38:07 -0400
From:   joel@joelfernandes.org
To:     linux-kernel@vger.kernel.org
Cc:     neeraju@codeaurora.org, Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        "Paul E. McKenney" <paulmck@kernel.org>, rcu@vger.kernel.org,
        Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v2] rcu/tree: nocb: Avoid raising softirq when there are
 ready to execute CBs
Message-ID: <20201005143807.GA524504@google.com>
References: <20201005021132.146534-1-joel@joelfernandes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201005021132.146534-1-joel@joelfernandes.org>
Precedence: bulk

On Sun, Oct 04, 2020 at 10:11:32PM -0400, Joel Fernandes (Google) wrote:
> During testing, I see it is possible that rcu_pending() returns 1 when
> offloaded callbacks are ready to execute thus raising the RCU softirq.
> 
> However, softirq does not execute offloaded callbacks. They are executed in a
> kthread which is awakened independent of the softirq.
> 
> This commit therefore avoids raising the softirq in the first place. That's
> probably a good thing considering that the purpose of callback offloading is to
> reduce softirq activity.
> 
> Passed 30 minute tests of TREE01 through TREE09 each.
> 
> On TREE08, I notice that there is atmost 150us from when the softirq was
> NOT raised when ready cbs were present, to when the ready callbacks were
> invoked by the rcuop thread. This also further confirms that there is no
> need to raise the softirq for ready cbs in the first place.

Hi Paul,
You had asked me about whether removing this softirq invocation indirectly
slows down grace period progression.

This morning, I ran rcutorture.fwd_progress on TREE08 and I don't see any
difference in number of grace periods with/without this patch. Just want to
let you know.

Thanks,

 - Joel

> 
> Cc: neeraju@codeaurora.org
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> ---
> 
> v1->v2: Also cleaned up another test of the nocb configuration macro.
> 
>  kernel/rcu/tree.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index f78ee759af9c..2b1e1b21db92 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3652,13 +3652,13 @@ static int rcu_pending(int user)
>  		return 1;
>  
>  	/* Does this CPU have callbacks ready to invoke? */
> -	if (rcu_segcblist_ready_cbs(&rdp->cblist))
> +	if (!rcu_segcblist_is_offloaded(&rdp->cblist) &&
> +	    rcu_segcblist_ready_cbs(&rdp->cblist))
>  		return 1;
>  
>  	/* Has RCU gone idle with this CPU needing another grace period? */
>  	if (!gp_in_progress && rcu_segcblist_is_enabled(&rdp->cblist) &&
> -	    (!IS_ENABLED(CONFIG_RCU_NOCB_CPU) ||
> -	     !rcu_segcblist_is_offloaded(&rdp->cblist)) &&
> +	    (!rcu_segcblist_is_offloaded(&rdp->cblist)) &&
>  	    !rcu_segcblist_restempty(&rdp->cblist, RCU_NEXT_READY_TAIL))
>  		return 1;
>  
> -- 
> 2.28.0.806.g8561365e88-goog