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;
Sender: neeraju=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 2380AC433CB
Subject: Re: [PATCH v2] rcu/tree: nocb: Avoid raising softirq when there are
 ready to execute CBs
To:     paulmck@kernel.org,
        "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>
References: <20201005021132.146534-1-joel@joelfernandes.org>
 <20201007223438.GU29330@paulmck-ThinkPad-P72>
From:   Neeraj Upadhyay <neeraju@codeaurora.org>
Message-ID: <7fb89900-18e3-f438-ec31-2bce21c02afe@codeaurora.org>
Date:   Thu, 8 Oct 2020 12:49:40 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.1
MIME-Version: 1.0
In-Reply-To: <20201007223438.GU29330@paulmck-ThinkPad-P72>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 10/8/2020 4:04 AM, Paul E. McKenney wrote:
> 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.
>>
>> Cc: neeraju@codeaurora.org
>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> Looks good, applied, thank you!  I reworked things a bit based on
> previous patches and to more precisely capture why this patch does
> not cause additional problems.  Please let me know if I messed
> anything up.
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit 33847a34a2d261354a79b4a24d9d37222e8ec888
> Author: Joel Fernandes (Google) <joel@joelfernandes.org>
> Date:   Wed Oct 7 13:50:36 2020 -0700
> 
>      rcu/tree: nocb: Avoid raising softirq for offloaded ready-to-execute CBs
>      
>      Testing showed that rcu_pending() can return 1 when offloaded callbacks
>      are ready to execute.  This invokes RCU core processing, for example,
>      by raising RCU_SOFTIRQ, eventually resulting in a call to rcu_core().
>      However, rcu_core() explicitly avoids in any way manipulating offloaded
>      callbacks, which are instead handled by the rcuog and rcuoc kthreads,
>      which work independently of rcu_core().
>      
>      One exception to this independence is that rcu_core() invokes
>      do_nocb_deferred_wakeup(), however, rcu_pending() also checks
>      rcu_nocb_need_deferred_wakeup() in order to correctly handle this case,
>      invoking rcu_core() when needed.
>      
>      This commit therefore avoids needlessly invoking RCU core processing
>      by checking rcu_segcblist_ready_cbs() only on non-offloaded CPUs.
>      This reduces overhead, for example, by reducing softirq activity.
>      
>      This change passed 30 minute tests of TREE01 through TREE09 each.
>      
>      On TREE08, there is at most 150us from the time that rcu_pending() chose
>      not to invoke RCU core processing to the time when the ready callbacks
>      were invoked by the rcuoc kthread.  This provides further evidence that
>      there is no need to invoke rcu_core() for offloaded callbacks that are
>      ready to invoke.
>      
>      Cc: Neeraj Upadhyay <neeraju@codeaurora.org>
>      Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>      Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> 

Reviewed-by: Neeraj Upadhyay <neeraju@codeaurora.org>


Thanks
Neeraj

> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 85e3f29..bfd38f2 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3716,7 +3716,8 @@ 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? */
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member of the Code Aurora Forum, hosted by The Linux Foundation