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. Alan
