On 19/05/21(Wed) 16:17, Mark Kettenis wrote: > > Date: Tue, 18 May 2021 13:24:42 +0200 > > From: Martin Pieuchot <m...@openbsd.org> > > > > On 18/05/21(Tue) 12:07, Mark Kettenis wrote: > > > > Date: Tue, 18 May 2021 12:02:19 +0200 > > > > From: Martin Pieuchot <m...@openbsd.org> > > > > > > > > This allows us to not rely on the KERNEL_LOCK() to check reference > > > > counts. > > > > > > > > Also reduces differences with NetBSD and shrink my upcoming `vmobjlock' > > > > diff. > > > > > > > > ok? > > > > > > Shouldn't we make ref_count volatile in that case? > > > > I don't know, I couldn't find any evidence about where to use "volatile" > > in the kernel. > > > > My understanding is that using "volatile" tells the compiler to not > > "cache" the value of such field in a register because it can change at > > any time. Is it so? > > Right. So if you want the access to be atomic, it needs to be > "uncached" and therefore you need to use volatile. Now the atomic > APIs explicitly cast their pointer arguments to volatile, so if you > exclusively through those APIs you don't strictly need the variable > itself to be declared volatile. But I think it still is a good idea > to declare them as such.
Thanks for the explanation. Do you suggest we use the "volatile" keyword as a hint and/or to avoid surprises? If we agree on this I'll look at similar uses of atomic operations to unify them. > > There's only a couple of 'volatile' usages in sys/sys. These annotations > > do not explicitly indicate which piece of code requires it. Maybe it would > > be clearer to use a cast or a macro where necessary. This might help us > > understand why and where "volatile" is needed. > > 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"?
