* [PATCH v2] lib/bitmap: Make parameter len unsigned @ 2022-07-08 7:52 Paul Menzel 2022-07-08 14:38 ` Yury Norov 0 siblings, 1 reply; 3+ messages in thread From: Paul Menzel @ 2022-07-08 7:52 UTC (permalink / raw) To: Yury Norov, Andy Shevchenko, Rasmus Villemoes; +Cc: Paul Menzel, linux-kernel The length is non-negative, so make it unsigned. Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> --- v2: Update signature in header file include/linux/bitmap.h | 2 +- lib/bitmap.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 2e6cd5681040..feaf84cbc487 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -164,7 +164,7 @@ bool __bitmap_intersects(const unsigned long *bitmap1, bool __bitmap_subset(const unsigned long *bitmap1, const unsigned long *bitmap2, unsigned int nbits); int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits); -void __bitmap_set(unsigned long *map, unsigned int start, int len); +void __bitmap_set(unsigned long *map, unsigned int start, unsigned int len); void __bitmap_clear(unsigned long *map, unsigned int start, int len); unsigned long bitmap_find_next_zero_area_off(unsigned long *map, diff --git a/lib/bitmap.c b/lib/bitmap.c index b18e31ea6e66..0746beb336df 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -348,14 +348,14 @@ int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) } EXPORT_SYMBOL(__bitmap_weight); -void __bitmap_set(unsigned long *map, unsigned int start, int len) +void __bitmap_set(unsigned long *map, unsigned int start, unsigned int len) { unsigned long *p = map + BIT_WORD(start); const unsigned int size = start + len; int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG); unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start); - while (len - bits_to_set >= 0) { + while (len >= bits_to_set) { *p |= mask_to_set; len -= bits_to_set; bits_to_set = BITS_PER_LONG; -- 2.36.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] lib/bitmap: Make parameter len unsigned 2022-07-08 7:52 [PATCH v2] lib/bitmap: Make parameter len unsigned Paul Menzel @ 2022-07-08 14:38 ` Yury Norov 2022-07-09 12:27 ` Paul Menzel 0 siblings, 1 reply; 3+ messages in thread From: Yury Norov @ 2022-07-08 14:38 UTC (permalink / raw) To: Paul Menzel; +Cc: Andy Shevchenko, Rasmus Villemoes, linux-kernel On Fri, Jul 08, 2022 at 09:52:40AM +0200, Paul Menzel wrote: > The length is non-negative, so make it unsigned. > > Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> Hi Paul, Can you please tell more about your motivation for fixing __bitmap_set? The following __bitmap_clear has the same problem, and bitmap_parse{,_user}, and bitmap_print_to_pagebuf, and bitmap_parselist... Is there a particular problem that is resolved after fixing __bitmap_set()? I'm OK if this is a single patch, but for a cleanup work it would be more logical to clean everything in a single patch/series... Thanks, Yury > --- > v2: Update signature in header file > > include/linux/bitmap.h | 2 +- > lib/bitmap.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index 2e6cd5681040..feaf84cbc487 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -164,7 +164,7 @@ bool __bitmap_intersects(const unsigned long *bitmap1, > bool __bitmap_subset(const unsigned long *bitmap1, > const unsigned long *bitmap2, unsigned int nbits); > int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits); > -void __bitmap_set(unsigned long *map, unsigned int start, int len); > +void __bitmap_set(unsigned long *map, unsigned int start, unsigned int len); > void __bitmap_clear(unsigned long *map, unsigned int start, int len); > > unsigned long bitmap_find_next_zero_area_off(unsigned long *map, > diff --git a/lib/bitmap.c b/lib/bitmap.c > index b18e31ea6e66..0746beb336df 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -348,14 +348,14 @@ int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) > } > EXPORT_SYMBOL(__bitmap_weight); > > -void __bitmap_set(unsigned long *map, unsigned int start, int len) > +void __bitmap_set(unsigned long *map, unsigned int start, unsigned int len) > { > unsigned long *p = map + BIT_WORD(start); > const unsigned int size = start + len; > int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG); > unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start); > > - while (len - bits_to_set >= 0) { > + while (len >= bits_to_set) { > *p |= mask_to_set; > len -= bits_to_set; > bits_to_set = BITS_PER_LONG; > -- > 2.36.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] lib/bitmap: Make parameter len unsigned 2022-07-08 14:38 ` Yury Norov @ 2022-07-09 12:27 ` Paul Menzel 0 siblings, 0 replies; 3+ messages in thread From: Paul Menzel @ 2022-07-09 12:27 UTC (permalink / raw) To: Yury Norov; +Cc: Andy Shevchenko, Rasmus Villemoes, linux-kernel Dear Yuri, Thank you for your reply, and sorry for all the failure messages the first version created. Am 08.07.22 um 16:38 schrieb Yury Norov: > On Fri, Jul 08, 2022 at 09:52:40AM +0200, Paul Menzel wrote: >> The length is non-negative, so make it unsigned. >> >> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> > Can you please tell more about your motivation for fixing > __bitmap_set? It’s just about semantics(?) that a count can’t be negative, and only seems to generate tiny smaller code (less instructions): ``` __bitmap_set: __bitmap_set: movl %esi, %eax movl %esi, %eax movq %rdi, %r8 < movl %esi, %ecx movl %esi, %ecx movl %edx, %edi | movl %esi, %r8d > movl $64, %esi > andl $63, %ecx shrl $6, %eax shrl $6, %eax andl $63, %esi | movl %edx, %r9d movq $-1, %rdx | leaq (%rdi,%rax,8), %rax leaq (%r8,%rax,8), %rax | subl %ecx, %esi leal -64(%rsi,%rdi), %r8d | movq $-1, %rdi salq %cl, %rdx | salq %cl, %rdi testl %r8d, %r8d | cmpl %edx, %esi js .L88 | ja .L85 movl %r8d, %r9d < shrl $6, %r9d < leal 1(%r9), %esi < leaq (%rax,%rsi,8), %rsi < .L86: .L86: orq %rdx, (%rax) | subl %esi, %edx > orq %rdi, (%rax) > movl $64, %esi addq $8, %rax addq $8, %rax movq $-1, %rdx | movq $-1, %rdi cmpq %rsi, %rax | cmpl $63, %edx jne .L86 | ja .L86 sall $6, %r9d < subl %r9d, %r8d < .L85: .L85: testl %r8d, %r8d | testl %edx, %edx je .L84 je .L84 addl %edi, %ecx | leal (%r8,%r9), %ecx movq $-1, %rax | movq $-1, %rdx negl %ecx negl %ecx shrq %cl, %rax | shrq %cl, %rdx andq %rax, %rdx | andq %rdx, %rdi orq %rdx, (%rsi) | orq %rdi, (%rax) .L84: .L84: ret ret .L88: < movq %rax, %rsi < movl %edi, %r8d < jmp .L85 < .size __bitmap_set, .-__bitmap_set .size __bitmap_set, .-__bitmap_set .p2align 4 .p2align 4 .globl __bitmap_clear .globl __bitmap_clear .type __bitmap_clear, @function .type __bitmap_clear, @function ``` $ diff lib/bitmap.1.S lib/bitmap.2.S | diffstat unknown | 55 ++++++++++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 31 deletions(-) > The following __bitmap_clear has the same problem, and > bitmap_parse{,_user}, and bitmap_print_to_pagebuf, and > bitmap_parselist... Indeed. > Is there a particular problem that is resolved after fixing > __bitmap_set()? > > I'm OK if this is a single patch, but for a cleanup work it would be > more logical to clean everything in a single patch/series... If you agree, I can change the other places too. Kind regards, Paul ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-09 12:27 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-08 7:52 [PATCH v2] lib/bitmap: Make parameter len unsigned Paul Menzel 2022-07-08 14:38 ` Yury Norov 2022-07-09 12:27 ` Paul Menzel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®