On Fri, Jul 23, 2021 at 05:03:47PM -0400, Alan Stern wrote: > On Fri, Jul 23, 2021 at 01:28:11PM -0700, Paul E. McKenney wrote: > > On Fri, Jul 23, 2021 at 02:11:38PM -0400, Alan Stern wrote: > > > In other words, if the second read races with the WRITE_ONCE, it needs > to > > > get either the value before the write or the value after the write; > > > nothing else will do because it isn't a heuristic here. Fair point. > > > > > > > (If the value changes immediately after being read, the fact that > > > > ->f_lock is held prevents begin_global() from completing.) > > > > > > This seems like something worth explaining in the document. That > > > "IMPORTANT" comment doesn't really get the full point across. > > > > How about this comment instead? > > > > /* This works even if data_race() returns nonsense. */ > > That's somewhat better. > > > On Fri, Jul 23, 2021 at 01:32:48PM -0700, Paul E. McKenney wrote: > > On Fri, Jul 23, 2021 at 01:08:20PM -0400, Alan Stern wrote: > > > This doesn't mention the reason for the acquire-release > > > synchronization of global_flag. It's needed because work done between > > > begin_global() and end_global() can affect a foo structure without > > > holding its private f_lock member, and we want all such work to be > > > visible to other threads when they call do_something_locked() later. > > > > Like this added paragraph at the end? > > > > The smp_load_acquire() and smp_store_release() are required > > because changes to a foo structure between calls to begin_global() > > and end_global() are carried out without holding that structure's > > ->f_lock. The smp_load_acquire() and smp_store_release() > > ensure that the next invocation of do_something() from the call > > to do_something_locked() that acquires that ->f_lock will see > > those changes. > > I'd shorten the last sentence: > > The smp_load_acquire() and smp_store_release() ensure that the > next invocation of do_something() from do_something_locked() > will see those changes. Sold! ;-) Thanx, Paul
