linux-kernel - Re: [PATCH v2 18/27] rcu: Rename rcu_dynticks_in_eqs_since() into rcu_watching_changed

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: <xhsmhttj1h7xy.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Mon, 13 May 2024 20:40:09 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: Frederic Weisbecker <frederic@...nel.org>, "Paul E. McKenney"
 <paulmck@...nel.org>
Cc: rcu@...r.kernel.org, linux-kernel@...r.kernel.org, Peter Zijlstra
 <peterz@...radead.org>, Neeraj Upadhyay <quic_neeraju@...cinc.com>, Joel
 Fernandes <joel@...lfernandes.org>, Josh Triplett <josh@...htriplett.org>,
 Boqun Feng <boqun.feng@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Lai Jiangshan
 <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>
Subject: Re: [PATCH v2 18/27] rcu: Rename rcu_dynticks_in_eqs_since() into
 rcu_watching_changed_since()

On 08/05/24 12:59, Frederic Weisbecker wrote:
> Le Tue, May 07, 2024 at 10:14:08AM -0700, Paul E. McKenney a écrit :
>> On Tue, May 07, 2024 at 03:48:18PM +0200, Frederic Weisbecker wrote:
>> > Indeed in practice the function only checks a change. But semantically it really
>> > checks a trip to eqs because this function is only ever called after a failing
>> > call to rcu_dynticks_in_eqs().
>> >
>> > So not sure about that one rename. Paul?
>>
>> As you say, Valentin is technically correct.  Me, I am having a hard
>> time getting too excited one way or the other.  ;-)
>>
>> I suggest thinking in terms of rate-bounding the change.  If you do
>> change it, don't change it again for a few years.
>
> Makes sense!
>
>>
>> Either way, should comments be changed or added?
>>
>> Of course, the scientific way to evaluate this is to whose a couple
>> dozen people the old code and a couple dozen other people the new code,
>> and see if one group or the other has statistically significantly lower
>> levels of confusion.  I don't see how this is feasible, but it is the
>> (painfully) correct way.  On the other hand, it would have the beneficial
>> side effect of getting more people exposed to Linux-kernel-RCU internals.
>> Unfortunately, it might also have the additional side effect of making
>> them (more) annoyed at RCU.  ;-)
>
> Sounds good!
>
> I divided myself in two blank RCU subjects for a double blind study
> and locked those people up overnight with a paper containing both proposals.
>
> I opened the door five minutes ago and they both elected by mutual agreement
> rcu_watching_changed_since()! Also they are thirsty.
>
> Congratulations Valentin! :-)

:-)

Now, not that I like wasting everyone's time, but... I hadn't taken a step
back to realize the calling context implied this would always be used to
check an entry into EQS, per the waiting loop structures. With this in
mind, how about the following? 


/**
 * rcu_watching_stopped_since() - Has RCU stopped watching a given CPU since
 * the specified @snap?
 *
 * @rdp: The rcu_data corresponding to the CPU for which to check EQS.
 * @snap: rcu_watching snapshot taken when the CPU wasn't in an EQS.
 *
 * Returns true if the CPU corresponding to @rcu_data has spent some time in an
 * extended quiescent state since @snap. Note that this doesn't check if it
 * /still/ is in an EQS, just that it went through one since @snap.
 *
 * This is meant to be used in a loop waiting for a CPU to go through an EQS.
 */
static bool rcu_watching_stopped_since(struct rcu_data *rdp, int snap)
{
	if (WARN_ON_ONCE(rcu_watching_in_eqs(snap)))
		return true;

	return snap != rcu_dynticks_snap(rdp->cpu);
}