linux-kernel - Re: [PATCH -rcu -next 4/7] rcu: Remove redundant check for irq state during unlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6174e60d-6973-4307-960f-27f31657b12f@nvidia.com>
Date: Fri, 11 Jul 2025 12:39:34 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: paulmck@...nel.org
Cc: linux-kernel@...r.kernel.org, Frederic Weisbecker <frederic@...nel.org>,
 Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
 Josh Triplett <josh@...htriplett.org>, Boqun Feng <boqun.feng@...il.com>,
 Uladzislau Rezki <urezki@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Lai Jiangshan <jiangshanlai@...il.com>, Zqiang <qiang.zhang@...ux.dev>,
 rcu@...r.kernel.org
Subject: Re: [PATCH -rcu -next 4/7] rcu: Remove redundant check for irq state
 during unlock



On 7/11/2025 12:30 PM, Joel Fernandes wrote:
> 
> 
> On 7/10/2025 8:00 PM, Paul E. McKenney wrote:
>> On Tue, Jul 08, 2025 at 10:22:21AM -0400, Joel Fernandes wrote:
>>> The check for irqs_were_disabled is redundant in
>>> rcu_unlock_needs_exp_handling() as the caller already checks for this.
>>> This includes the boost case as well. Just remove the redundant check.
>>
>> But in the very last "if" statement in the context of the last hunk of
>> this patch, isn't it instead checking for !irqs_were_disabled?
>>
>> Or is there something that I am missing that makes this work out OK?
> 
> You are right, after going over all the cases I introduced a behavioral change.
> 
> Say, irqs_were_disabled was false. And say we are RCU-boosted. needs_exp might
> return true now, but previously it was returning false. Further say, we are not
> in hardirq.
> 
> New code will raise softirq, old code would go to the ELSE and just set:
>                 set_tsk_need_resched(current);
> set_preempt_need_resched();
> 
> But preempt_bh_were_disabled that's why we're here.
> 
> So we need to only set resched and not raise softirq, because the preempt enable
> would reschedule.
> 
> Sorry I missed this, but unless this behavior is correct or needs further work,
> lets drop this patch. Thanks and kudos on the catch!
Btw, Neeraj, the related patch 3/7 should still be good for applying (which has
Paul's Review tag).

Thanks.