* [PATCH] bitops: simplify get_count_order_long() @ 2020-05-24 12:35 Wei Yang 2020-05-25 9:14 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2020-05-24 12:35 UTC (permalink / raw) To: akpm, andriy.shevchenko, christian.brauner; +Cc: linux-kernel, Wei Yang These two cases could be unified into one. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- include/linux/bitops.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/linux/bitops.h b/include/linux/bitops.h index 9acf654f0b19..5b5609e81a84 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -206,10 +206,7 @@ static inline int get_count_order_long(unsigned long l) { if (l == 0UL) return -1; - else if (l & (l - 1UL)) - return (int)fls_long(l); - else - return (int)fls_long(l) - 1; + return (int)fls_long(--l); } /** -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-24 12:35 [PATCH] bitops: simplify get_count_order_long() Wei Yang @ 2020-05-25 9:14 ` Andy Shevchenko 2020-05-25 14:43 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2020-05-25 9:14 UTC (permalink / raw) To: Wei Yang; +Cc: akpm, christian.brauner, linux-kernel On Sun, May 24, 2020 at 12:35:51PM +0000, Wei Yang wrote: > These two cases could be unified into one. Care to provide a test case which supports your change? > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > --- > include/linux/bitops.h | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index 9acf654f0b19..5b5609e81a84 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -206,10 +206,7 @@ static inline int get_count_order_long(unsigned long l) > { > if (l == 0UL) > return -1; > - else if (l & (l - 1UL)) > - return (int)fls_long(l); > - else > - return (int)fls_long(l) - 1; > + return (int)fls_long(--l); > } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 9:14 ` Andy Shevchenko @ 2020-05-25 14:43 ` Wei Yang 2020-05-25 15:32 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2020-05-25 14:43 UTC (permalink / raw) To: Andy Shevchenko; +Cc: Wei Yang, akpm, christian.brauner, linux-kernel On Mon, May 25, 2020 at 12:14:58PM +0300, Andy Shevchenko wrote: >On Sun, May 24, 2020 at 12:35:51PM +0000, Wei Yang wrote: >> These two cases could be unified into one. > >Care to provide a test case which supports your change? > Hmm.. where should I put the test? tools/testing/selftests/ ? >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> --- >> include/linux/bitops.h | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/include/linux/bitops.h b/include/linux/bitops.h >> index 9acf654f0b19..5b5609e81a84 100644 >> --- a/include/linux/bitops.h >> +++ b/include/linux/bitops.h >> @@ -206,10 +206,7 @@ static inline int get_count_order_long(unsigned long l) >> { >> if (l == 0UL) >> return -1; >> - else if (l & (l - 1UL)) >> - return (int)fls_long(l); >> - else >> - return (int)fls_long(l) - 1; >> + return (int)fls_long(--l); >> } > >-- >With Best Regards, >Andy Shevchenko > -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 14:43 ` Wei Yang @ 2020-05-25 15:32 ` Andy Shevchenko 2020-05-25 20:41 ` Andrew Morton 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2020-05-25 15:32 UTC (permalink / raw) To: Wei Yang, Jesse Brandeburg; +Cc: akpm, christian.brauner, linux-kernel On Mon, May 25, 2020 at 02:43:12PM +0000, Wei Yang wrote: > On Mon, May 25, 2020 at 12:14:58PM +0300, Andy Shevchenko wrote: > >On Sun, May 24, 2020 at 12:35:51PM +0000, Wei Yang wrote: > >> These two cases could be unified into one. > > > >Care to provide a test case which supports your change? > > > > Hmm.. where should I put the test? tools/testing/selftests/ ? I guess into test_bitops.c [1]? I though it eventually should make kernel, but I don't see it. Andrew, can you apply that or do you need Jesse to resend? [1]: https://lore.kernel.org/lkml/20200310221747.2848474-2-jesse.brandeburg@intel.com/ > > >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >> --- > >> include/linux/bitops.h | 5 +---- > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/include/linux/bitops.h b/include/linux/bitops.h > >> index 9acf654f0b19..5b5609e81a84 100644 > >> --- a/include/linux/bitops.h > >> +++ b/include/linux/bitops.h > >> @@ -206,10 +206,7 @@ static inline int get_count_order_long(unsigned long l) > >> { > >> if (l == 0UL) > >> return -1; > >> - else if (l & (l - 1UL)) > >> - return (int)fls_long(l); > >> - else > >> - return (int)fls_long(l) - 1; > >> + return (int)fls_long(--l); > >> } > > > >-- > >With Best Regards, > >Andy Shevchenko > > > > -- > Wei Yang > Help you, Help me -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 15:32 ` Andy Shevchenko @ 2020-05-25 20:41 ` Andrew Morton 2020-05-25 21:57 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: Andrew Morton @ 2020-05-25 20:41 UTC (permalink / raw) To: Andy Shevchenko Cc: Wei Yang, Jesse Brandeburg, christian.brauner, linux-kernel On Mon, 25 May 2020 18:32:16 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, May 25, 2020 at 02:43:12PM +0000, Wei Yang wrote: > > On Mon, May 25, 2020 at 12:14:58PM +0300, Andy Shevchenko wrote: > > >On Sun, May 24, 2020 at 12:35:51PM +0000, Wei Yang wrote: > > >> These two cases could be unified into one. > > > > > >Care to provide a test case which supports your change? I hurt my brain convincing myself, so I got practical: int fls(unsigned int x) { return x ? sizeof(x) * 8 - __builtin_clz(x) : 0; } static int get_count_order(unsigned l) { if (l == 0) return -1; else if (l & (l - 1UL)) return fls(l); else return fls(l) - 1; } static int get_count_order2(unsigned long l) { if (l == 0) return -1; return fls(--l); } main() { unsigned i; for (i = 1; i < 64; i++) { printf("%d %d\n", get_count_order(i), get_count_order2(i)); } } > > > > Hmm.. where should I put the test? tools/testing/selftests/ ? > > I guess into test_bitops.c [1]? I though it eventually should make kernel, but I don't see it. > > Andrew, can you apply that or do you need Jesse to resend? > Got it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 20:41 ` Andrew Morton @ 2020-05-25 21:57 ` Wei Yang 2020-05-25 22:31 ` Andrew Morton 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2020-05-25 21:57 UTC (permalink / raw) To: Andrew Morton Cc: Andy Shevchenko, Wei Yang, Jesse Brandeburg, christian.brauner, linux-kernel On Mon, May 25, 2020 at 01:41:10PM -0700, Andrew Morton wrote: >On Mon, 25 May 2020 18:32:16 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > >> On Mon, May 25, 2020 at 02:43:12PM +0000, Wei Yang wrote: >> > On Mon, May 25, 2020 at 12:14:58PM +0300, Andy Shevchenko wrote: >> > >On Sun, May 24, 2020 at 12:35:51PM +0000, Wei Yang wrote: >> > >> These two cases could be unified into one. >> > > >> > >Care to provide a test case which supports your change? > >I hurt my brain convincing myself, so I got practical: > > >int fls(unsigned int x) >{ > return x ? sizeof(x) * 8 - __builtin_clz(x) : 0; >} > >static int get_count_order(unsigned l) >{ > if (l == 0) > return -1; > else if (l & (l - 1UL)) > return fls(l); > else > return fls(l) - 1; >} > >static int get_count_order2(unsigned long l) >{ > if (l == 0) > return -1; > return fls(--l); >} > >main() >{ > unsigned i; > > for (i = 1; i < 64; i++) { > printf("%d %d\n", get_count_order(i), > get_count_order2(i)); > } >} > > >> > >> > Hmm.. where should I put the test? tools/testing/selftests/ ? >> >> I guess into test_bitops.c [1]? I though it eventually should make kernel, but I don't see it. >> >> Andrew, can you apply that or do you need Jesse to resend? >> > >Got it. I see the patch just merged, so I suppose to add the above test code into that one? -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 21:57 ` Wei Yang @ 2020-05-25 22:31 ` Andrew Morton 2020-05-27 22:45 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: Andrew Morton @ 2020-05-25 22:31 UTC (permalink / raw) To: Wei Yang Cc: Andy Shevchenko, Jesse Brandeburg, christian.brauner, linux-kernel On Mon, 25 May 2020 21:57:41 +0000 Wei Yang <richard.weiyang@gmail.com> wrote: > I see the patch just merged, so I suppose to add the above test code into that > one? Well, that's not really test code. But yes, something which tests both the 32-bit and 64-bit functions would be nice, sometime. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-25 22:31 ` Andrew Morton @ 2020-05-27 22:45 ` Wei Yang 2020-05-27 23:05 ` Andrew Morton 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2020-05-27 22:45 UTC (permalink / raw) To: Andrew Morton Cc: Wei Yang, Andy Shevchenko, Jesse Brandeburg, christian.brauner, linux-kernel On Mon, May 25, 2020 at 03:31:46PM -0700, Andrew Morton wrote: >On Mon, 25 May 2020 21:57:41 +0000 Wei Yang <richard.weiyang@gmail.com> wrote: > >> I see the patch just merged, so I suppose to add the above test code into that >> one? > >Well, that's not really test code. > >But yes, something which tests both the 32-bit and 64-bit functions would be >nice, sometime. Mimic the test_bitops.c, I wrote a test like this: /* a tiny module only meant to test get_count_order/long */ unsigned int order_comb[][2] = { {0x00000003, 2}, {0x00000004, 2}, {0x00001fff, 13}, {0x00002000, 13}, {0x50000000, 32}, {0x80000000, 32}, }; static int __init test_getorder_startup(void) { int i; for (i = 0; i < ARRAY_SIZE(order_comb); i++) { if (order_comb[i][1] != get_count_order(order_comb[i][0])) pr_warn("get_count_order wrong for %lx\n", order_comb[i][0]); } return 0; } Since I don't get a way to iterate all the possibilities, some random combination is chosen. Is this one looks good? -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-27 22:45 ` Wei Yang @ 2020-05-27 23:05 ` Andrew Morton 2020-05-28 22:18 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: Andrew Morton @ 2020-05-27 23:05 UTC (permalink / raw) To: Wei Yang Cc: Andy Shevchenko, Jesse Brandeburg, christian.brauner, linux-kernel On Wed, 27 May 2020 22:45:42 +0000 Wei Yang <richard.weiyang@gmail.com> wrote: > /* a tiny module only meant to test get_count_order/long */ > unsigned int order_comb[][2] = { > {0x00000003, 2}, > {0x00000004, 2}, > {0x00001fff, 13}, > {0x00002000, 13}, > {0x50000000, 32}, > {0x80000000, 32}, > }; > > static int __init test_getorder_startup(void) > { > int i; > > for (i = 0; i < ARRAY_SIZE(order_comb); i++) { > if (order_comb[i][1] != get_count_order(order_comb[i][0])) > pr_warn("get_count_order wrong for %lx\n", > order_comb[i][0]); > } > > return 0; > } > > Since I don't get a way to iterate all the possibilities, some random > combination is chosen. Is this one looks good? Looks good. You might want to add a less-negative number as well? 0x80030000. Something that won't turn positive if it has 1 subtracted from it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bitops: simplify get_count_order_long() 2020-05-27 23:05 ` Andrew Morton @ 2020-05-28 22:18 ` Wei Yang 0 siblings, 0 replies; 10+ messages in thread From: Wei Yang @ 2020-05-28 22:18 UTC (permalink / raw) To: Andrew Morton Cc: Wei Yang, Andy Shevchenko, Jesse Brandeburg, christian.brauner, linux-kernel On Wed, May 27, 2020 at 04:05:08PM -0700, Andrew Morton wrote: >On Wed, 27 May 2020 22:45:42 +0000 Wei Yang <richard.weiyang@gmail.com> wrote: > >> /* a tiny module only meant to test get_count_order/long */ >> unsigned int order_comb[][2] = { >> {0x00000003, 2}, >> {0x00000004, 2}, >> {0x00001fff, 13}, >> {0x00002000, 13}, >> {0x50000000, 32}, >> {0x80000000, 32}, >> }; >> >> static int __init test_getorder_startup(void) >> { >> int i; >> >> for (i = 0; i < ARRAY_SIZE(order_comb); i++) { >> if (order_comb[i][1] != get_count_order(order_comb[i][0])) >> pr_warn("get_count_order wrong for %lx\n", >> order_comb[i][0]); >> } >> >> return 0; >> } >> >> Since I don't get a way to iterate all the possibilities, some random >> combination is chosen. Is this one looks good? > >Looks good. > >You might want to add a less-negative number as well? 0x80030000. >Something that won't turn positive if it has 1 subtracted from it. Thanks, this is a good suggestion. -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-28 22:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-24 12:35 [PATCH] bitops: simplify get_count_order_long() Wei Yang 2020-05-25 9:14 ` Andy Shevchenko 2020-05-25 14:43 ` Wei Yang 2020-05-25 15:32 ` Andy Shevchenko 2020-05-25 20:41 ` Andrew Morton 2020-05-25 21:57 ` Wei Yang 2020-05-25 22:31 ` Andrew Morton 2020-05-27 22:45 ` Wei Yang 2020-05-27 23:05 ` Andrew Morton 2020-05-28 22:18 ` Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).