RE: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Reshetova, Elena]

> * Reshetova, Elena  wrote:
> 
> > > Subject: refcount: Out-of-line everything
> > > From: Peter Zijlstra 
> > > Date: Fri Feb 10 16:27:52 CET 2017
> > >
> > > Linus asked to please make this real C code.
> >
> > Perhaps a completely stupid question, but I am going to ask anyway since 
> > only
> > this way I can learn. What a real difference it makes? Or are we talking 
> > more
> > about styling and etc.? Is it because of size concerns? This way it is 
> > certainly
> > now done differently than rest of atomic and kref...
> 
> It's about generated code size mostly.
> 
> This inline function is way too large to be inlined:
> 
> static inline __refcount_check
> bool refcount_add_not_zero(unsigned int i, refcount_t *r)
> {
>   unsigned int old, new, val = atomic_read(>refs);
> 
>   for (;;) {
>   if (!val)
>   return false;
> 
>   if (unlikely(val == UINT_MAX))
>   return true;
> 
>   new = val + i;
>   if (new < val)
>   new = UINT_MAX;
>   old = atomic_cmpxchg_relaxed(>refs, val, new);
>   if (old == val)
>   break;
> 
>   val = old;
>   }
> 
>   REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated;
> leaking memory.\n");
> 
>   return true;
> }
> 
> When used then this function generates this much code on x86-64 defconfig:
> 
> 84d0 :
> 84d0: 8b 0f   mov(%rdi),%ecx
> 84d2: 55  push   %rbp
> 84d3: 48 89 e5mov%rsp,%rbp
> 
> 84d6: 85 c9   test   %ecx,%ecx|
> 84d8: 74 30   je 850a  |
> 84da: 83 f9 ffcmp$0x,%ecx |
> 84dd: be ff ff ff ff  mov$0x,%esi |
> 84e2: 75 04   jne84e8  |
> 84e4: eb 1d   jmp8503  |
> 84e6: 89 c1   mov%eax,%ecx|
> 84e8: 8d 51 01lea0x1(%rcx),%edx   |
> 84eb: 89 c8   mov%ecx,%eax|
> 84ed: 39 ca   cmp%ecx,%edx|
> 84ef: 0f 42 d6cmovb  %esi,%edx|
> 84f2: f0 0f b1 17 lock cmpxchg %edx,(%rdi)|
> 84f6: 39 c8   cmp%ecx,%eax|
> 84f8: 74 09   je 8503  |
> 84fa: 85 c0   test   %eax,%eax|
> 84fc: 74 0c   je 850a  |
> 84fe: 83 f8 ffcmp$0x,%eax |
> 8501: 75 e3   jne84e6  |
> 8503: b8 01 00 00 00  mov$0x1,%eax|
> 
> 8508: 5d  pop%rbp
> 8509: c3  retq
> 850a: 31 c0   xor%eax,%eax
> 850c: 5d  pop%rbp
> 850d: c3  retq
> 
> 
> (I've annotated the fastpath impact with '|'. Out of line code generally does 
> not
> count.)
> 
> It's about ~50 bytes of code per usage site. It might be better in some 
> cases, but
> not by much.
> 
> This is way above any sane inlining threshold. The 'unconditionally good' 
> inlining
> threshold is at 1-2 instructions and below ~10 bytes of code.
> 
> So for example refcount_set() and refcount_read() can stay inlined:
> 
> static inline void refcount_set(refcount_t *r, unsigned int n)
> {
>   atomic_set(>refs, n);
> }
> 
> static inline unsigned int refcount_read(const refcount_t *r)
> {
>   return atomic_read(>refs);
> }
> 
> 
> ... beacuse they compile into a single instruction with 2-5 bytes I$ overhead.
> 
> Thanks,
> 
>   Ingo

Thank you very much Ingo for such detailed and nice explanation! 

Best Regards,
Elena

RE: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Reshetova, Elena]

> * Reshetova, Elena  wrote:
> 
> > > Subject: refcount: Out-of-line everything
> > > From: Peter Zijlstra 
> > > Date: Fri Feb 10 16:27:52 CET 2017
> > >
> > > Linus asked to please make this real C code.
> >
> > Perhaps a completely stupid question, but I am going to ask anyway since 
> > only
> > this way I can learn. What a real difference it makes? Or are we talking 
> > more
> > about styling and etc.? Is it because of size concerns? This way it is 
> > certainly
> > now done differently than rest of atomic and kref...
> 
> It's about generated code size mostly.
> 
> This inline function is way too large to be inlined:
> 
> static inline __refcount_check
> bool refcount_add_not_zero(unsigned int i, refcount_t *r)
> {
>   unsigned int old, new, val = atomic_read(>refs);
> 
>   for (;;) {
>   if (!val)
>   return false;
> 
>   if (unlikely(val == UINT_MAX))
>   return true;
> 
>   new = val + i;
>   if (new < val)
>   new = UINT_MAX;
>   old = atomic_cmpxchg_relaxed(>refs, val, new);
>   if (old == val)
>   break;
> 
>   val = old;
>   }
> 
>   REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated;
> leaking memory.\n");
> 
>   return true;
> }
> 
> When used then this function generates this much code on x86-64 defconfig:
> 
> 84d0 :
> 84d0: 8b 0f   mov(%rdi),%ecx
> 84d2: 55  push   %rbp
> 84d3: 48 89 e5mov%rsp,%rbp
> 
> 84d6: 85 c9   test   %ecx,%ecx|
> 84d8: 74 30   je 850a  |
> 84da: 83 f9 ffcmp$0x,%ecx |
> 84dd: be ff ff ff ff  mov$0x,%esi |
> 84e2: 75 04   jne84e8  |
> 84e4: eb 1d   jmp8503  |
> 84e6: 89 c1   mov%eax,%ecx|
> 84e8: 8d 51 01lea0x1(%rcx),%edx   |
> 84eb: 89 c8   mov%ecx,%eax|
> 84ed: 39 ca   cmp%ecx,%edx|
> 84ef: 0f 42 d6cmovb  %esi,%edx|
> 84f2: f0 0f b1 17 lock cmpxchg %edx,(%rdi)|
> 84f6: 39 c8   cmp%ecx,%eax|
> 84f8: 74 09   je 8503  |
> 84fa: 85 c0   test   %eax,%eax|
> 84fc: 74 0c   je 850a  |
> 84fe: 83 f8 ffcmp$0x,%eax |
> 8501: 75 e3   jne84e6  |
> 8503: b8 01 00 00 00  mov$0x1,%eax|
> 
> 8508: 5d  pop%rbp
> 8509: c3  retq
> 850a: 31 c0   xor%eax,%eax
> 850c: 5d  pop%rbp
> 850d: c3  retq
> 
> 
> (I've annotated the fastpath impact with '|'. Out of line code generally does 
> not
> count.)
> 
> It's about ~50 bytes of code per usage site. It might be better in some 
> cases, but
> not by much.
> 
> This is way above any sane inlining threshold. The 'unconditionally good' 
> inlining
> threshold is at 1-2 instructions and below ~10 bytes of code.
> 
> So for example refcount_set() and refcount_read() can stay inlined:
> 
> static inline void refcount_set(refcount_t *r, unsigned int n)
> {
>   atomic_set(>refs, n);
> }
> 
> static inline unsigned int refcount_read(const refcount_t *r)
> {
>   return atomic_read(>refs);
> }
> 
> 
> ... beacuse they compile into a single instruction with 2-5 bytes I$ overhead.
> 
> Thanks,
> 
>   Ingo

Thank you very much Ingo for such detailed and nice explanation! 

Best Regards,
Elena

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Ingo Molnar]

* Reshetova, Elena  wrote:

> > Subject: refcount: Out-of-line everything
> > From: Peter Zijlstra 
> > Date: Fri Feb 10 16:27:52 CET 2017
> > 
> > Linus asked to please make this real C code.
> 
> Perhaps a completely stupid question, but I am going to ask anyway since only 
> this way I can learn. What a real difference it makes? Or are we talking more 
> about styling and etc.? Is it because of size concerns? This way it is 
> certainly 
> now done differently than rest of atomic and kref...

It's about generated code size mostly.

This inline function is way too large to be inlined:

static inline __refcount_check
bool refcount_add_not_zero(unsigned int i, refcount_t *r)
{
unsigned int old, new, val = atomic_read(>refs);

for (;;) {
if (!val)
return false;

if (unlikely(val == UINT_MAX))
return true;

new = val + i;
if (new < val)
new = UINT_MAX;
old = atomic_cmpxchg_relaxed(>refs, val, new);
if (old == val)
break;

val = old;
}

REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking 
memory.\n");

return true;
}

When used then this function generates this much code on x86-64 defconfig:

84d0 :
84d0:   8b 0f   mov(%rdi),%ecx
84d2:   55  push   %rbp
84d3:   48 89 e5mov%rsp,%rbp

84d6:   85 c9   test   %ecx,%ecx|
84d8:   74 30   je 850a  |
84da:   83 f9 ffcmp$0x,%ecx |
84dd:   be ff ff ff ff  mov$0x,%esi |
84e2:   75 04   jne84e8  |
84e4:   eb 1d   jmp8503  |
84e6:   89 c1   mov%eax,%ecx|
84e8:   8d 51 01lea0x1(%rcx),%edx   |
84eb:   89 c8   mov%ecx,%eax|
84ed:   39 ca   cmp%ecx,%edx|
84ef:   0f 42 d6cmovb  %esi,%edx|
84f2:   f0 0f b1 17 lock cmpxchg %edx,(%rdi)|
84f6:   39 c8   cmp%ecx,%eax|
84f8:   74 09   je 8503  |
84fa:   85 c0   test   %eax,%eax|
84fc:   74 0c   je 850a  |
84fe:   83 f8 ffcmp$0x,%eax |
8501:   75 e3   jne84e6  |
8503:   b8 01 00 00 00  mov$0x1,%eax|

8508:   5d  pop%rbp
8509:   c3  retq   
850a:   31 c0   xor%eax,%eax
850c:   5d  pop%rbp
850d:   c3  retq   


(I've annotated the fastpath impact with '|'. Out of line code generally does 
not 
count.)

It's about ~50 bytes of code per usage site. It might be better in some cases, 
but 
not by much.

This is way above any sane inlining threshold. The 'unconditionally good' 
inlining 
threshold is at 1-2 instructions and below ~10 bytes of code.

So for example refcount_set() and refcount_read() can stay inlined:

static inline void refcount_set(refcount_t *r, unsigned int n)
{
atomic_set(>refs, n);
}

static inline unsigned int refcount_read(const refcount_t *r)
{
return atomic_read(>refs);
}


... beacuse they compile into a single instruction with 2-5 bytes I$ overhead.

Thanks,

Ingo

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Ingo Molnar]

* Reshetova, Elena  wrote:

> > Subject: refcount: Out-of-line everything
> > From: Peter Zijlstra 
> > Date: Fri Feb 10 16:27:52 CET 2017
> > 
> > Linus asked to please make this real C code.
> 
> Perhaps a completely stupid question, but I am going to ask anyway since only 
> this way I can learn. What a real difference it makes? Or are we talking more 
> about styling and etc.? Is it because of size concerns? This way it is 
> certainly 
> now done differently than rest of atomic and kref...

It's about generated code size mostly.

This inline function is way too large to be inlined:

static inline __refcount_check
bool refcount_add_not_zero(unsigned int i, refcount_t *r)
{
unsigned int old, new, val = atomic_read(>refs);

for (;;) {
if (!val)
return false;

if (unlikely(val == UINT_MAX))
return true;

new = val + i;
if (new < val)
new = UINT_MAX;
old = atomic_cmpxchg_relaxed(>refs, val, new);
if (old == val)
break;

val = old;
}

REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking 
memory.\n");

return true;
}

When used then this function generates this much code on x86-64 defconfig:

84d0 :
84d0:   8b 0f   mov(%rdi),%ecx
84d2:   55  push   %rbp
84d3:   48 89 e5mov%rsp,%rbp

84d6:   85 c9   test   %ecx,%ecx|
84d8:   74 30   je 850a  |
84da:   83 f9 ffcmp$0x,%ecx |
84dd:   be ff ff ff ff  mov$0x,%esi |
84e2:   75 04   jne84e8  |
84e4:   eb 1d   jmp8503  |
84e6:   89 c1   mov%eax,%ecx|
84e8:   8d 51 01lea0x1(%rcx),%edx   |
84eb:   89 c8   mov%ecx,%eax|
84ed:   39 ca   cmp%ecx,%edx|
84ef:   0f 42 d6cmovb  %esi,%edx|
84f2:   f0 0f b1 17 lock cmpxchg %edx,(%rdi)|
84f6:   39 c8   cmp%ecx,%eax|
84f8:   74 09   je 8503  |
84fa:   85 c0   test   %eax,%eax|
84fc:   74 0c   je 850a  |
84fe:   83 f8 ffcmp$0x,%eax |
8501:   75 e3   jne84e6  |
8503:   b8 01 00 00 00  mov$0x1,%eax|

8508:   5d  pop%rbp
8509:   c3  retq   
850a:   31 c0   xor%eax,%eax
850c:   5d  pop%rbp
850d:   c3  retq   


(I've annotated the fastpath impact with '|'. Out of line code generally does 
not 
count.)

It's about ~50 bytes of code per usage site. It might be better in some cases, 
but 
not by much.

This is way above any sane inlining threshold. The 'unconditionally good' 
inlining 
threshold is at 1-2 instructions and below ~10 bytes of code.

So for example refcount_set() and refcount_read() can stay inlined:

static inline void refcount_set(refcount_t *r, unsigned int n)
{
atomic_set(>refs, n);
}

static inline unsigned int refcount_read(const refcount_t *r)
{
return atomic_read(>refs);
}


... beacuse they compile into a single instruction with 2-5 bytes I$ overhead.

Thanks,

Ingo

RE: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Reshetova, Elena]

> Subject: refcount: Out-of-line everything
> From: Peter Zijlstra 
> Date: Fri Feb 10 16:27:52 CET 2017
> 
> Linus asked to please make this real C code.

Perhaps a completely stupid question, but I am going to ask anyway since only 
this way I can learn.
What a real difference it makes? Or are we talking more about styling and etc.? 
Is it because of size concerns?
This way it is certainly now done differently than rest of atomic and kref... 

Best Regards,
Elena.

RE: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Reshetova, Elena]

> Subject: refcount: Out-of-line everything
> From: Peter Zijlstra 
> Date: Fri Feb 10 16:27:52 CET 2017
> 
> Linus asked to please make this real C code.

Perhaps a completely stupid question, but I am going to ask anyway since only 
this way I can learn.
What a real difference it makes? Or are we talking more about styling and etc.? 
Is it because of size concerns?
This way it is certainly now done differently than rest of atomic and kref... 

Best Regards,
Elena.

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Linus Torvalds]

On Mon, Feb 13, 2017 at 10:00 AM, Peter Zijlstra  wrote:
>
> I think Linus' email ended up being private; not much discussion other
> than him saying he would like to see this.

I ended up reacting to the tipbot email, so I just replied to whoever
were on the cc list there, I guess.

  Linus

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Linus Torvalds]

On Mon, Feb 13, 2017 at 10:00 AM, Peter Zijlstra  wrote:
>
> I think Linus' email ended up being private; not much discussion other
> than him saying he would like to see this.

I ended up reacting to the tipbot email, so I just replied to whoever
were on the cc list there, I guess.

  Linus

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Ingo Molnar]

* Peter Zijlstra  wrote:

> > > And since size then isn't an issue what so ever anymore, remove the debug 
> > > knob and make all WARN()s unconditional.
> > 
> > Are you still going to land the x86 WARN_ON improvements?
> 
> Yes, once I manage to eke some response out of the relevant arch maintainers 
> on 
> the subject ;-)

so I'm enthusiastically in support of it all! Does that count as a response? ;-)

Thanks,

Ingo

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Ingo Molnar]

* Peter Zijlstra  wrote:

> > > And since size then isn't an issue what so ever anymore, remove the debug 
> > > knob and make all WARN()s unconditional.
> > 
> > Are you still going to land the x86 WARN_ON improvements?
> 
> Yes, once I manage to eke some response out of the relevant arch maintainers 
> on 
> the subject ;-)

so I'm enthusiastically in support of it all! Does that count as a response? ;-)

Thanks,

Ingo

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Peter Zijlstra]

On Mon, Feb 13, 2017 at 09:48:42AM -0800, Kees Cook wrote:
> On Mon, Feb 13, 2017 at 6:34 AM, Peter Zijlstra  wrote:

> > Linus asked to please make this real C code.
> 
> No objection from me, but I'm curious to see the conversation. Where
> did this discussion happen? (I'm curious to see the reasoning behind
> the decisions about the various trade-offs.)

I think Linus' email ended up being private; not much discussion other
than him saying he would like to see this.

Given the current state of code generation I wasn't in a state to argue
much. We can always revisit later.

> > And since size then isn't an issue what so ever anymore, remove the
> > debug knob and make all WARN()s unconditional.
> 
> Are you still going to land the x86 WARN_ON improvements?

Yes, once I manage to eke some response out of the relevant arch
maintainers on the subject ;-)

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Peter Zijlstra]

On Mon, Feb 13, 2017 at 09:48:42AM -0800, Kees Cook wrote:
> On Mon, Feb 13, 2017 at 6:34 AM, Peter Zijlstra  wrote:

> > Linus asked to please make this real C code.
> 
> No objection from me, but I'm curious to see the conversation. Where
> did this discussion happen? (I'm curious to see the reasoning behind
> the decisions about the various trade-offs.)

I think Linus' email ended up being private; not much discussion other
than him saying he would like to see this.

Given the current state of code generation I wasn't in a state to argue
much. We can always revisit later.

> > And since size then isn't an issue what so ever anymore, remove the
> > debug knob and make all WARN()s unconditional.
> 
> Are you still going to land the x86 WARN_ON improvements?

Yes, once I manage to eke some response out of the relevant arch
maintainers on the subject ;-)

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Kees Cook]

On Mon, Feb 13, 2017 at 6:34 AM, Peter Zijlstra  wrote:
> On Fri, Feb 10, 2017 at 12:31:15AM -0800, tip-bot for Peter Zijlstra wrote:
>> Commit-ID:  f405df5de3170c00e5c54f8b7cf4766044a032ba
>> Gitweb: 
>> http://git.kernel.org/tip/f405df5de3170c00e5c54f8b7cf4766044a032ba
>> Author: Peter Zijlstra 
>> AuthorDate: Mon, 14 Nov 2016 18:06:19 +0100
>> Committer:  Ingo Molnar 
>> CommitDate: Fri, 10 Feb 2017 09:04:19 +0100
>>
>> refcount_t: Introduce a special purpose refcount type
>>
>> Provide refcount_t, an atomic_t like primitive built just for
>> refcounting.
>>
>> It provides saturation semantics such that overflow becomes impossible
>> and thereby 'spurious' use-after-free is avoided.
>>
>> Signed-off-by: Peter Zijlstra (Intel) 
>
> ---
> Subject: refcount: Out-of-line everything
> From: Peter Zijlstra 
> Date: Fri Feb 10 16:27:52 CET 2017
>
> Linus asked to please make this real C code.

No objection from me, but I'm curious to see the conversation. Where
did this discussion happen? (I'm curious to see the reasoning behind
the decisions about the various trade-offs.)

> And since size then isn't an issue what so ever anymore, remove the
> debug knob and make all WARN()s unconditional.

Are you still going to land the x86 WARN_ON improvements?

-Kees

-- 
Kees Cook
Pixel Security

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Kees Cook]

On Mon, Feb 13, 2017 at 6:34 AM, Peter Zijlstra  wrote:
> On Fri, Feb 10, 2017 at 12:31:15AM -0800, tip-bot for Peter Zijlstra wrote:
>> Commit-ID:  f405df5de3170c00e5c54f8b7cf4766044a032ba
>> Gitweb: 
>> http://git.kernel.org/tip/f405df5de3170c00e5c54f8b7cf4766044a032ba
>> Author: Peter Zijlstra 
>> AuthorDate: Mon, 14 Nov 2016 18:06:19 +0100
>> Committer:  Ingo Molnar 
>> CommitDate: Fri, 10 Feb 2017 09:04:19 +0100
>>
>> refcount_t: Introduce a special purpose refcount type
>>
>> Provide refcount_t, an atomic_t like primitive built just for
>> refcounting.
>>
>> It provides saturation semantics such that overflow becomes impossible
>> and thereby 'spurious' use-after-free is avoided.
>>
>> Signed-off-by: Peter Zijlstra (Intel) 
>
> ---
> Subject: refcount: Out-of-line everything
> From: Peter Zijlstra 
> Date: Fri Feb 10 16:27:52 CET 2017
>
> Linus asked to please make this real C code.

No objection from me, but I'm curious to see the conversation. Where
did this discussion happen? (I'm curious to see the reasoning behind
the decisions about the various trade-offs.)

> And since size then isn't an issue what so ever anymore, remove the
> debug knob and make all WARN()s unconditional.

Are you still going to land the x86 WARN_ON improvements?

-Kees

-- 
Kees Cook
Pixel Security

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Peter Zijlstra]

On Fri, Feb 10, 2017 at 12:31:15AM -0800, tip-bot for Peter Zijlstra wrote:
> Commit-ID:  f405df5de3170c00e5c54f8b7cf4766044a032ba
> Gitweb: http://git.kernel.org/tip/f405df5de3170c00e5c54f8b7cf4766044a032ba
> Author: Peter Zijlstra 
> AuthorDate: Mon, 14 Nov 2016 18:06:19 +0100
> Committer:  Ingo Molnar 
> CommitDate: Fri, 10 Feb 2017 09:04:19 +0100
> 
> refcount_t: Introduce a special purpose refcount type
> 
> Provide refcount_t, an atomic_t like primitive built just for
> refcounting.
> 
> It provides saturation semantics such that overflow becomes impossible
> and thereby 'spurious' use-after-free is avoided.
> 
> Signed-off-by: Peter Zijlstra (Intel) 

---
Subject: refcount: Out-of-line everything
From: Peter Zijlstra 
Date: Fri Feb 10 16:27:52 CET 2017

Linus asked to please make this real C code.

And since size then isn't an issue what so ever anymore, remove the
debug knob and make all WARN()s unconditional.

Cc: gre...@linuxfoundation.org
Cc: keesc...@chromium.org
Cc: dwind...@gmail.com
Cc: elena.reshet...@intel.com
Cc: ishkam...@gmail.com
Suggested-by: Linus Torvalds 
Signed-off-by: Peter Zijlstra (Intel) 
---
 include/linux/refcount.h |  279 ++-
 lib/Kconfig.debug|   13 --
 lib/Makefile |2 
 lib/refcount.c   |  267 
 4 files changed, 281 insertions(+), 280 deletions(-)

--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -1,55 +1,10 @@
 #ifndef _LINUX_REFCOUNT_H
 #define _LINUX_REFCOUNT_H
 
-/*
- * Variant of atomic_t specialized for reference counts.
- *
- * The interface matches the atomic_t interface (to aid in porting) but only
- * provides the few functions one should use for reference counting.
- *
- * It differs in that the counter saturates at UINT_MAX and will not move once
- * there. This avoids wrapping the counter and causing 'spurious'
- * use-after-free issues.
- *
- * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
- * and provide only what is strictly required for refcounts.
- *
- * The increments are fully relaxed; these will not provide ordering. The
- * rationale is that whatever is used to obtain the object we're increasing the
- * reference count on will provide the ordering. For locked data structures,
- * its the lock acquire, for RCU/lockless data structures its the dependent
- * load.
- *
- * Do note that inc_not_zero() provides a control dependency which will order
- * future stores against the inc, this ensures we'll never modify the object
- * if we did not in fact acquire a reference.
- *
- * The decrements will provide release order, such that all the prior loads and
- * stores will be issued before, it also provides a control dependency, which
- * will order us against the subsequent free().
- *
- * The control dependency is against the load of the cmpxchg (ll/sc) that
- * succeeded. This means the stores aren't fully ordered, but this is fine
- * because the 1->0 transition indicates no concurrency.
- *
- * Note that the allocator is responsible for ordering things between free()
- * and alloc().
- *
- */
-
 #include 
-#include 
 #include 
 #include 
 
-#ifdef CONFIG_DEBUG_REFCOUNT
-#define REFCOUNT_WARN(cond, str) WARN_ON(cond)
-#define __refcount_check   __must_check
-#else
-#define REFCOUNT_WARN(cond, str) (void)(cond)
-#define __refcount_check
-#endif
-
 typedef struct refcount_struct {
atomic_t refs;
 } refcount_t;
@@ -66,229 +21,21 @@ static inline unsigned int refcount_read
return atomic_read(>refs);
 }
 
-static inline __refcount_check
-bool refcount_add_not_zero(unsigned int i, refcount_t *r)
-{
-   unsigned int old, new, val = atomic_read(>refs);
-
-   for (;;) {
-   if (!val)
-   return false;
-
-   if (unlikely(val == UINT_MAX))
-   return true;
-
-   new = val + i;
-   if (new < val)
-   new = UINT_MAX;
-   old = atomic_cmpxchg_relaxed(>refs, val, new);
-   if (old == val)
-   break;
-
-   val = old;
-   }
-
-   REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking 
memory.\n");
-
-   return true;
-}
-
-static inline void refcount_add(unsigned int i, refcount_t *r)
-{
-   REFCOUNT_WARN(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; 
use-after-free.\n");
-}
-
-/*
- * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
- *
- * Provides no memory ordering, it is assumed the caller has guaranteed the
- * object memory to be stable (RCU, etc.). It does provide a control dependency
- * and thereby orders future stores. See the comment on top.
- */
-static inline __refcount_check
-bool 

Re: [tip:locking/core] refcount_t: Introduce a special purpose refcount type

[Peter Zijlstra]

On Fri, Feb 10, 2017 at 12:31:15AM -0800, tip-bot for Peter Zijlstra wrote:
> Commit-ID:  f405df5de3170c00e5c54f8b7cf4766044a032ba
> Gitweb: http://git.kernel.org/tip/f405df5de3170c00e5c54f8b7cf4766044a032ba
> Author: Peter Zijlstra 
> AuthorDate: Mon, 14 Nov 2016 18:06:19 +0100
> Committer:  Ingo Molnar 
> CommitDate: Fri, 10 Feb 2017 09:04:19 +0100
> 
> refcount_t: Introduce a special purpose refcount type
> 
> Provide refcount_t, an atomic_t like primitive built just for
> refcounting.
> 
> It provides saturation semantics such that overflow becomes impossible
> and thereby 'spurious' use-after-free is avoided.
> 
> Signed-off-by: Peter Zijlstra (Intel) 

---
Subject: refcount: Out-of-line everything
From: Peter Zijlstra 
Date: Fri Feb 10 16:27:52 CET 2017

Linus asked to please make this real C code.

And since size then isn't an issue what so ever anymore, remove the
debug knob and make all WARN()s unconditional.

Cc: gre...@linuxfoundation.org
Cc: keesc...@chromium.org
Cc: dwind...@gmail.com
Cc: elena.reshet...@intel.com
Cc: ishkam...@gmail.com
Suggested-by: Linus Torvalds 
Signed-off-by: Peter Zijlstra (Intel) 
---
 include/linux/refcount.h |  279 ++-
 lib/Kconfig.debug|   13 --
 lib/Makefile |2 
 lib/refcount.c   |  267 
 4 files changed, 281 insertions(+), 280 deletions(-)

--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -1,55 +1,10 @@
 #ifndef _LINUX_REFCOUNT_H
 #define _LINUX_REFCOUNT_H
 
-/*
- * Variant of atomic_t specialized for reference counts.
- *
- * The interface matches the atomic_t interface (to aid in porting) but only
- * provides the few functions one should use for reference counting.
- *
- * It differs in that the counter saturates at UINT_MAX and will not move once
- * there. This avoids wrapping the counter and causing 'spurious'
- * use-after-free issues.
- *
- * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
- * and provide only what is strictly required for refcounts.
- *
- * The increments are fully relaxed; these will not provide ordering. The
- * rationale is that whatever is used to obtain the object we're increasing the
- * reference count on will provide the ordering. For locked data structures,
- * its the lock acquire, for RCU/lockless data structures its the dependent
- * load.
- *
- * Do note that inc_not_zero() provides a control dependency which will order
- * future stores against the inc, this ensures we'll never modify the object
- * if we did not in fact acquire a reference.
- *
- * The decrements will provide release order, such that all the prior loads and
- * stores will be issued before, it also provides a control dependency, which
- * will order us against the subsequent free().
- *
- * The control dependency is against the load of the cmpxchg (ll/sc) that
- * succeeded. This means the stores aren't fully ordered, but this is fine
- * because the 1->0 transition indicates no concurrency.
- *
- * Note that the allocator is responsible for ordering things between free()
- * and alloc().
- *
- */
-
 #include 
-#include 
 #include 
 #include 
 
-#ifdef CONFIG_DEBUG_REFCOUNT
-#define REFCOUNT_WARN(cond, str) WARN_ON(cond)
-#define __refcount_check   __must_check
-#else
-#define REFCOUNT_WARN(cond, str) (void)(cond)
-#define __refcount_check
-#endif
-
 typedef struct refcount_struct {
atomic_t refs;
 } refcount_t;
@@ -66,229 +21,21 @@ static inline unsigned int refcount_read
return atomic_read(>refs);
 }
 
-static inline __refcount_check
-bool refcount_add_not_zero(unsigned int i, refcount_t *r)
-{
-   unsigned int old, new, val = atomic_read(>refs);
-
-   for (;;) {
-   if (!val)
-   return false;
-
-   if (unlikely(val == UINT_MAX))
-   return true;
-
-   new = val + i;
-   if (new < val)
-   new = UINT_MAX;
-   old = atomic_cmpxchg_relaxed(>refs, val, new);
-   if (old == val)
-   break;
-
-   val = old;
-   }
-
-   REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking 
memory.\n");
-
-   return true;
-}
-
-static inline void refcount_add(unsigned int i, refcount_t *r)
-{
-   REFCOUNT_WARN(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; 
use-after-free.\n");
-}
-
-/*
- * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
- *
- * Provides no memory ordering, it is assumed the caller has guaranteed the
- * object memory to be stable (RCU, etc.). It does provide a control dependency
- * and thereby orders future stores. See the comment on top.
- */
-static inline __refcount_check
-bool refcount_inc_not_zero(refcount_t *r)
-{
-   unsigned int old, new, val = atomic_read(>refs);
+extern __must_check bool refcount_add_not_zero(unsigned int i,