All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Menzel <pmenzel@molgen.mpg.de>
To: Yury Norov <yury.norov@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] lib/bitmap: Make parameter len unsigned
Date: Sat, 9 Jul 2022 14:27:45 +0200	[thread overview]
Message-ID: <8caea951-5781-92f5-0d12-bd0e66452b77@molgen.mpg.de> (raw)
In-Reply-To: <YshBa/87hSAZwIP3@yury-laptop>

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

      reply	other threads:[~2022-07-09 12:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8caea951-5781-92f5-0d12-bd0e66452b77@molgen.mpg.de \
    --to=pmenzel@molgen.mpg.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.