linux-kernel - Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20161216073632.ma3fqcjhgzfyebog@phenom.ffwll.local>
Date:   Fri, 16 Dec 2016 08:36:32 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Christoph Hellwig <hch@....de>,
        Thomas Hellstrom <thellstrom@...are.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH] kref: prefer atomic_inc_not_zero to atomic_add_unless

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@...c4.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@...are.com>
> > Reviewed-by: Christoph Hellwig <hch@....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