> From: Philip Guenther <guent...@gmail.com> > Date: Thu, 20 May 2021 23:45:46 -0900 > > On Wed, May 19, 2021 at 11:29 PM Martin Pieuchot <m...@openbsd.org> wrote: > > On 19/05/21(Wed) 16:17, Mark Kettenis wrote: > > ... > > > There are the READ_ONCE() and WRITE_ONCE() macros. I'm not a big fan > > of those (since they add clutter) but they do take care of dependency > > ordering issues that exist in the alpha memory model. Must admit that > > I only vaguely understand that issue, but I think it involves ordered > > access to two atomic variables which doesn't seem to be the case. > > These macros are used in places where declaring the field as "volatile" > could also work, no? We can look at __mp_lock and SMR implementations. > So could we agree one way to do things? > > Visa, David, why did you pick READ_ONCE() in SMR and veb(4)? Anything > we overlooked regarding the use of "volatile"? > > If _all_ references to a member/variable use READ/WRITE_ONCE, then declaring > it volatile should be equivalent, but if there are any uses which have a > lock for protection instead than making the member volatile will result in > worse object code for the protected sequences where *_ONCE are not needed.
I'd argue that mixing atomic and non-atomic (protected by a lock) access to a member would be a bug, but I suppose one can argue that there are scenarios where using a lock to prevent simultanious writes and use atomic access for reads is valid. > Also, initialization of structs with volatile members can be pessimal: the > compiler has to assume this could be in uncached memory mapped from a device > where writes to the member have to be of the size of the member: no paving > with larger writes or deferring initializations. Right. But if you really want to access that variable atomically, that is really what you want. > volatile is a *really* blunt hammer. READ/WRITE_ONCE use it carefully to > build a sharper tool. Unifying on "just plain volatile" when the work has > already been done to use a sharper tool correctly..well, if that's a good > idea then why have SMR at all when locks would be easier for everyone to > think about, despite being a blunter hammer? /s That said, in the example at hand, the checks on the reference count should be fine as the checks should only be done by CPUs that hold a reference. So we don't need true atomic access. So Martin, I think you should commit the diff as-is. ok kettenis@
