On Wed, Mar 06, 2024 at 01:01:03PM -0500, Steven Rostedt wrote: > On Wed, 6 Mar 2024 09:36:16 -0800 > "Paul E. McKenney" <paulmck@xxxxxxxxxx> wrote: > > > > If we take the policy of handling a compiler that can tear reads and writes > > > of any size word, then we should have proper macros to handle it. > > > > Those are in fact READ_ONCE() and WRITE_ONCE() when given machine-word > > sized/aligned variables. > > IIRC, the original purpose of READ_ONCE() and WRITE_ONCE() was to make sure > that the compiler only reads or writes the variable "once". Hence the name. > That way after a load, you don't need to worry that the content of the > variable you read isn't going to be read again from the original location > because the compiler decided to save stack space and registers. > > But that macro has now been extended for other purposes. If I remember correctly, some 32-bit system had 64-bit PTEs that it wanted to use WRITE_ONCE() on. Does Linux still support that system? If not, maybe it is time to remove that extension. > > > Perhaps READ_SHARED(), WRITE_SHARED(), ADD_SHARED(), SUB_SHARED(). The ONCE > > > has nothing to do with the reasons for these changes. But at least "SHARED" > > > can be considered "this variable is shared between different contexts". > > > Note, this is different than "atomic". It's just to document that this > > > variable must be loaded or stored in one transaction. > > > > We already have READ_ONCE() and WRITE_ONCE(). An ADD_SHARED() might > > be useful, though compilers are starting to learn how to emit good code > > for things like WRITE_ONCE(a, READ_ONCE(a) + 1). > > Well, if we keep the _ONCE() naming, it should be ADD_ONCE(). Because > > WRITE_ONCE(a, READ_ONCE(a) + 1) > > is an abomination and should only be present in obfuscation contests. I have no problem with replacing that sort of thing with ADD_ONCE(). > > But such things should also be documented and added to LKMM. > > > > > I don't know if Linus even cares about fixing "read/write tearing" which is > > > why I Cc'd him. > > > > I am sure that whatever his views, he will not suffer in silence. ;-) > > > > > But I'm not going to take any patches that add these macros to fix > > > compilers that tear words on load and store until we have a set policy on > > > what to do with them. > > > > Maintainer's choice! > > > > For RCU, I want the code to just work with future compiler optimizations > > as well as with current ones. This stuff is fun enough without giving > > the compiler opportunities for more mischief! > > I'm not against the changes. I'm against the ugliness of the changes. > Should we just create a ADD_ONCE() macro? Works for me! We should also update tools/memory-model/linux-kernel.defs to allow it to be used in litmus tests. (I can help with that.) Plus of course documentation. > If the approach is now to find all places that access a variable between > different contexts, and create READ_ONCE()/WRITE_ONCE() around them, I'm > fine with it. I don't know that the entire kernel is going that far, but RCU has had that philosophy for a very long time. Yes, KCSAN sometimes finds places where we slipped up, but those get fixed. > Perhaps we need a way to annotate them, like we have with __rcu. "__shared"? > > Then all accesses to that variable must be wrapped with a READ_ONCE() or > WRITE_ONCE()? I mean, if this can cause legitimate bugs, we should probably > address it like we do with locking and RCU. If we want that, just mark the field "volatile", as in "jiffies". And one of the strengths of READ_ONCE() and WRITE_ONCE() is that they allow non-volatile access where it is safe. For example, if you hold the lock protecting all stores to that variable, you still need WRITE_ONCE() but not READ_ONCE(). In initialization and cleanup code, you don't need either. Thanx, Paul
