Date: Fri, 11 Nov 2016 13:47:55 +0100 From: Peter Zijlstra <peterz@...radead.org> To: Mark Rutland <mark.rutland@....com> Cc: kernel-hardening@...ts.openwall.com, Kees Cook <keescook@...omium.org>, Greg KH <gregkh@...uxfoundation.org>, Will Deacon <will.deacon@....com>, Elena Reshetova <elena.reshetova@...el.com>, Arnd Bergmann <arnd@...db.de>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <h.peter.anvin@...el.com>, LKML <linux-kernel@...r.kernel.org> Subject: Re: Re: [RFC v4 PATCH 00/13] HARDENED_ATOMIC On Fri, Nov 11, 2016 at 12:41:27PM +0000, Mark Rutland wrote: > On Fri, Nov 11, 2016 at 01:29:21AM +0100, Colin Vidal wrote: > > I wonder if we didn't make a confusion between naming and > > specifications. I have thought about Kees idea and what you're saying: > > > > - The name "atomic_t" name didn't tell anything about if the variable > > can wrap or not. It just tells there is no race condition on > > concurrent access, nothing else, and users are well with that. OK > > then, we don't modify atomic_t, it makes sense. > > > > - Hence, let's say a new type "refcount_t". It names exactly what we > > try to protect in this patch set. A much more simpler interface than > > atomic_t would be needed, and it protects on race condition and > > overflows (precisely what is expected of a counter reference). Not > > an opt-in solution, but it is much less invasive since we "just" > > have to modify the kref implementation and some vfs reference > > counters. > > > > That didn't tell us how actually implements refcount_t: reuse some > > atomic_t code or not (it would be simpler anyways, since we don't have > > to implement the whole atomic_t interface). Still, this is another > > problem. > > > > Sounds better? > > Regardless of atomic_t semantics, a refcount_t would be far more obvious > to developers than atomic_t and/or kref, and better documents the intent > of code using it. > > We'd still see abuse of atomic_t (and so this won't solve the problems > Kees mentioned), but even as something orthogonal I think that would > make sense to have. Furthermore, you could implement that refcount_t stuff using atomic_cmpxchg() in generic code. While that is sub-optimal for ll/sc architectures you at least get generic code that works to get started. Also, I suspect that if your refcounts are heavily contended, you'll have other problems than the performance of these primitives. Code for refcount_inc(), refcount_inc_not_zero() and refcount_sub_and_test() can be copy-pasted from the kref patch I send yesterday.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.