On Wed, Dec 18, 2024 at 03:21:42PM +0900, Akira Yokosawa wrote: > Akira Yokosawa wrote: > > Daniel Xu wrote: > >> Hi Akira, > >> > >> Thanks for taking a look! > >> > >> On Tue, Dec 17, 2024, at 5:53 PM, Akira Yokosawa wrote: > >>> Hello, > >>> > >>> Daniel Xu wrote: > >>>> WRITE_ONCE() is needed here to prevent store tears and other unwanted > >>>> compiler optimizations. > >>> > >>> That might be true if there were chances of these two accesses to > >>> race with each other. > >>> > >>> I don't see any possibility of such races. > >>> > >>> Can you elaborate? > >> > >> My understanding is that read_seqbegin() and read_seqretry() > >> can execute at any time. That means the read side access of > >> the sequence number can occur doing an increment. > >> > >> To prevent the reader from reading a partially written value, > >> we need the WRITE_ONCE() to ensure the relaxed atomic > >> write. > > > > OK, now I see the race there. > > > > But see below. > > > [...] > > > > Those two READ_ONCE()s don't have any concurrent writes. > > So there seems to be no need of them, IIUC. > > Furthermore, in sequence locking, readers will retry if they see > an inconsistent sequence count due to store tearing, so WRITE_ONCE() > is not necessary, either. True, but this is needlessly increasing the state space. Plus there are other potential optimizations. These points were discussed for the Linux-kernel counterpart to this patch here: https://lore.kernel.org/all/0eaea03ecc9df536649763cfecda356fc38b6938.1734477414.git.dxu@xxxxxxxxx/ > READ_ONCE()s in read_seqbegin() and read_seqretry() are there for > preventing read accesses in retry loops from being optimized out. Completely agreed! Thanx, Paul
