RE: [PATCH] refcount: provide same memory ordering guarantees as in atomic_t

From: Reshetova, Elena
Date: Fri Oct 27 2017 - 02:50:03 EST


> On Mon, Oct 23, 2017 at 02:09:44PM +0300, Elena Reshetova wrote:
> > Currently arch. independent implementation of refcount_t in
> > lib/refcount.c provides weak memory ordering guarantees
> > compare to its analog atomic_t implementations.
> > While it should not be a problem for most of the actual
> > cases of refcounters, it is more understandable for everyone
> > (and more error-prone for future users) to provide exactly
> > same memory ordering guarantees as atomics.
> >
> > If speed is of a concern, then either more efficient arch.
> > dependent refcount_t implementation should be used or if there
> > are enough users in the future we might need to provide both
> > strict and relaxed refcount_t APIs.
> >
> > Suggested-by: Kees Cook <keescook@xxxxxxxxxxxx>
>
> NAK

Could we possibly have a bit more elaborate discussion on this?

Or alternatively, what then should be the correct way for a certain variable (that behaves like
a standard refcounter) to check if the relaxed memory ordering is ok?
This is what Thomas was asking me to do for core kernel conversions and this
is what I don't know how to do correctly. Also, I got exactly the same question
from xfs maintainer, so if we provide an API that we hope will be used correctly in
the future, we should have a way for people to verify it.

Maybe it is just me, but I would think that having a way to verify that your code
is ok with this refcounter-specific relaxed memory ordering applies to any memory ordering
requirements, refcounters are just a subset with certain properties, so is then
the full answer is to figure out how to verify any memory ordering requirement
of your code?

We can also document this logic in more docs or even maybe try to create a better
documentation for current memory ordering bits since it is not the most easiest read
at the moment. Otherwise this might be just bad enough reason for many people to
avoid refcount_t type if it is just easier to tell "let's take just old atomic, we knew it
somehow worked before"....

Best Regards,
Elena.