From: "Paul E. McKenney" <paulmck@kernel.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Jonas Oberhauser <jonas.oberhauser@huaweicloud.com>,
Andrea Parri <parri.andrea@gmail.com>,
Jonas Oberhauser <jonas.oberhauser@huawei.com>,
Peter Zijlstra <peterz@infradead.org>, will <will@kernel.org>,
"boqun.feng" <boqun.feng@gmail.com>, npiggin <npiggin@gmail.com>,
dhowells <dhowells@redhat.com>, "j.alglave" <j.alglave@ucl.ac.uk>,
"luc.maranget" <luc.maranget@inria.fr>, akiyks <akiyks@gmail.com>,
dlustig <dlustig@nvidia.com>, joel <joel@joelfernandes.org>,
urezki <urezki@gmail.com>,
quic_neeraju <quic_neeraju@quicinc.com>,
frederic <frederic@kernel.org>,
Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: Internal vs. external barriers (was: Re: Interesting LKMM litmus test)
Date: Sat, 21 Jan 2023 12:10:26 -0800 [thread overview]
Message-ID: <20230121201026.GH2948950@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <Y8xDieO/iYOk9Ty1@rowland.harvard.edu>
On Sat, Jan 21, 2023 at 02:56:57PM -0500, Alan Stern wrote:
> On Sat, Jan 21, 2023 at 10:40:32AM -0800, Paul E. McKenney wrote:
> > On Sat, Jan 21, 2023 at 12:36:26PM -0500, Alan Stern wrote:
> > > On Fri, Jan 20, 2023 at 10:41:14PM +0100, Jonas Oberhauser wrote:
> > > >
> > > >
> > > > On 1/20/2023 5:18 PM, Alan Stern wrote:
> > > > > On Fri, Jan 20, 2023 at 11:13:00AM +0100, Jonas Oberhauser wrote:
> > > > > > Perhaps we could say that reading an index without using it later is
> > > > > > forbidden?
> > > > > >
> > > > > > flag ~empty [Srcu-lock];data;rf;[~ domain(data;[Srcu-unlock])] as
> > > > > > thrown-srcu-cookie-on-floor
> > > > > We already flag locks that don't have a matching unlock.
> > > >
> > > > Of course, but as you know this is completely orthogonal.
> > >
> > > Yeah, okay. It doesn't hurt to add this check, but the check isn't
> > > complete. For example, it won't catch the invalid usage here:
> > >
> > > P0(srcu_struct *ss)
> > > {
> > > int r1, r2;
> > >
> > > r1 = srcu_read_lock(ss);
> > > srcu_read_unlock(&ss, r1);
> > > r2 = srcu_read_lock(ss);
> > > srcu_read_unlock(&ss, r2);
> > > }
> > >
> > > exists (~0:r1=0:r2)
> > >
> > > On the other hand, how often will people make this sort of mistake in
> > > their litmus tests? My guess is not very.
> >
> > I must be blind this morning. I see a well-formed pair of back-to-back
> > SRCU read-side critical sections. A rather useless pair, given that
> > both are empty,
>
> And there are no synchronize_srcu() calls.
Agreed, an additional level of uselessness, though not invalidity. After
all, the more advantageous SRCU use cases execute lots of srcu_read_lock()
and srcu_read_unlock() calls and very few synchronize_srcu() calls.
> > but valid nonetheless.
> >
> > Or is the bug the use of 0:r1 and 0:r2 in the "exists" clause? If so,
> > then I agree that this is not at all a high-priority bug to flag.
>
> Yes, that is the bug. The patched version of LKMM and the
> implementation you described say the exist clause will never be
> satisfied, the current version of LKMM says it will always be
> satisfied, and the theoretical model for SRCU says it will sometimes
> be satisfied -- which is the answer we want.
Got it, thank you.
> > > > Can you briefly explain how the operational model you have in mind for
> > > > srcu's up and down allows x==1 (and y==0 and idx1==idx2) in the example I
> > > > sent before (copied with minor edit below for convenience)?
> > > >
> > > > P0{
> > > > idx1 = srcu_down(&ss);
> > > > store_rel(p1, true);
> > > >
> > > >
> > > > shared cs
> > > >
> > > > R x == 1
> > > >
> > > > while (! load_acq(p2));
> > > > R idx2 == idx1 // for some reason, we got lucky!
> > > > srcu_up(&ss,idx1);
> > > > }
> > > >
> > > > P1{
> > > > idx2 = srcu_down(&ss);
> > > > store_rel(p2, true);
> > > >
> > > > shared cs
> > > >
> > > > R y == 0
> > > >
> > > > while (! load_acq(p1));
> > > > srcu_up(&ss,idx2);
> > > > }
> > > >
> > > > P2 {
> > > > W y = 1
> > > > srcu_sync(&ss);
> > > > W x = 1
> > > > }
> > > >
> > > >
> > > > I can imagine models that allow this but they aren't pretty. Maybe you have
> > > > a better operational model?
> > >
> > > The operational model is not very detailed as far as SRCU is concerned.
> > > It merely says that synchronize_srcu() executing on CPU C waits until:
> > >
> > > All writes received by C prior to the start of the function have
> > > propagated to all CPUs (call this time t1). This could be
> > > arranged by having synchronize_srcu() start with an smp_mb().
> > >
> > > For every srcu_down_read() that executed prior to t1, the
> > > matching srcu_up_read() has finished and all writes received
> > > by the unlocking CPU prior to the unlock have propagated to all
> > > CPUs. This could be arranged by having the srcu_up_read()
> > > call include a release write which has been received by C and
> > > having synchronize_srcu() end with an smp_mb().
> >
> > Agreed. It took me a few reads to see that this prohibited later writes
> > by other CPUs affecting reads in the prior critical section, but the "all
> > writes received by the unlocking CPU" does seem to me to prohibit this.
> >
> > > The operational model doesn't specify exactly how synchronize_srcu()
> > > manages to do these things, though.
> >
> > Which is a good thing, given the wide variety of possible implementations.
> >
> > > Oh yes, it also says that the value returned by srcu_down_read() is an
> > > unpredictable int. This differs from the code in the patched herd
> > > model, which says that the value will always be 0.
> >
> > As noted earlier, I believe that this is fine. If significant problems
> > arise, then we might need to do something. However, there is some
> > cost to complexity, so we should avoid getting too speculative about
> > possible probems.
> >
> > > Anyway, the operational model says the litmus test can succeed as
> > > follows:
> > >
> > > P0 P1 P2
> > > --------------------- ---------------------- -------------------------
> > > Widx2=srcu_down_read()
> > > Wrel p2=1
> > > Ry=0
> > > Wy=1
> > > synchronize_srcu() starts
> > > ... idx2, p2, and y propagate to all CPUs ...
> > > Time t1
> > > Widx1=srcu_down_read()
> > > Wrel p1=1
> > > ,,, idx1 and p1 propagate to all CPUs ...
> > > Racq p1=1
> > > srcu_up_read(idx2)
> > > synchronize_srcu() ends
> > > Wx=1
> > > Rx=1
> > > Racq p2=1
> > > Ridx2=idx1
> > > srcu_up_read(idx1)
> > >
> > > (The final equality in P0 is allowed because idx1 and idx2 are both
> > > random numbers, so they might be equal.)
> >
> > This all makes sense to me.
> >
> > > Incidentally, it's worth pointing out that the algorithm Paul described
> > > will forbid this litmus test even if you remove the while loop and the
> > > read of idx2 from P0.
> >
> > Given that the values returned by those two srcu_down_read() calls must
> > be the same, then, yes, the current Linux-kernel Tree RCU implementation
> > would forbid this.
> >
> > On the other hand, if the two indexes differ, then P2's synchronize_srcu()
> > can see that there are no really old readers on !Widx2, then flip
> > the index. This would mean that P0's Widx1 would be equal to !Widx2,
> > which has already been waited on. Then P2's synchronize_srcu() can
> > return as soon as it sees P1's srcu_up_read().
>
> Sorry, what I said may not have been clear. I meant that even if you
> remove the while loop and read of idx2 from P0, your algorithm will
> still not allow idx1 = idx2 provided everything else is as written.
If synchronize_srcu() has flipped ->srcu_idx by the time that P0's
srcu_down_read() executes, agreed. Otherwise, Widx1 and Widx2 might
well be equal.
> > > If you don't pass the value to exactly one srcu_up_read() call,
> > > you void the SRCU warranty. In addition, if you do anything
> > > else with the value that might affect the outcome of the litmus
> > > test, you incur the risk that herd7 might compute an incorrect
> > > result [as in the litmus test I gave near the start of this
> > > email].
> > >
> > > Merely storing the value in a shared variable which then doesn't get
> > > used or is used only for something inconsequential would not cause any
> > > problems.
> >
> > That is consistent with my understanding, but please let me try again
> > in list form:
>
> ...
>
> > 4. If a value returned from a given srcu_read_lock() is passed to
> > exactly one srcu_read_unlock(), and then that value is later
> > manipulated, that is bad practice (exactly what are you trying
> > to accomplish by so doing?), but SRCU won't know the difference.
> >
> > In particular, the Linux-kernel SRCU implementation doesn't know
> > about the herd7 "exists" clause, but kudos to Jonas for casting
> > his conceptual net widely indeed!
>
> In addition, herd7 might give an answer different from what would
> actually happen in the kernel, depending on what the manipulation does.
True, given that the kernel's srcu_read_unlock() can return a
non-zero value.
> Yes, that is more or less what I was trying to express.
Sounds good!
Thanx, Paul
next prev parent reply other threads:[~2023-01-21 20:10 UTC|newest]
Thread overview: 161+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20220921173109.GA1214281@paulmck-ThinkPad-P17-Gen-1>
[not found] ` <YytfFiMT2Xsdwowf@rowland.harvard.edu>
[not found] ` <YywXuzZ/922LHfjI@hirez.programming.kicks-ass.net>
[not found] ` <114ECED5-FED1-4361-94F7-8D9BC02449B7>
[not found] ` <YzSAnclenTz7KQyt@rowland.harvard.edu>
[not found] ` <f763bd5ff835458d8750b61da47fe316@huawei.com>
2023-01-03 18:56 ` Internal vs. external barriers (was: Re: Interesting LKMM litmus test) Alan Stern
2023-01-04 15:37 ` Andrea Parri
2023-01-04 20:58 ` Alan Stern
[not found] ` <ee186bc17a5e48298a5373f688496dce@huawei.com>
2023-01-05 17:32 ` Paul E. McKenney
[not found] ` <bea712c82e6346f8973399a5711ff78a@huawei.com>
2023-01-11 15:06 ` Alan Stern
[not found] ` <768ffe7edc7f4ddfacd5b0a8e844ed84@huawei.com>
2023-01-11 17:01 ` Alan Stern
[not found] ` <07579baee4b84532a76ea8b0b33052bb@huawei.com>
2023-01-12 21:57 ` Paul E. McKenney
2023-01-13 16:38 ` Alan Stern
2023-01-13 19:54 ` Paul E. McKenney
[not found] ` <06a8aef7eb8d46bca34521a80880dae3@huawei.com>
2023-01-14 17:42 ` Paul E. McKenney
[not found] ` <e51c82a113484b6bb62354a49376f248@huawei.com>
2023-01-14 16:42 ` Alan Stern
2023-01-17 17:48 ` Jonas Oberhauser
2023-01-17 21:19 ` Alan Stern
2023-01-18 11:25 ` Jonas Oberhauser
2023-01-19 2:28 ` Alan Stern
2023-01-19 11:22 ` Jonas Oberhauser
2023-01-19 16:41 ` Alan Stern
2023-01-19 18:43 ` Paul E. McKenney
2023-01-23 16:16 ` Jonas Oberhauser
2023-01-23 19:58 ` Alan Stern
2023-01-23 20:06 ` Jonas Oberhauser
2023-01-23 20:41 ` Alan Stern
2023-01-24 13:21 ` Jonas Oberhauser
2023-01-24 15:54 ` Jonas Oberhauser
2023-01-24 17:22 ` Alan Stern
[not found] ` <4c1abc7733794519ad7c5153ae8b58f9@huawei.com>
2023-01-13 16:28 ` Alan Stern
2023-01-13 20:07 ` Paul E. McKenney
2023-01-13 20:32 ` Paul E. McKenney
2023-01-14 17:40 ` Alan Stern
2023-01-14 17:48 ` Paul E. McKenney
[not found] ` <136d019d8c8049f6b737627df830e66f@huawei.com>
2023-01-14 17:53 ` Paul E. McKenney
2023-01-14 18:15 ` Paul E. McKenney
2023-01-14 19:58 ` Alan Stern
2023-01-15 5:19 ` Paul E. McKenney
2023-01-14 20:19 ` Alan Stern
2023-01-15 5:15 ` Paul E. McKenney
2023-01-15 16:23 ` Alan Stern
2023-01-15 18:10 ` Paul E. McKenney
2023-01-15 20:46 ` Alan Stern
2023-01-16 4:23 ` Paul E. McKenney
2023-01-16 18:11 ` Alan Stern
2023-01-16 19:06 ` Paul E. McKenney
2023-01-16 19:20 ` Alan Stern
2023-01-16 22:13 ` Paul E. McKenney
2023-01-17 11:46 ` Andrea Parri
2023-01-17 15:14 ` Paul E. McKenney
2023-01-17 15:56 ` Alan Stern
2023-01-17 17:43 ` Paul E. McKenney
2023-01-17 18:27 ` Jonas Oberhauser
2023-01-17 18:55 ` Paul E. McKenney
2023-01-17 20:20 ` Jonas Oberhauser
2023-01-17 20:15 ` Alan Stern
2023-01-18 3:50 ` Paul E. McKenney
2023-01-18 16:50 ` Alan Stern
2023-01-18 19:42 ` Jonas Oberhauser
2023-01-18 20:19 ` Paul E. McKenney
2023-01-18 20:30 ` Jonas Oberhauser
2023-01-18 21:12 ` Paul E. McKenney
2023-01-18 21:24 ` Jonas Oberhauser
2023-01-19 0:11 ` Paul E. McKenney
2023-01-19 13:39 ` Jonas Oberhauser
2023-01-19 18:41 ` Paul E. McKenney
2023-01-19 19:51 ` Alan Stern
2023-01-19 21:53 ` Paul E. McKenney
2023-01-19 22:04 ` Alan Stern
2023-01-19 23:03 ` Paul E. McKenney
2023-01-20 9:43 ` Jonas Oberhauser
2023-01-20 15:39 ` Paul E. McKenney
2023-01-20 20:46 ` Jonas Oberhauser
2023-01-20 21:37 ` Paul E. McKenney
2023-01-20 22:36 ` Jonas Oberhauser
2023-01-20 23:19 ` Paul E. McKenney
2023-01-21 0:03 ` Jonas Oberhauser
2023-01-21 0:34 ` Paul E. McKenney
2023-01-20 3:55 ` Paul E. McKenney
2023-01-20 9:20 ` Jonas Oberhauser
2023-01-20 12:34 ` Jonas Oberhauser
2023-01-20 12:51 ` Jonas Oberhauser
2023-01-20 15:32 ` Paul E. McKenney
2023-01-20 20:56 ` Jonas Oberhauser
2023-01-20 21:40 ` Paul E. McKenney
2023-01-20 16:14 ` Alan Stern
2023-01-20 17:30 ` Paul E. McKenney
2023-01-20 18:15 ` Alan Stern
2023-01-20 18:59 ` Paul E. McKenney
2023-01-20 10:13 ` Jonas Oberhauser
2023-01-20 15:47 ` Paul E. McKenney
2023-01-20 22:21 ` Jonas Oberhauser
2023-01-20 16:18 ` Alan Stern
2023-01-20 21:41 ` Jonas Oberhauser
2023-01-21 4:38 ` Paul E. McKenney
2023-01-21 17:36 ` Alan Stern
2023-01-21 18:40 ` Paul E. McKenney
2023-01-21 19:56 ` Alan Stern
2023-01-21 20:10 ` Paul E. McKenney [this message]
2023-01-21 21:03 ` Alan Stern
2023-01-21 23:49 ` Paul E. McKenney
2023-01-23 11:48 ` Jonas Oberhauser
2023-01-23 15:55 ` Alan Stern
2023-01-23 19:40 ` Jonas Oberhauser
2023-01-23 20:34 ` Alan Stern
2023-01-18 20:06 ` Paul E. McKenney
2023-01-18 20:54 ` Alan Stern
2023-01-18 21:05 ` Jonas Oberhauser
2023-01-19 0:02 ` Paul E. McKenney
2023-01-19 2:19 ` Alan Stern
2023-01-19 11:23 ` Paul E. McKenney
2023-01-20 16:01 ` Alan Stern
2023-01-20 17:58 ` Paul E. McKenney
2023-01-20 18:37 ` Alan Stern
2023-01-20 19:20 ` Paul E. McKenney
2023-01-20 20:36 ` Alan Stern
2023-01-20 21:20 ` Paul E. McKenney
2023-01-22 20:32 ` Alan Stern
2023-01-23 20:16 ` Paul E. McKenney
2023-01-24 2:18 ` Alan Stern
2023-01-24 4:06 ` Paul E. McKenney
2023-01-24 11:09 ` Andrea Parri
2023-01-24 14:54 ` Paul E. McKenney
2023-01-24 15:11 ` Jonas Oberhauser
2023-01-24 16:22 ` Paul E. McKenney
2023-01-24 16:39 ` Jonas Oberhauser
2023-01-24 17:26 ` Paul E. McKenney
2023-01-24 19:30 ` Jonas Oberhauser
2023-01-24 22:15 ` Paul E. McKenney
2023-01-24 22:35 ` Alan Stern
2023-01-24 22:54 ` Paul E. McKenney
2023-01-25 1:54 ` Alan Stern
2023-01-25 2:20 ` Paul E. McKenney
2023-01-25 13:10 ` Jonas Oberhauser
2023-01-25 15:05 ` Paul E. McKenney
2023-01-25 15:34 ` Alan Stern
2023-01-25 17:18 ` Paul E. McKenney
2023-01-25 17:42 ` Jonas Oberhauser
2023-01-25 19:08 ` Alan Stern
2023-01-25 19:46 ` Paul E. McKenney
2023-01-25 20:36 ` Andrea Parri
2023-01-25 21:10 ` Jonas Oberhauser
2023-01-25 21:23 ` Paul E. McKenney
2023-01-25 20:46 ` Alan Stern
2023-01-25 21:38 ` Paul E. McKenney
2023-01-25 23:33 ` Paul E. McKenney
2023-01-26 1:45 ` Alan Stern
2023-01-26 1:53 ` Paul E. McKenney
2023-01-26 12:17 ` Jonas Oberhauser
2023-01-26 18:48 ` Paul E. McKenney
2023-01-27 15:03 ` Jonas Oberhauser
2023-01-27 16:50 ` Paul E. McKenney
2023-01-27 16:54 ` Paul E. McKenney
2023-01-18 19:57 ` Jonas Oberhauser
2023-01-18 21:06 ` Paul E. McKenney
2023-01-18 2:15 ` Alan Stern
2023-01-18 5:17 ` Paul E. McKenney
2023-01-18 16:03 ` Alan Stern
2023-01-18 16:59 ` Boqun Feng
2023-01-18 17:08 ` Alan Stern
2023-01-18 17:41 ` Paul E. McKenney
2023-01-19 19:07 ` Paul E. McKenney
2023-01-14 16:55 ` Alan Stern
2023-01-14 17:35 ` Paul E. McKenney
[not found] ` <17078dd97cb6480f9c51e27058af3197@huawei.com>
2023-01-14 17:27 ` Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230121201026.GH2948950@paulmck-ThinkPad-P17-Gen-1 \
--to=paulmck@kernel.org \
--cc=akiyks@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=dlustig@nvidia.com \
--cc=frederic@kernel.org \
--cc=j.alglave@ucl.ac.uk \
--cc=joel@joelfernandes.org \
--cc=jonas.oberhauser@huawei.com \
--cc=jonas.oberhauser@huaweicloud.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.maranget@inria.fr \
--cc=npiggin@gmail.com \
--cc=parri.andrea@gmail.com \
--cc=peterz@infradead.org \
--cc=quic_neeraju@quicinc.com \
--cc=stern@rowland.harvard.edu \
--cc=urezki@gmail.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).