1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v3 0/2] bitmap: refine bitmap_set
  4. View patch

[Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly

Wei Yang posted 2 patches 6 years, 6 months ago
[Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Wei Yang 6 years, 6 months ago 
The value left in nr is the number of bits for the last word, which
could be calculate the last word mask directly.

Remove the unnecessary size.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

---
v2: refine bitmap_set_atomic too, suggested from Peter
---
 util/bitmap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/util/bitmap.c b/util/bitmap.c
index 1753ff7f5b..5b15249796 100644
--- a/util/bitmap.c
+++ b/util/bitmap.c
@@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 void bitmap_set(unsigned long *map, long start, long nr)
 {
     unsigned long *p = map + BIT_WORD(start);
-    const long size = start + nr;
     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
 
@@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
         p++;
     }
     if (nr) {
-        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
+        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
         *p |= mask_to_set;
     }
 }
@@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
 void bitmap_set_atomic(unsigned long *map, long start, long nr)
 {
     unsigned long *p = map + BIT_WORD(start);
-    const long size = start + nr;
     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
 
@@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
 
     /* Last word */
     if (nr) {
-        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
+        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
         atomic_or(p, mask_to_set);
     } else {
         /* If we avoided the full barrier in atomic_or(), issue a
@@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
 void bitmap_clear(unsigned long *map, long start, long nr)
 {
     unsigned long *p = map + BIT_WORD(start);
-    const long size = start + nr;
     int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
     unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
 
@@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
         p++;
     }
     if (nr) {
-        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
+        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
         *p &= ~mask_to_clear;
     }
 }
-- 
2.17.1
Re: [Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Wei Yang 6 years, 6 months ago 
On Thu, Jul 18, 2019 at 09:04:55AM +0800, Wei Yang wrote:
>The value left in nr is the number of bits for the last word, which
>could be calculate the last word mask directly.
>
>Remove the unnecessary size.
>

May I ask why Patch 2 is picked up, but this one is not?

>Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>---
>v2: refine bitmap_set_atomic too, suggested from Peter
>---
> util/bitmap.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/util/bitmap.c b/util/bitmap.c
>index 1753ff7f5b..5b15249796 100644
>--- a/util/bitmap.c
>+++ b/util/bitmap.c
>@@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
> void bitmap_set(unsigned long *map, long start, long nr)
> {
>     unsigned long *p = map + BIT_WORD(start);
>-    const long size = start + nr;
>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
> 
>@@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
>         p++;
>     }
>     if (nr) {
>-        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>+        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>         *p |= mask_to_set;
>     }
> }
>@@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
> void bitmap_set_atomic(unsigned long *map, long start, long nr)
> {
>     unsigned long *p = map + BIT_WORD(start);
>-    const long size = start + nr;
>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
> 
>@@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
> 
>     /* Last word */
>     if (nr) {
>-        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>+        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>         atomic_or(p, mask_to_set);
>     } else {
>         /* If we avoided the full barrier in atomic_or(), issue a
>@@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
> void bitmap_clear(unsigned long *map, long start, long nr)
> {
>     unsigned long *p = map + BIT_WORD(start);
>-    const long size = start + nr;
>     int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
>     unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
> 
>@@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>         p++;
>     }
>     if (nr) {
>-        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
>+        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
>         *p &= ~mask_to_clear;
>     }
> }
>-- 
>2.17.1
>

-- 
Wei Yang
Help you, Help me
Re: [Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Paolo Bonzini 6 years, 6 months ago 
On 21/07/19 02:33, Wei Yang wrote:
> On Thu, Jul 18, 2019 at 09:04:55AM +0800, Wei Yang wrote:
>> The value left in nr is the number of bits for the last word, which
>> could be calculate the last word mask directly.
>>
>> Remove the unnecessary size.
>>
> 
> May I ask why Patch 2 is picked up, but this one is not?

Tests are always good to have, this cleanup will wait for 4.2 but it's
in the queue.

Paolo

>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>
>> ---
>> v2: refine bitmap_set_atomic too, suggested from Peter
>> ---
>> util/bitmap.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/util/bitmap.c b/util/bitmap.c
>> index 1753ff7f5b..5b15249796 100644
>> --- a/util/bitmap.c
>> +++ b/util/bitmap.c
>> @@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
>> void bitmap_set(unsigned long *map, long start, long nr)
>> {
>>     unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>
>> @@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
>>         p++;
>>     }
>>     if (nr) {
>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>         *p |= mask_to_set;
>>     }
>> }
>> @@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
>> void bitmap_set_atomic(unsigned long *map, long start, long nr)
>> {
>>     unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>
>> @@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>
>>     /* Last word */
>>     if (nr) {
>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>         atomic_or(p, mask_to_set);
>>     } else {
>>         /* If we avoided the full barrier in atomic_or(), issue a
>> @@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>> void bitmap_clear(unsigned long *map, long start, long nr)
>> {
>>     unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>     int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
>>     unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
>>
>> @@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>>         p++;
>>     }
>>     if (nr) {
>> -        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
>>         *p &= ~mask_to_clear;
>>     }
>> }
>> -- 
>> 2.17.1
>>
>
Re: [Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Wei Yang 6 years, 6 months ago 
On Sun, Jul 21, 2019 at 07:27:14PM +0200, Paolo Bonzini wrote:
>On 21/07/19 02:33, Wei Yang wrote:
>> On Thu, Jul 18, 2019 at 09:04:55AM +0800, Wei Yang wrote:
>>> The value left in nr is the number of bits for the last word, which
>>> could be calculate the last word mask directly.
>>>
>>> Remove the unnecessary size.
>>>
>> 
>> May I ask why Patch 2 is picked up, but this one is not?
>
>Tests are always good to have, this cleanup will wait for 4.2 but it's
>in the queue.
>

Thanks :-)

>Paolo
>
>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>>
>>> ---
>>> v2: refine bitmap_set_atomic too, suggested from Peter
>>> ---
>>> util/bitmap.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/util/bitmap.c b/util/bitmap.c
>>> index 1753ff7f5b..5b15249796 100644
>>> --- a/util/bitmap.c
>>> +++ b/util/bitmap.c
>>> @@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
>>> void bitmap_set(unsigned long *map, long start, long nr)
>>> {
>>>     unsigned long *p = map + BIT_WORD(start);
>>> -    const long size = start + nr;
>>>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>>
>>> @@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
>>>         p++;
>>>     }
>>>     if (nr) {
>>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>>         *p |= mask_to_set;
>>>     }
>>> }
>>> @@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
>>> void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>> {
>>>     unsigned long *p = map + BIT_WORD(start);
>>> -    const long size = start + nr;
>>>     int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>>     unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>>
>>> @@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>>
>>>     /* Last word */
>>>     if (nr) {
>>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>>         atomic_or(p, mask_to_set);
>>>     } else {
>>>         /* If we avoided the full barrier in atomic_or(), issue a
>>> @@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>> void bitmap_clear(unsigned long *map, long start, long nr)
>>> {
>>>     unsigned long *p = map + BIT_WORD(start);
>>> -    const long size = start + nr;
>>>     int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
>>>     unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
>>>
>>> @@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>>>         p++;
>>>     }
>>>     if (nr) {
>>> -        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
>>> +        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
>>>         *p &= ~mask_to_clear;
>>>     }
>>> }
>>> -- 
>>> 2.17.1
>>>
>> 

-- 
Wei Yang
Help you, Help me
Re: [Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Paolo Bonzini 6 years, 5 months ago 
On 18/07/19 03:04, Wei Yang wrote:
> The value left in nr is the number of bits for the last word, which
> could be calculate the last word mask directly.
> 
> Remove the unnecessary size.

Hi,

the value left in nr is _not_ the number of bits for the last word if
the start and the end are in the same word.  For example, if start %
BITS_PER_LONG was 3 and nr == 1, you'd have:

- before the patch BITMAP_LAST_WORD_MASK(4)

- after the patch BITMAP_LAST_WORD_MASK(1)

Paolo

> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> 
> ---
> v2: refine bitmap_set_atomic too, suggested from Peter
> ---
>  util/bitmap.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/util/bitmap.c b/util/bitmap.c
> index 1753ff7f5b..5b15249796 100644
> --- a/util/bitmap.c
> +++ b/util/bitmap.c
> @@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
>  void bitmap_set(unsigned long *map, long start, long nr)
>  {
>      unsigned long *p = map + BIT_WORD(start);
> -    const long size = start + nr;
>      int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>      unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>  
> @@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
>          p++;
>      }
>      if (nr) {
> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>          *p |= mask_to_set;
>      }
>  }
> @@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
>  void bitmap_set_atomic(unsigned long *map, long start, long nr)
>  {
>      unsigned long *p = map + BIT_WORD(start);
> -    const long size = start + nr;
>      int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>      unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>  
> @@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>  
>      /* Last word */
>      if (nr) {
> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>          atomic_or(p, mask_to_set);
>      } else {
>          /* If we avoided the full barrier in atomic_or(), issue a
> @@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>  void bitmap_clear(unsigned long *map, long start, long nr)
>  {
>      unsigned long *p = map + BIT_WORD(start);
> -    const long size = start + nr;
>      int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
>      unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
>  
> @@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>          p++;
>      }
>      if (nr) {
> -        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
> +        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
>          *p &= ~mask_to_clear;
>      }
>  }
>
Re: [Qemu-devel] [PATCH v3 1/2] bitmap: get last word mask from nr directly
Posted by Wei Yang 6 years, 5 months ago 
On Mon, Aug 12, 2019 at 06:18:43PM +0200, Paolo Bonzini wrote:
>On 18/07/19 03:04, Wei Yang wrote:
>> The value left in nr is the number of bits for the last word, which
>> could be calculate the last word mask directly.
>> 
>> Remove the unnecessary size.
>
>Hi,
>
>the value left in nr is _not_ the number of bits for the last word if
>the start and the end are in the same word.  For example, if start %
>BITS_PER_LONG was 3 and nr == 1, you'd have:
>
>- before the patch BITMAP_LAST_WORD_MASK(4)
>
>- after the patch BITMAP_LAST_WORD_MASK(1)
>

You are right. I missed this case.

Thanks for pointing out.

>Paolo
>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> 
>> ---
>> v2: refine bitmap_set_atomic too, suggested from Peter
>> ---
>>  util/bitmap.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>> 
>> diff --git a/util/bitmap.c b/util/bitmap.c
>> index 1753ff7f5b..5b15249796 100644
>> --- a/util/bitmap.c
>> +++ b/util/bitmap.c
>> @@ -160,7 +160,6 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
>>  void bitmap_set(unsigned long *map, long start, long nr)
>>  {
>>      unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>      int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>      unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>  
>> @@ -174,7 +173,7 @@ void bitmap_set(unsigned long *map, long start, long nr)
>>          p++;
>>      }
>>      if (nr) {
>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>          *p |= mask_to_set;
>>      }
>>  }
>> @@ -182,7 +181,6 @@ void bitmap_set(unsigned long *map, long start, long nr)
>>  void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>  {
>>      unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>      int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
>>      unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
>>  
>> @@ -208,7 +206,7 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>  
>>      /* Last word */
>>      if (nr) {
>> -        mask_to_set &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_set &= BITMAP_LAST_WORD_MASK(nr);
>>          atomic_or(p, mask_to_set);
>>      } else {
>>          /* If we avoided the full barrier in atomic_or(), issue a
>> @@ -221,7 +219,6 @@ void bitmap_set_atomic(unsigned long *map, long start, long nr)
>>  void bitmap_clear(unsigned long *map, long start, long nr)
>>  {
>>      unsigned long *p = map + BIT_WORD(start);
>> -    const long size = start + nr;
>>      int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
>>      unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
>>  
>> @@ -235,7 +232,7 @@ void bitmap_clear(unsigned long *map, long start, long nr)
>>          p++;
>>      }
>>      if (nr) {
>> -        mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
>> +        mask_to_clear &= BITMAP_LAST_WORD_MASK(nr);
>>          *p &= ~mask_to_clear;
>>      }
>>  }
>> 
>

-- 
Wei Yang
Help you, Help me

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.