2020-05-25 22:02:19

by Wei Yang

Subject: [PATCH] bitops: use the same mechanism for get_count_order[_long]

These two functions share the same logic.

Signed-off-by: Wei Yang <[email protected]>
---
include/linux/bitops.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 5b5609e81a84..80703ef27aee 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -188,12 +188,10 @@ static inline unsigned fls_long(unsigned long l)

static inline int get_count_order(unsigned int count)
{
- int order;
+ if (count == 0)
+ return -1;

- order = fls(count) - 1;
- if (count & (count - 1))
- order++;
- return order;
+ return fls(--count);
}

/**
--
2.23.0

2020-05-26 07:07:44

by Andy Shevchenko

Subject: Re: [PATCH] bitops: use the same mechanism for get_count_order[_long]

On Mon, May 25, 2020 at 09:59:58PM +0000, Wei Yang wrote:
> These two functions share the same logic.

So, same comment. Please, add test first, make sure it works on current kernel,
then after your patch applied, and send it as a series, thanks!

P.S. W/o test code looks good, but based on my experience I'm very suspicious
about "small" changes that may lead to big issues. Hope you understand my point.

> Signed-off-by: Wei Yang <[email protected]>
> ---
> include/linux/bitops.h | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index 5b5609e81a84..80703ef27aee 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -188,12 +188,10 @@ static inline unsigned fls_long(unsigned long l)
>
> static inline int get_count_order(unsigned int count)
> {
> - int order;
> + if (count == 0)
> + return -1;
>
> - order = fls(count) - 1;
> - if (count & (count - 1))
> - order++;
> - return order;
> + return fls(--count);
> }
>
> /**
> --
> 2.23.0
>

--
With Best Regards,
Andy Shevchenko

2020-05-27 01:02:58

by Wei Yang

Subject: Re: [PATCH] bitops: use the same mechanism for get_count_order[_long]

On Tue, May 26, 2020 at 10:04:57AM +0300, Andy Shevchenko wrote:
>On Mon, May 25, 2020 at 09:59:58PM +0000, Wei Yang wrote:
>> These two functions share the same logic.
>
>So, same comment. Please, add test first, make sure it works on current kernel,
>then after your patch applied, and send it as a series, thanks!
>
>P.S. W/o test code looks good, but based on my experience I'm very suspicious
>about "small" changes that may lead to big issues. Hope you understand my point.
>

Sure, I see your point.

>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> include/linux/bitops.h | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>> index 5b5609e81a84..80703ef27aee 100644
>> --- a/include/linux/bitops.h
>> +++ b/include/linux/bitops.h
>> @@ -188,12 +188,10 @@ static inline unsigned fls_long(unsigned long l)
>>
>> static inline int get_count_order(unsigned int count)
>> {
>> - int order;
>> + if (count == 0)
>> + return -1;
>>
>> - order = fls(count) - 1;
>> - if (count & (count - 1))
>> - order++;
>> - return order;
>> + return fls(--count);
>> }
>>
>> /**
>> --
>> 2.23.0
>>
>
>--
>With Best Regards,
>Andy Shevchenko
>

--
Wei Yang
Help you, Help me

2020-05-30 21:27:02

by Wei Yang

Subject: Re: [PATCH] bitops: use the same mechanism for get_count_order[_long]

Andrew,

Would you mind picking up this one? The test module doesn't show any warning
after this on applied.

On Mon, May 25, 2020 at 09:59:58PM +0000, Wei Yang wrote:
>These two functions share the same logic.
>
>Signed-off-by: Wei Yang <[email protected]>
>---
> include/linux/bitops.h | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
>diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>index 5b5609e81a84..80703ef27aee 100644
>--- a/include/linux/bitops.h
>+++ b/include/linux/bitops.h
>@@ -188,12 +188,10 @@ static inline unsigned fls_long(unsigned long l)
>
> static inline int get_count_order(unsigned int count)
> {
>- int order;
>+ if (count == 0)
>+ return -1;
>
>- order = fls(count) - 1;
>- if (count & (count - 1))
>- order++;
>- return order;
>+ return fls(--count);
> }
>
> /**
>--
>2.23.0

--
Wei Yang
Help you, Help me