On Fri, Oct 09, 2020 at 01:45:43PM -0700, Kees Cook wrote:
On Fri, Oct 09, 2020 at 09:37:46PM +0200, Peter Zijlstra wrote:
On Fri, Oct 09, 2020 at 09:55:55AM -0600, Shuah Khan wrote:
Simple atomic counters api provides interfaces for simple atomic counters that just count, and don't guard resource lifetimes. The interfaces are built on top of atomic_t api, providing a smaller subset of atomic_t interfaces necessary to support simple counters.
To what actual purpose?!? AFACIT its pointless wrappery, it gets us nothing.
It's not pointless. There is value is separating types for behavioral constraint to avoid flaws. atomic_t provides a native operation. We gained refcount_t for the "must not wrap" type, and this gets us the other side of that behavioral type, which is "wrapping is expected". Separating the atomic_t uses allows for a clearer path to being able to reason about code flow, whether it be a human or a static analyzer.
refcount_t got us actual rutime exceptions that atomic_t doesn't. This propsal gets us nothing.
atomic_t is very much expected to wrap.
The counter wrappers add nothing to the image size, and only serve to confine the API to one that cannot be used for lifetime management.
It doesn't add anything period. It doesn't get us new behaviour, it splits a 'can wrap' use-case from a 'can wrap' type. That's sodding pointless.
Worse, it mixes 2 unrelated cases into one type, which just makes a mockery of things (all the inc_return users are not statistics, some might even mis-behave if they wrap).
Once conversions are done, we have a clean line between refcounting and statistical atomics, which means we have a much lower chance of introducing new flaws (and maybe we'll fix flaws during the conversion, which we've certainly seen before when doing this stricter type/language changes).
I don't see why this is an objectionable goal.
People can and will always find a way to mess things up.
Only add types when you get behavioural changes, otherwise it's pointless noise.
My NAK stands.