On Wed, Nov 13, 2024 at 03:36:14PM +0100, Uros Bizjak wrote: > On Wed, Nov 13, 2024 at 3:28 PM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > > On Wed, Nov 13, 2024 at 03:14:09PM +0100, Uros Bizjak wrote: > > > > > Even without atomicity guarantee, __READ_ONCE() still prevents the > > > > > compiler from performing unwanted optimizations (please see the first > > > > > comment in include/asm-generic/rwonce.h) and unwanted reordering of > > > > > reads and writes when this function is inlined. This macro does cast > > > > > the read to volatile, but IMO it is much more readable to use > > > > > __READ_ONCE() than volatile qualifier. > > > > > > > > Yes it does, but please explain to me what "unwanted reordering" is > > > > allowed here? > > > > > > It is a static function that will be inlined by the compiler > > > somewhere, so "unwanted reordering" depends on where it will be > > > inlined. *IF* it will be called from safe code, then this limitation > > > for the compiler can be lifted. > > > > As long as the values are read within the spinlock the order does not > > matter. READ_ONCE() is not required to contain reads within spinlocks. > > Indeed. But then why complicate things with cmpxchg, when we have > exclusive access to the shared memory? No other thread can access the > data, protected by spinlock; it won't change between invocations of > cmpxchg in the loop, and atomic access via cmpxchg is not needed. This is writing to memory shared by HW and HW is doing a 256 bit atomic load. It is important that the CPU do a 128 bit atomic write. cmpxchg is not required, but a 128 bit store is. cmpxchg128 is the only primitive Linux offers. Jason
