> Date: Thu, 20 May 2021 10:28:29 +0200 > From: Martin Pieuchot <m...@openbsd.org> > > 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.
Yes, I think we should do that. The volatile keyword shouldn't hurt and is a clear signal that there is something special about a variable. > > > 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? Not 100%; see the comment about the alpha memory model above. So as long as we support OpenBSD/alpha, READ_ONCE() and WRITE_ONCE() will be necessary in certain cases. > Visa, David, why did you pick READ_ONCE() in SMR and veb(4)? Anything > we overlooked regarding the use of "volatile"?
