linux-kernel - Re: [PATCH 1/5] rcu/exp: Protect against early QS report

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] [day] [month] [year] [list]

Message-ID: <Z9dPiCVpxaX1aGEi@pavilion.home>
Date: Sun, 16 Mar 2025 23:24:08 +0100
From: Frederic Weisbecker <frederic@...nel.org>
To: Joel Fernandes <joelagnelf@...dia.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Boqun Feng <boqun.feng@...il.com>,
	Neeraj Upadhyay <neeraj.upadhyay@....com>,
	"Paul E . McKenney" <paulmck@...nel.org>,
	Uladzislau Rezki <urezki@...il.com>,
	Zqiang <qiang.zhang1211@...il.com>, rcu <rcu@...r.kernel.org>
Subject: Re: [PATCH 1/5] rcu/exp: Protect against early QS report

Le Sun, Mar 16, 2025 at 10:23:45AM -0400, Joel Fernandes a écrit :
> >> A small side effect of this patch could be:
> >>
> >> In the existing code, if between the sync_exp_reset_tree() and the
> >> __sync_rcu_exp_select_node_cpus(), if a pre-existing reader unblocked and
> >> completed, then I think it wouldn't be responsible for blocking the GP
> >> anymore.
> > Hmm, I don't see how that changes after this patch.
> > 
> >> Where as with this patch, it would not get a chance to be removed from the
> >> blocked list because it would have to wait on the rnp lock, which after this
> >> patch would now be held across the setting of exp_mask and exp_tasks?
> > So that's sync_exp_reset_tree(). I'm a bit confused. An unblocking task
> > contend on rnp lock in any case. But after this patch it is still going
> > to remove itself from the blocking task once the rnp lock is released by
> > sync_exp_reset_tree().
> > 
> > What am I missing?
> You are probably not missing anything and I'm the one missing something.
> 
> But I was thinking:
> 
> In in the original code, in __sync_rcu_exp_select_node_cpus() if
> rcu_preempt_has_tasks() returns FALSE because of the finer grained locking, then
> there is a chance for the GP to conclude sooner,

Why do you think it's finer grained locking?
 
> On the other hand, after the patch because the unblocking task had to wait (on
> the lock) to remove itself from the blocked task list, the GP may conclude later
> than usual. This is just an intuitive guess.
> 
> Because this is an expedited GP, my intuition is to unblock + reader unlock and
> get out of the way ASAP than hoping that it will get access to the lock before
> any IPIs go out or quiescent state reports/checks happen which are required to
> conclude the GP
> 
> Its just a theory and you're right, if it acquires the lock soon enough and gets
> out of the way, then it doesn't matter either way.

I think I understand where the confusion is. A task that is preempted within an
RCU read side section _always_ adds itself to the rnp's list of blocked tasks
(&rnp->blkd_tasks). The only thing that changes with expedited GPs is that
rnp->exp_tasks may or may not be updated on the way. But rnp->exp_tasks is only
a pointer to an arbitrary element within the rnp->blkd_tasks list.

This means that an unblocking task must always delete itself from
rnp->blkd_tasks, and possibly update rnp->exp_tasks along the way.

Both the add and the delete happen with rnp locked.

Therefore a task unblocking before __sync_rcu_exp_select_node_cpus()
can make __sync_rcu_exp_select_node_cpus() contend on rnp locking.

But this patch doesn't change the behaviour in this regard.

Thanks.


> 
> Thanks!
> 
>  - Joel
> 
> 
>