linux-kernel - Re: [PATCH RFC] srcu: Yet more detail for srcu_readers_active_idx

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]

Date:   Thu, 15 Dec 2022 13:39:00 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     Frederic Weisbecker <frederic@...nel.org>, boqun.feng@...il.com,
        neeraj.iitr10@...il.com, urezki@...il.com, rcu@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] srcu: Yet more detail for
 srcu_readers_active_idx_check() comments

On Thu, Dec 15, 2022 at 03:33:39PM -0500, Joel Fernandes wrote:
> On Thu, Dec 15, 2022 at 3:03 PM Joel Fernandes <joel@...lfernandes.org> wrote:
> >
> > Hi Paul,
> >
> > On Thu, Dec 15, 2022 at 2:58 PM Paul E. McKenney <paulmck@...nel.org> wrote:
> > [...]
> > > > If the first read section's srcu_read_unlock() and its corresponding
> > > > smp_mb()  happened before the flip, then the increment of old idx
> > > > would happen only once. The next srcu_read_lock() will read the new
> > > > index. If the srcu_read_unlock() and it's corresponding smp_mb()
> > > > happened after the flip, the old_idx will be sampled again and can be
> > > > incremented twice. So it depends on how the flip races with
> > > > srcu_read_unlock().
> > >
> > > I do understand that a number of people like reasoning about
> > > memory-barrier ordering, courtesy of the sequentially consistent portions
> > > of the C and C++ memory models, but thinking in terms of the accesses
> > > surrounding the memory barriers has been far less error-prone.
> >
> > Sure, but we are already talking in terms of the access to idx right?
> > That's what we're saying is visible by memory barriers and we are
> > trying to reason here about the ordering (flip does the write to idx
> > and followed by smp_mb(), and there is corresponding read of idx on
> > the srcu_read_lock() side. So we are indeed talking in terms of
> > access, but let me know if I missed something.
> >
> > > > Also, since this is all hard to reason about I started making some
> > > > diagrams, LOL. For your amusement, here is why need to scan both idx
> > > > during grace period detection: https://i.imgur.com/jz4bNKd.png
> > >
> > > Nice!
> > >
> > > I suggest placing a gap between GP 2 and GP 3.  That way, you can make it
> > > very clear that Reader 1's critical section starts after the end of GP 2
> > > (thus clearly never blocking GP 2) and before GP 3 (thus possibly having
> > > a reference to some data that is going to be freed at the end of GP 3).
> > >
> > > I also suggest coloring Reader 1 red and Reader 2 green, given that the
> > > color red generally indicates danger.
> >
> > Thanks for these suggestions! I will make the update. I am planning to
> > make a number of diagrams for other scenarios as well, as it helps
> > visualize. Google drawing is nice for these. I am happy to share these
> > with you all if there is interest :).
> 
> I made these updates, please see: https://i.imgur.com/hoKLvtt.png
> 
> Feel free to use the image for any purpose and thanks ;-)

Very good, thank you!

Would it be possible to have an arrow marked "X" or "reference to X"
from the beginning of the 'Mark "x" for GC' box to the box labeled
'Enter RSCS (access "X")'?

							Thanx, Paul