Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2017-01-30 Thread Peter Zijlstra
On Fri, Jan 27, 2017 at 01:07:35PM -0800, Kees Cook wrote: > On Fri, Jan 27, 2017 at 1:58 AM, Peter Zijlstra wrote: > > Nothing much, except lack of time. I spend the last several days hunting > > bugs, that trumps new features on my todo list. > > Totally understood. I was just trying to see if

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2017-01-27 Thread Kees Cook
On Fri, Jan 27, 2017 at 1:58 AM, Peter Zijlstra wrote: > On Thu, Jan 26, 2017 at 03:14:41PM -0800, Kees Cook wrote: >> On Mon, Nov 14, 2016 at 9:39 AM, Peter Zijlstra wrote: >> > Provide refcount_t, an atomic_t like primitive built just for >> > refcounting. >> > >> > It provides overflow and und

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2017-01-27 Thread Peter Zijlstra
On Thu, Jan 26, 2017 at 03:14:41PM -0800, Kees Cook wrote: > On Mon, Nov 14, 2016 at 9:39 AM, Peter Zijlstra wrote: > > Provide refcount_t, an atomic_t like primitive built just for > > refcounting. > > > > It provides overflow and underflow checks as well as saturation > > semantics such that whe

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2017-01-26 Thread Kees Cook
On Mon, Nov 14, 2016 at 9:39 AM, Peter Zijlstra wrote: > Provide refcount_t, an atomic_t like primitive built just for > refcounting. > > It provides overflow and underflow checks as well as saturation > semantics such that when it overflows, we'll never attempt to free it > again, ever. > > Signe

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-21 Thread Boqun Feng
On Mon, Nov 21, 2016 at 10:02:23AM +0100, Peter Zijlstra wrote: > On Mon, Nov 21, 2016 at 04:44:28PM +0800, Boqun Feng wrote: > > On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: > > [snip] > > > + > > > +/* > > > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN. > > >

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-21 Thread Peter Zijlstra
On Mon, Nov 21, 2016 at 04:44:28PM +0800, Boqun Feng wrote: > On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: > [snip] > > + > > +/* > > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN. > > + * > > + * Provides no memory ordering, it is assumed the caller already has

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-21 Thread Boqun Feng
On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: [snip] > + > +/* > + * Similar to atomic_inc(), will saturate at UINT_MAX and WARN. > + * > + * Provides no memory ordering, it is assumed the caller already has a > + * reference on the object, will WARN when this is not so. > + */ >

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-21 Thread Boqun Feng
On Mon, Nov 21, 2016 at 08:48:26AM +0100, Ingo Molnar wrote: > > * Boqun Feng wrote: > > > > It also fails to decrement in the underflow case (which is fine, but not > > > obvious from the comment). Same thing below. > > > > > > > Maybe a table in the comment like the following helps? > > > >

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-20 Thread Ingo Molnar
* Boqun Feng wrote: > > It also fails to decrement in the underflow case (which is fine, but not > > obvious from the comment). Same thing below. > > > > Maybe a table in the comment like the following helps? > > /* > * T: return true, F: return fasle > * W: trigger WARNING > * N: no effec

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-20 Thread Boqun Feng
On Fri, Nov 18, 2016 at 05:06:55PM +, Will Deacon wrote: > On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: > > On Fri, Nov 18, 2016 at 10:07:26AM +, Reshetova, Elena wrote: > > > > > > Peter do you have the changes to the refcount_t interface compare to > > > the version in

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-19 Thread Peter Zijlstra
On Sat, Nov 19, 2016 at 07:14:08AM +, Reshetova, Elena wrote: > > Well, if you get to tools (cocci script or whatever) to reliably work > > fork atomic_t, then converting the few atomic_long_t's later should be > > trivial. > > I am using coccinelle to find all occurrences, but I do the change

RE: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Reshetova, Elena
> On Fri, Nov 18, 2016 at 04:58:52PM +, Reshetova, Elena wrote: > > > Could you please fix you mailer to not unwrap the emails? > > > > I wish I understand what you mean by "unwrap"... ? > > Where I always have lines wrapped at 78 characters, but often when I see > them back in your reply, th

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Peter Zijlstra
On Fri, Nov 18, 2016 at 05:06:55PM +, Will Deacon wrote: > On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: > > +static inline void refcount_set(refcount_t *r, int n) > > +{ > > + atomic_set(&r->refs, n); > > +} > > + > > +static inline unsigned int refcount_read(const refcount

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Peter Zijlstra
On Fri, Nov 18, 2016 at 04:58:52PM +, Reshetova, Elena wrote: > > Could you please fix you mailer to not unwrap the emails? > > I wish I understand what you mean by "unwrap"... ? Where I always have lines wrapped at 78 characters, but often when I see them back in your reply, they're unwrappe

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Will Deacon
On Fri, Nov 18, 2016 at 12:37:18PM +0100, Peter Zijlstra wrote: > On Fri, Nov 18, 2016 at 10:07:26AM +, Reshetova, Elena wrote: > > > > Peter do you have the changes to the refcount_t interface compare to > > the version in this patch? > > > We are now starting working on atomic_t --> refcou

RE: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Reshetova, Elena
> Could you please fix you mailer to not unwrap the emails? I wish I understand what you mean by "unwrap"... ? On Fri, Nov 18, 2016 at 10:47:40AM +, Reshetova, Elena wrote: > >Provide refcount_t, an atomic_t like primitive built just for > >refcounting. It provides overflow and underflow ch

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Peter Zijlstra
On Fri, Nov 18, 2016 at 10:07:26AM +, Reshetova, Elena wrote: > > Peter do you have the changes to the refcount_t interface compare to > the version in this patch? > We are now starting working on atomic_t --> refcount_t conversions and > it would save a bit of work to have latest version fr

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Peter Zijlstra
Could you please fix you mailer to not unwrap the emails? On Fri, Nov 18, 2016 at 10:47:40AM +, Reshetova, Elena wrote: > >Provide refcount_t, an atomic_t like primitive built just for > >refcounting. It provides overflow and underflow checks as well as > >saturation semantics such that when

RE: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Reshetova, Elena
>Provide refcount_t, an atomic_t like primitive built just for refcounting. >It provides overflow and underflow checks as well as saturation semantics such >that when it overflows, we'll never attempt to free it again, ever. >Peter do you have the changes to the refcount_t interface compare to th

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Will Deacon
On Fri, Nov 18, 2016 at 04:26:34PM +0800, Boqun Feng wrote: > On Thu, Nov 17, 2016 at 04:36:24PM +, Will Deacon wrote: > > On Thu, Nov 17, 2016 at 05:11:10PM +0100, Peter Zijlstra wrote: > > > On Thu, Nov 17, 2016 at 12:08:36PM +, Will Deacon wrote: > > > > All sounds reasonable to me. It's

RE: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Reshetova, Elena
>Provide refcount_t, an atomic_t like primitive built just for refcounting. >It provides overflow and underflow checks as well as saturation semantics such >that when it overflows, we'll never attempt to free it again, ever. Peter do you have the changes to the refcount_t interface compare to the

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-18 Thread Boqun Feng
On Thu, Nov 17, 2016 at 04:36:24PM +, Will Deacon wrote: > On Thu, Nov 17, 2016 at 05:11:10PM +0100, Peter Zijlstra wrote: > > On Thu, Nov 17, 2016 at 12:08:36PM +, Will Deacon wrote: > > > All sounds reasonable to me. It's worth pointing out that you can't create > > > order using a contro

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Kees Cook
On Thu, Nov 17, 2016 at 12:33 AM, Peter Zijlstra wrote: > On Wed, Nov 16, 2016 at 10:55:16AM -0800, Kees Cook wrote: >> My intention with what I'm designing is to couple the "panic_on_oops" > > There is a panic_on_warn knob too. Yes, and that tends to be "too much". There is a need to create some

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Thu, Nov 17, 2016 at 06:54:32PM +0800, Boqun Feng wrote: > > If our freshly allocated object got trampled by stores in free(), that's the > problem of allocator and free(), right? Because in that case, it's them who > mess up the synchronization. > Oh right, duh.

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Thu, Nov 17, 2016 at 12:08:36PM +, Will Deacon wrote: > All sounds reasonable to me. It's worth pointing out that you can't create > order using a control dependency hanging off the status flag of a > store-conditional, but the code in question here has the dependency from > the loaded value

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Thu, Nov 17, 2016 at 12:03:33PM +0100, Greg KH wrote: > On Thu, Nov 17, 2016 at 11:39:27AM +0100, Peter Zijlstra wrote: > > And let me note here that RCU users can use a fully relaxed put, because > > call_rcu() guarantees a grace-period between the call_rcu and the > > free(), which in turn pr

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Will Deacon
On Thu, Nov 17, 2016 at 05:11:10PM +0100, Peter Zijlstra wrote: > On Thu, Nov 17, 2016 at 12:08:36PM +, Will Deacon wrote: > > All sounds reasonable to me. It's worth pointing out that you can't create > > order using a control dependency hanging off the status flag of a > > store-conditional,

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Will Deacon
On Thu, Nov 17, 2016 at 10:28:00AM +0100, Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 10:19:09PM +0800, Boqun Feng wrote: > > But as I said, we actually only need the pairing of orderings: > > > > 1) load part of cmpxchg -> free() > > 2) object accesses -> store part of cmpxchg > > > > Order

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Greg KH
On Thu, Nov 17, 2016 at 11:39:27AM +0100, Peter Zijlstra wrote: > On Thu, Nov 17, 2016 at 11:29:59AM +0100, Peter Zijlstra wrote: > > On Thu, Nov 17, 2016 at 05:48:51PM +0800, Boqun Feng wrote: > > > > > But as I said, we actually only need the pairing of orderings: > > > > > > > > > > 1) load par

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Thu, Nov 17, 2016 at 11:29:59AM +0100, Peter Zijlstra wrote: > On Thu, Nov 17, 2016 at 05:48:51PM +0800, Boqun Feng wrote: > > > > But as I said, we actually only need the pairing of orderings: > > > > > > > > 1) load part of cmpxchg -> free() > > > > 2) object accesses -> store part of cmpxch

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Thu, Nov 17, 2016 at 05:48:51PM +0800, Boqun Feng wrote: > > > But as I said, we actually only need the pairing of orderings: > > > > > > 1) load part of cmpxchg -> free() > > > 2) object accesses -> store part of cmpxchg > > > > > > Ordering #1 can be achieved via control dependency as you p

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Boqun Feng
On Thu, Nov 17, 2016 at 10:28:00AM +0100, Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 10:19:09PM +0800, Boqun Feng wrote: > > On Tue, Nov 15, 2016 at 02:01:54PM +0100, Peter Zijlstra wrote: > > > On Tue, Nov 15, 2016 at 08:33:37PM +0800, Boqun Feng wrote: > > > > Hi Peter, > > > > > > > > On M

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Tue, Nov 15, 2016 at 10:19:09PM +0800, Boqun Feng wrote: > On Tue, Nov 15, 2016 at 02:01:54PM +0100, Peter Zijlstra wrote: > > On Tue, Nov 15, 2016 at 08:33:37PM +0800, Boqun Feng wrote: > > > Hi Peter, > > > > > > On Mon, Nov 14, 2016 at 06:39:53PM +0100, Peter Zijlstra wrote: > > > [...] > >

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-17 Thread Peter Zijlstra
On Wed, Nov 16, 2016 at 10:55:16AM -0800, Kees Cook wrote: > My intention with what I'm designing is to couple the "panic_on_oops" There is a panic_on_warn knob too.

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Kees Cook
On Wed, Nov 16, 2016 at 2:15 AM, Peter Zijlstra wrote: > On Wed, Nov 16, 2016 at 09:31:55AM +0100, Ingo Molnar wrote: >> >> * Kees Cook wrote: >> >> > On Tue, Nov 15, 2016 at 11:16 AM, Peter Zijlstra >> > wrote: >> > > >> > > >> > > On 15 November 2016 19:06:28 CET, Kees Cook >> > > wrote: >>

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Kees Cook
On Wed, Nov 16, 2016 at 12:31 AM, Ingo Molnar wrote: > > * Kees Cook wrote: > >> On Tue, Nov 15, 2016 at 11:16 AM, Peter Zijlstra >> wrote: >> > >> > >> > On 15 November 2016 19:06:28 CET, Kees Cook wrote: >> > >> >>I'll want to modify this in the future; I have a config already doing >> >>"Bu

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Peter Zijlstra
On Wed, Nov 16, 2016 at 09:31:55AM +0100, Ingo Molnar wrote: > > * Kees Cook wrote: > > > On Tue, Nov 15, 2016 at 11:16 AM, Peter Zijlstra > > wrote: > > > > > > > > > On 15 November 2016 19:06:28 CET, Kees Cook wrote: > > > > > >>I'll want to modify this in the future; I have a config alread

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Greg KH
On Wed, Nov 16, 2016 at 10:07:37AM +0100, Ingo Molnar wrote: > > * Greg KH wrote: > > > On Wed, Nov 16, 2016 at 09:31:55AM +0100, Ingo Molnar wrote: > > > So what I'd love to see is to have a kernel option that re-introduces > > > some > > > historic root (and other) holes that can be exploite

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Ingo Molnar
* Greg KH wrote: > On Wed, Nov 16, 2016 at 09:31:55AM +0100, Ingo Molnar wrote: > > So what I'd love to see is to have a kernel option that re-introduces some > > historic root (and other) holes that can be exploited deterministically - > > obviously default disabled. > > Ick, I don't want to

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Greg KH
On Wed, Nov 16, 2016 at 09:31:55AM +0100, Ingo Molnar wrote: > So what I'd love to see is to have a kernel option that re-introduces some > historic root (and other) holes that can be exploited deterministically - > obviously default disabled. Ick, I don't want to have to support nasty #ifdefs f

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-16 Thread Ingo Molnar
* Kees Cook wrote: > On Tue, Nov 15, 2016 at 11:16 AM, Peter Zijlstra wrote: > > > > > > On 15 November 2016 19:06:28 CET, Kees Cook wrote: > > > >>I'll want to modify this in the future; I have a config already doing > >>"Bug on data structure corruption" that makes the warn/bug choice. > >>I

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Kees Cook
On Tue, Nov 15, 2016 at 11:16 AM, Peter Zijlstra wrote: > > > On 15 November 2016 19:06:28 CET, Kees Cook wrote: > >>I'll want to modify this in the future; I have a config already doing >>"Bug on data structure corruption" that makes the warn/bug choice. >>It'll need some massaging to fit into t

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Peter Zijlstra
On 15 November 2016 19:06:28 CET, Kees Cook wrote: >I'll want to modify this in the future; I have a config already doing >"Bug on data structure corruption" that makes the warn/bug choice. >It'll need some massaging to fit into the new refcount_t checks, but >it should be okay -- there needs t

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Kees Cook
On Tue, Nov 15, 2016 at 5:03 AM, Ingo Molnar wrote: > > * Peter Zijlstra wrote: > >> On Tue, Nov 15, 2016 at 11:03:59AM +0100, Ingo Molnar wrote: >> > > Should I also make a CONFIG knob that implements refcount_t with the >> > > 'normal' atomic_t primitives? >> > >> > I'd suggest doing the satura

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Boqun Feng
On Tue, Nov 15, 2016 at 02:01:54PM +0100, Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 08:33:37PM +0800, Boqun Feng wrote: > > Hi Peter, > > > > On Mon, Nov 14, 2016 at 06:39:53PM +0100, Peter Zijlstra wrote: > > [...] > > > +/* > > > + * Similar to atomic_dec_and_test(), it will BUG on underfl

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Ingo Molnar
* Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 11:03:59AM +0100, Ingo Molnar wrote: > > > Should I also make a CONFIG knob that implements refcount_t with the > > > 'normal' atomic_t primitives? > > > > I'd suggest doing the saturation/safe-wrap semantics only for now (i.e. the > > current

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Peter Zijlstra
On Tue, Nov 15, 2016 at 08:33:37PM +0800, Boqun Feng wrote: > Hi Peter, > > On Mon, Nov 14, 2016 at 06:39:53PM +0100, Peter Zijlstra wrote: > [...] > > +/* > > + * Similar to atomic_dec_and_test(), it will BUG on underflow and fail to > > + * decrement when saturated at UINT_MAX. > > + * > > + * P

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Boqun Feng
Hi Peter, On Mon, Nov 14, 2016 at 06:39:53PM +0100, Peter Zijlstra wrote: [...] > +/* > + * Similar to atomic_dec_and_test(), it will BUG on underflow and fail to > + * decrement when saturated at UINT_MAX. > + * > + * Provides release memory ordering, such that prior loads and stores are > done

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Peter Zijlstra
On Tue, Nov 15, 2016 at 11:03:59AM +0100, Ingo Molnar wrote: > > Should I also make a CONFIG knob that implements refcount_t with the > > 'normal' atomic_t primitives? > > I'd suggest doing the saturation/safe-wrap semantics only for now (i.e. the > current patch, split into two perhaps), and rec

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Ingo Molnar
* Peter Zijlstra wrote: > On Tue, Nov 15, 2016 at 09:40:09AM +0100, Ingo Molnar wrote: > > > > * Peter Zijlstra wrote: > > > > > Provide refcount_t, an atomic_t like primitive built just for > > > refcounting. > > > > > > It provides overflow and underflow checks as well as saturation > > >

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Peter Zijlstra
On Tue, Nov 15, 2016 at 09:40:09AM +0100, Ingo Molnar wrote: > > * Peter Zijlstra wrote: > > > Provide refcount_t, an atomic_t like primitive built just for > > refcounting. > > > > It provides overflow and underflow checks as well as saturation > > semantics such that when it overflows, we'll

Re: [RFC][PATCH 7/7] kref: Implement using refcount_t

2016-11-15 Thread Ingo Molnar
* Peter Zijlstra wrote: > Provide refcount_t, an atomic_t like primitive built just for > refcounting. > > It provides overflow and underflow checks as well as saturation > semantics such that when it overflows, we'll never attempt to free it > again, ever. > > Signed-off-by: Peter Zijlstra (I