Paul E. McKenney wrote: > 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/ Thank you for the link. Hopefully, INC_ONCE() can land in the near future. Thanks, Akira
