These two cases could be unified into one.
Signed-off-by: Wei Yang <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
On Mon, 25 May 2020 18:32:16 +0300 Andy Shevchenko <[email protected]> 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.
On Mon, May 25, 2020 at 01:41:10PM -0700, Andrew Morton wrote:
>On Mon, 25 May 2020 18:32:16 +0300 Andy Shevchenko <[email protected]> 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
On Mon, 25 May 2020 21:57:41 +0000 Wei Yang <[email protected]> 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.
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/[email protected]/
>
> >> Signed-off-by: Wei Yang <[email protected]>
> >> ---
> >> 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
On Mon, May 25, 2020 at 03:31:46PM -0700, Andrew Morton wrote:
>On Mon, 25 May 2020 21:57:41 +0000 Wei Yang <[email protected]> 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
On Wed, 27 May 2020 22:45:42 +0000 Wei Yang <[email protected]> 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.
On Wed, May 27, 2020 at 04:05:08PM -0700, Andrew Morton wrote:
>On Wed, 27 May 2020 22:45:42 +0000 Wei Yang <[email protected]> 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