Patch for drm-next WAS Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

Patch for drm-next WAS Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Thomas Hellstrom]

Dave,

Since kref_get_unless_zero() was brought in by drm, could we add this to
drm-next?

Thanks,
Thomas


On 06/30/2016 12:52 AM, Jason A. Donenfeld wrote:
> This was positively reviewed by maintainers but never picked up. Can
> someone queue this for 4.7 or 4.8?
>
> Thanks,
> Jason
>
> On Mon, Feb 1, 2016 at 10:53 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> This was positively reviewed but never picked up. Can someone queue
>> this for rc3?
>>
>> Thanks,
>> Jason
>>
>> On Sun, Oct 11, 2015 at 9:59 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>>> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>>>
>>>
>>> On 10/10/2015 12:56 PM, Jason A. Donenfeld wrote:
>>>> On most platforms, there exists this ifdef:
>>>>
>>>>  #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)
>>>>
>>>> This makes this patch functionally useless. However, on PPC, there is
>>>> actually an explicit definition of atomic_inc_not_zero with its own
>>>> assembly that is slightly more optimized than atomic_add_unless. So,
>>>> this patch changes kref to use atomic_inc_not_zero instead, for PPC and
>>>> any future platforms that might provide an explicit implementation.
>>>>
>>>> This also puts this usage of kref more in line with a verbatim reading
>>>> of the examples in Paul McKenney's paper [1] in the section titled "2.4
>>>> Atomic Counting With Check and Release Memory Barrier", which uses
>>>> atomic_inc_not_zero.
>>>>
>>>> [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__open-2Dstd.org_jtc1_sc22_wg21_docs_papers_2007_n2167.pdf&d=BQIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=z5Nd9sYiJMKiphNjyZp6XT5CbayXMBlcb903f260pDY&s=HEHX3CuXRs2GRRQWuC4Vef6iJMwdilKVRkiZgJpjEpA&e=
>>>>
>>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>>> ---
>>>>  include/linux/kref.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/kref.h b/include/linux/kref.h
>>>> index 484604d..83d1f94 100644
>>>> --- a/include/linux/kref.h
>>>> +++ b/include/linux/kref.h
>>>> @@ -166,6 +166,6 @@ static inline int kref_put_mutex(struct kref *kref,
>>>>   */
>>>>  static inline int __must_check kref_get_unless_zero(struct kref *kref)
>>>>  {
>>>> -     return atomic_add_unless(&kref->refcount, 1, 0);
>>>> +     return atomic_inc_not_zero(&kref->refcount);
>>>>  }
>>>>  #endif /* _KREF_H_ */


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Patch for drm-next WAS Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Daniel Vetter]

On Fri, Jul 01, 2016 at 09:08:34AM +0200, Thomas Hellstrom wrote:
> Dave,
> 
> Since kref_get_unless_zero() was brought in by drm, could we add this to
> drm-next?

Sure can do, but I can't find the raw patch anywhere (I suck, I know).
Care to resend?

Thanks, Daniel

> 
> Thanks,
> Thomas
> 
> 
> On 06/30/2016 12:52 AM, Jason A. Donenfeld wrote:
> > This was positively reviewed by maintainers but never picked up. Can
> > someone queue this for 4.7 or 4.8?
> >
> > Thanks,
> > Jason
> >
> > On Mon, Feb 1, 2016 at 10:53 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >> This was positively reviewed but never picked up. Can someone queue
> >> this for rc3?
> >>
> >> Thanks,
> >> Jason
> >>
> >> On Sun, Oct 11, 2015 at 9:59 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> >>> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> >>>
> >>>
> >>> On 10/10/2015 12:56 PM, Jason A. Donenfeld wrote:
> >>>> On most platforms, there exists this ifdef:
> >>>>
> >>>>  #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)
> >>>>
> >>>> This makes this patch functionally useless. However, on PPC, there is
> >>>> actually an explicit definition of atomic_inc_not_zero with its own
> >>>> assembly that is slightly more optimized than atomic_add_unless. So,
> >>>> this patch changes kref to use atomic_inc_not_zero instead, for PPC and
> >>>> any future platforms that might provide an explicit implementation.
> >>>>
> >>>> This also puts this usage of kref more in line with a verbatim reading
> >>>> of the examples in Paul McKenney's paper [1] in the section titled "2.4
> >>>> Atomic Counting With Check and Release Memory Barrier", which uses
> >>>> atomic_inc_not_zero.
> >>>>
> >>>> [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__open-2Dstd.org_jtc1_sc22_wg21_docs_papers_2007_n2167.pdf&d=BQIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=z5Nd9sYiJMKiphNjyZp6XT5CbayXMBlcb903f260pDY&s=HEHX3CuXRs2GRRQWuC4Vef6iJMwdilKVRkiZgJpjEpA&e=
> >>>>
> >>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >>>> ---
> >>>>  include/linux/kref.h | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/linux/kref.h b/include/linux/kref.h
> >>>> index 484604d..83d1f94 100644
> >>>> --- a/include/linux/kref.h
> >>>> +++ b/include/linux/kref.h
> >>>> @@ -166,6 +166,6 @@ static inline int kref_put_mutex(struct kref *kref,
> >>>>   */
> >>>>  static inline int __must_check kref_get_unless_zero(struct kref *kref)
> >>>>  {
> >>>> -     return atomic_add_unless(&kref->refcount, 1, 0);
> >>>> +     return atomic_inc_not_zero(&kref->refcount);
> >>>>  }
> >>>>  #endif /* _KREF_H_ */
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Patch for drm-next WAS Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Jason A. Donenfeld]

On Tue, Jul 12, 2016 at 2:28 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> Sure can do, but I can't find the raw patch anywhere (I suck, I know).
> Care to resend?

Hey sorry I missed this email requesting the actual patch. I reposted it here:

https://lkml.org/lkml/2016/12/14/814

[PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Jason A. Donenfeld]

On most platforms, there exists this ifdef:

 #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)

This makes this patch functionally useless. However, on PPC, there is
actually an explicit definition of atomic_inc_not_zero with its own
assembly that is slightly more optimized than atomic_add_unless. So,
this patch changes kref to use atomic_inc_not_zero instead, for PPC and
any future platforms that might provide an explicit implementation.

This also puts this usage of kref more in line with a verbatim reading
of the examples in Paul McKenney's paper [1] in the section titled "2.4
Atomic Counting With Check and Release Memory Barrier", which uses
atomic_inc_not_zero.

[1] http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2167.pdf

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
---
This was reviewed favorably 14 months ago but never picked up.
I'm resubmitting it now in hopes that you can finally queue it
up for 4.10.

 include/linux/kref.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kref.h b/include/linux/kref.h
index e15828fd71f1..62f0a84ae94e 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -133,6 +133,6 @@ static inline int kref_put_mutex(struct kref *kref,
  */
 static inline int __must_check kref_get_unless_zero(struct kref *kref)
 {
-	return atomic_add_unless(&kref->refcount, 1, 0);
+	return atomic_inc_not_zero(&kref->refcount);
 }
 #endif /* _KREF_H_ */
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Daniel Vetter]

On Thu, Dec 15, 2016 at 06:01:10AM +0100, Jason A. Donenfeld wrote:
> On most platforms, there exists this ifdef:
> 
>  #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)
> 
> This makes this patch functionally useless. However, on PPC, there is
> actually an explicit definition of atomic_inc_not_zero with its own
> assembly that is slightly more optimized than atomic_add_unless. So,
> this patch changes kref to use atomic_inc_not_zero instead, for PPC and
> any future platforms that might provide an explicit implementation.
> 
> This also puts this usage of kref more in line with a verbatim reading
> of the examples in Paul McKenney's paper [1] in the section titled "2.4
> Atomic Counting With Check and Release Memory Barrier", which uses
> atomic_inc_not_zero.
> 
> [1] http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2167.pdf
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

Applied to drm-misc, but for 4.11 since 4.10 is bugfixes only already.

Thanks, Daniel

> ---
> This was reviewed favorably 14 months ago but never picked up.
> I'm resubmitting it now in hopes that you can finally queue it
> up for 4.10.
> 
>  include/linux/kref.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/kref.h b/include/linux/kref.h
> index e15828fd71f1..62f0a84ae94e 100644
> --- a/include/linux/kref.h
> +++ b/include/linux/kref.h
> @@ -133,6 +133,6 @@ static inline int kref_put_mutex(struct kref *kref,
>   */
>  static inline int __must_check kref_get_unless_zero(struct kref *kref)
>  {
> -	return atomic_add_unless(&kref->refcount, 1, 0);
> +	return atomic_inc_not_zero(&kref->refcount);
>  }
>  #endif /* _KREF_H_ */
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Fwd: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Thomas Hellstrom]

By request forwarded patch

This is also
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

/Thomas


-------- Forwarded Message --------
Subject: 	[PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless
Date: 	Sat, 10 Oct 2015 12:56:34 +0200
From: 	Jason A. Donenfeld <Jason@zx2c4.com>
To: 	Dave Airlie <airlied@redhat.com>, Thomas Hellstrom
<thellstrom@vmware.com>, linux-kernel@vger.kernel.org
CC: 	Jason A. Donenfeld <Jason@zx2c4.com>



On most platforms, there exists this ifdef:

 #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)

This makes this patch functionally useless. However, on PPC, there is
actually an explicit definition of atomic_inc_not_zero with its own
assembly that is slightly more optimized than atomic_add_unless. So,
this patch changes kref to use atomic_inc_not_zero instead, for PPC and
any future platforms that might provide an explicit implementation.

This also puts this usage of kref more in line with a verbatim reading
of the examples in Paul McKenney's paper [1] in the section titled "2.4
Atomic Counting With Check and Release Memory Barrier", which uses
atomic_inc_not_zero.

[1] https://urldefense.proofpoint.com/v2/url?u=http-3A__open-2Dstd.org_jtc1_sc22_wg21_docs_papers_2007_n2167.pdf&d=BQIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=z5Nd9sYiJMKiphNjyZp6XT5CbayXMBlcb903f260pDY&s=HEHX3CuXRs2GRRQWuC4Vef6iJMwdilKVRkiZgJpjEpA&e= 

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 include/linux/kref.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kref.h b/include/linux/kref.h
index 484604d..83d1f94 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -166,6 +166,6 @@ static inline int kref_put_mutex(struct kref *kref,
  */
 static inline int __must_check kref_get_unless_zero(struct kref *kref)
 {
-	return atomic_add_unless(&kref->refcount, 1, 0);
+	return atomic_inc_not_zero(&kref->refcount);
 }
 #endif /* _KREF_H_ */
-- 
2.6.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Jason A. Donenfeld]

On most platforms, there exists this ifdef:

 #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)

This makes this patch functionally useless. However, on PPC, there is
actually an explicit definition of atomic_inc_not_zero with its own
assembly that is slightly more optimized than atomic_add_unless. So,
this patch changes kref to use atomic_inc_not_zero instead, for PPC and
any future platforms that might provide an explicit implementation.

This also puts this usage of kref more in line with a verbatim reading
of the examples in Paul McKenney's paper [1] in the section titled "2.4
Atomic Counting With Check and Release Memory Barrier", which uses
atomic_inc_not_zero.

[1] http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2167.pdf

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
Sorry to submit this again, but people keep reviewing it saying it's fine,
but then point to somebody else to actually merge this. At the end of the
chain of fingerpointing is usually Greg. "Just have Greg do it." At this
point I'm confused, but it's certainly been sufficiently reviewed and
accepted. So can one of you just respond saying "I'll take it!"

 include/linux/kref.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kref.h b/include/linux/kref.h
index e15828fd71f1..62f0a84ae94e 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -133,6 +133,6 @@ static inline int kref_put_mutex(struct kref *kref,
  */
 static inline int __must_check kref_get_unless_zero(struct kref *kref)
 {
-	return atomic_add_unless(&kref->refcount, 1, 0);
+	return atomic_inc_not_zero(&kref->refcount);
 }
 #endif /* _KREF_H_ */
-- 
2.11.0

Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Greg KH]

On Thu, Dec 15, 2016 at 07:55:54PM +0100, Jason A. Donenfeld wrote:
> On most platforms, there exists this ifdef:
> 
>  #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)
> 
> This makes this patch functionally useless. However, on PPC, there is
> actually an explicit definition of atomic_inc_not_zero with its own
> assembly that is slightly more optimized than atomic_add_unless. So,
> this patch changes kref to use atomic_inc_not_zero instead, for PPC and
> any future platforms that might provide an explicit implementation.
> 
> This also puts this usage of kref more in line with a verbatim reading
> of the examples in Paul McKenney's paper [1] in the section titled "2.4
> Atomic Counting With Check and Release Memory Barrier", which uses
> atomic_inc_not_zero.
> 
> [1] http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2167.pdf
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> Sorry to submit this again, but people keep reviewing it saying it's fine,
> but then point to somebody else to actually merge this. At the end of the
> chain of fingerpointing is usually Greg. "Just have Greg do it." At this
> point I'm confused, but it's certainly been sufficiently reviewed and
> accepted. So can one of you just respond saying "I'll take it!"

Well, the crazies over in drm land were the ones that merged this new
api, so they should be the ones responsible for it.  But that was way
back in 2012, odds are they don't remember it given the lunacy that is
their subsystem...

I'll take it after 4.10-rc1 is out, thanks.

greg k-h

Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Jason A. Donenfeld]

Hi Greg,

On Thu, Dec 15, 2016 at 8:10 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> I'll take it after 4.10-rc1 is out, thanks.

Thank you!

Jason

Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

[Daniel Vetter]

On Thu, Dec 15, 2016 at 11:10:49AM -0800, Greg KH wrote:
> On Thu, Dec 15, 2016 at 07:55:54PM +0100, Jason A. Donenfeld wrote:
> > On most platforms, there exists this ifdef:
> > 
> >  #define atomic_inc_not_zero(v) atomic_add_unless((v), 1, 0)
> > 
> > This makes this patch functionally useless. However, on PPC, there is
> > actually an explicit definition of atomic_inc_not_zero with its own
> > assembly that is slightly more optimized than atomic_add_unless. So,
> > this patch changes kref to use atomic_inc_not_zero instead, for PPC and
> > any future platforms that might provide an explicit implementation.
> > 
> > This also puts this usage of kref more in line with a verbatim reading
> > of the examples in Paul McKenney's paper [1] in the section titled "2.4
> > Atomic Counting With Check and Release Memory Barrier", which uses
> > atomic_inc_not_zero.
> > 
> > [1] http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2167.pdf
> > 
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> > Sorry to submit this again, but people keep reviewing it saying it's fine,
> > but then point to somebody else to actually merge this. At the end of the
> > chain of fingerpointing is usually Greg. "Just have Greg do it." At this
> > point I'm confused, but it's certainly been sufficiently reviewed and
> > accepted. So can one of you just respond saying "I'll take it!"
> 
> Well, the crazies over in drm land were the ones that merged this new
> api, so they should be the ones responsible for it.  But that was way
> back in 2012, odds are they don't remember it given the lunacy that is
> their subsystem...

We do, it's just that I couldn't find Jason's patch when Thomas reviewed
it and asked for a resend and it took Jason a while to do that ...

Maybe we even remember this api way too well, we're constantly adding new
users of it in drm ;-)

> I'll take it after 4.10-rc1 is out, thanks.

Oh, here's another resubmission of this patch. I've already applied this
to my 4.11 queue, will show up in linux-next as soon as -rc1 is out.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel