Re: + find_bit-micro-optimise-find_next__bit.patch added to -mm tree

Re: + find_bit-micro-optimise-find_next__bit.patch added to -mm tree

[Yury Norov]

NACK.

I acked it in assumption that Matthew will send v2 that also fixes 
_find_next_bit_le() the same way.

Yury

On Thu, Jan 05, 2017 at 05:19:06PM -0800, akpm@linux-foundation.org wrote:
> 
> The patch titled
>      Subject: lib/find_bit.c: micro-optimise find_next_*_bit
> has been added to the -mm tree.  Its filename is
>      find_bit-micro-optimise-find_next__bit.patch
> 
> This patch should soon appear at
>     http://ozlabs.org/~akpm/mmots/broken-out/find_bit-micro-optimise-find_next__bit.patch
> and later at
>     http://ozlabs.org/~akpm/mmotm/broken-out/find_bit-micro-optimise-find_next__bit.patch
> 
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> 
> The -mm tree is included into linux-next and is updated
> there every 3-4 working days
> 
> ------------------------------------------------------
> From: Matthew Wilcox <mawilcox@microsoft.com>
> Subject: lib/find_bit.c: micro-optimise find_next_*_bit
> 
> This saves 20 bytes on my x86-64 build, mostly due to alignment
> considerations ...  I think it actually saves about five bytes of
> instructions.  There's really two parts to this commit.  First, the first
> half of the test: (!nbits || start >= nbits) is trivially a subset of the
> second half, since nbits and start are both unsigned.  Second, while
> looking at the disassembly, I noticed that GCC was predicting the branch
> taken.  Since this is a failure case, it's clearly the less likely of the
> two branches, so add an unlikely() to override GCC's heuristics.
> 
> Link: http://lkml.kernel.org/r/1482513603-9630-1-git-send-email-mawilcox@linuxonhyperv.com
> Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Acked-by: Yury Norov <ynorov@caviumnetworks.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  lib/find_bit.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -puN lib/find_bit.c~find_bit-micro-optimise-find_next__bit lib/find_bit.c
> --- a/lib/find_bit.c~find_bit-micro-optimise-find_next__bit
> +++ a/lib/find_bit.c
> @@ -33,7 +33,7 @@ static unsigned long _find_next_bit(cons
>  {
>  	unsigned long tmp;
>  
> -	if (!nbits || start >= nbits)
> +	if (unlikely(start >= nbits))
>  		return nbits;
>  
>  	tmp = addr[start / BITS_PER_LONG] ^ invert;
> _
> 
> Patches currently in -mm which might be from mawilcox@microsoft.com are
> 
> find_bit-micro-optimise-find_next__bit.patch
> reimplement-idr-and-ida-using-the-radix-tree-support-storing-null-in-the-idr.patch

RE: + find_bit-micro-optimise-find_next__bit.patch added to -mm tree

[Matthew Wilcox]

I have that patch in my tree (also touches tools/lib/); I wasn't expecting Andrew to pick it up yet.  I'll send v2 in just a minute that I assume Andrew can just replace this patch with.

> -----Original Message-----
> From: Yury Norov [mailto:ynorov@caviumnetworks.com]
> Sent: Friday, January 6, 2017 3:08 AM
> To: Matthew Wilcox <mawilcox@microsoft.com>; akpm@linux-foundation.org
> Cc: linux@rasmusvillemoes.dk; linux-arch@vger.kernel.org; mm-
> commits@vger.kernel.org
> Subject: Re: + find_bit-micro-optimise-find_next__bit.patch added to -mm tree
> 
> NACK.
> 
> I acked it in assumption that Matthew will send v2 that also fixes
> _find_next_bit_le() the same way.
> 
> Yury
> 
> On Thu, Jan 05, 2017 at 05:19:06PM -0800, akpm@linux-foundation.org wrote:
> >
> > The patch titled
> >      Subject: lib/find_bit.c: micro-optimise find_next_*_bit
> > has been added to the -mm tree.  Its filename is
> >      find_bit-micro-optimise-find_next__bit.patch
> >
> > This patch should soon appear at
> >
> https://na01.safelinks.protection.outlook.com/?url=http:%2F%2Fozlabs.org%2F
> ~akpm%2Fmmots%2Fbroken-out%2Ffind_bit-micro-optimise-
> find_next__bit.patch&data=02%7C01%7Cmawilcox%40microsoft.com%7C39c9
> 0703ef884b0abe9708d4360b2a5e%7C72f988bf86f141af91ab2d7cd011db47%
> 7C1%7C0%7C636192868992198567&sdata=I6VAxN4FZTNKaD5U%2FeCSKM5n
> CYiy02uDZI1cnAdk6vc%3D&reserved=0
> > and later at
> >
> https://na01.safelinks.protection.outlook.com/?url=http:%2F%2Fozlabs.org%2F
> ~akpm%2Fmmotm%2Fbroken-out%2Ffind_bit-micro-optimise-
> find_next__bit.patch&data=02%7C01%7Cmawilcox%40microsoft.com%7C39c9
> 0703ef884b0abe9708d4360b2a5e%7C72f988bf86f141af91ab2d7cd011db47%
> 7C1%7C0%7C636192868992198567&sdata=6Hp9fq8ZU4lsVu3%2FmZXKsBz2gj
> sEY0y5CZvWaijx9u8%3D&reserved=0
> >
> > Before you just go and hit "reply", please:
> >    a) Consider who else should be cc'ed
> >    b) Prefer to cc a suitable mailing list as well
> >    c) Ideally: find the original patch on the mailing list and do a
> >       reply-to-all to that, adding suitable additional cc's
> >
> > *** Remember to use Documentation/SubmitChecklist when testing your
> code ***
> >
> > The -mm tree is included into linux-next and is updated
> > there every 3-4 working days
> >
> > ------------------------------------------------------
> > From: Matthew Wilcox <mawilcox@microsoft.com>
> > Subject: lib/find_bit.c: micro-optimise find_next_*_bit
> >
> > This saves 20 bytes on my x86-64 build, mostly due to alignment
> > considerations ...  I think it actually saves about five bytes of
> > instructions.  There's really two parts to this commit.  First, the first
> > half of the test: (!nbits || start >= nbits) is trivially a subset of the
> > second half, since nbits and start are both unsigned.  Second, while
> > looking at the disassembly, I noticed that GCC was predicting the branch
> > taken.  Since this is a failure case, it's clearly the less likely of the
> > two branches, so add an unlikely() to override GCC's heuristics.
> >
> > Link:
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flkml.kerne
> l.org%2Fr%2F1482513603-9630-1-git-send-email-
> mawilcox%40linuxonhyperv.com&data=02%7C01%7Cmawilcox%40microsoft.c
> om%7C39c90703ef884b0abe9708d4360b2a5e%7C72f988bf86f141af91ab2d7c
> d011db47%7C1%7C0%7C636192868992198567&sdata=69Koc35BWkngs3CPH
> BsetOUPTzYsi2pHfMcgEluT2%2Fs%3D&reserved=0
> > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> > Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Acked-by: Yury Norov <ynorov@caviumnetworks.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> >  lib/find_bit.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff -puN lib/find_bit.c~find_bit-micro-optimise-find_next__bit lib/find_bit.c
> > --- a/lib/find_bit.c~find_bit-micro-optimise-find_next__bit
> > +++ a/lib/find_bit.c
> > @@ -33,7 +33,7 @@ static unsigned long _find_next_bit(cons
> >  {
> >  	unsigned long tmp;
> >
> > -	if (!nbits || start >= nbits)
> > +	if (unlikely(start >= nbits))
> >  		return nbits;
> >
> >  	tmp = addr[start / BITS_PER_LONG] ^ invert;
> > _
> >
> > Patches currently in -mm which might be from mawilcox@microsoft.com are
> >
> > find_bit-micro-optimise-find_next__bit.patch
> > reimplement-idr-and-ida-using-the-radix-tree-support-storing-null-in-the-
> idr.patch