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;
Message-ID: <de5bac34-7369-4764-bb56-86d3f8de32fa@quicinc.com>
Date:   Wed, 22 Jun 2022 17:56:14 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] rcu: Add exp QS check in rcu_exp_handler() for
 no-preemptible expedited RCU
Content-Language: en-US
To:     Zqiang <qiang1.zhang@intel.com>, <paulmck@kernel.org>,
        <frederic@kernel.org>
CC:     <rcu@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20220622103549.2840087-1-qiang1.zhang@intel.com>
From:   Neeraj Upadhyay <quic_neeraju@quicinc.com>
In-Reply-To: <20220622103549.2840087-1-qiang1.zhang@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/22/2022 4:05 PM, Zqiang wrote:
> In CONFIG_PREEMPT=n and CONFIG_PREEMPT_COUNT=y kernel, after a exp
> grace period begins, if detected current CPU enters idle in
> rcu_exp_handler() IPI handler, will immediately report the exp QS of the
> current cpu, at this time, maybe not being in an RCU read-side critical
> section, but need wait until rcu-softirq or sched-clock irq or sched-switch
> occurs on current CPU to check and report exp QS.
> 
> This commit add a exp QS check in rcu_exp_handler(), when not being
> in an RCU read-side critical section, report exp QS earlier.
> 
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---

I am a bit unsure on this change. For non-preempt kernel, technically 
(and by design) context switch points are considered RCU read side 
critical section end? This change, changes that behavior  between 
CONFIG_PREEMPT_COUNT=y and CONFIG_PREEMPT_COUNT=n case, for 
non-preemptive  kernels.

However, I could be missing something here, and would wait for Paul's 
review.


Thanks
Neeraj


>   kernel/rcu/tree_exp.h | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
> index be667583a554..34f08267410f 100644
> --- a/kernel/rcu/tree_exp.h
> +++ b/kernel/rcu/tree_exp.h
> @@ -828,11 +828,14 @@ static void rcu_exp_handler(void *unused)
>   {
>   	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>   	struct rcu_node *rnp = rdp->mynode;
> +	bool preempt_bh_disabled =
> +				!!(preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK));
>   
>   	if (!(READ_ONCE(rnp->expmask) & rdp->grpmask) ||
>   	    __this_cpu_read(rcu_data.cpu_no_qs.b.exp))
>   		return;
> -	if (rcu_is_cpu_rrupt_from_idle()) {
> +	if (rcu_is_cpu_rrupt_from_idle() ||
> +			(IS_ENABLED(CONFIG_PREEMPT_COUNT) && !preempt_bh_disabled)) {


>   		rcu_report_exp_rdp(this_cpu_ptr(&rcu_data));
>   		return;
>   	}