All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <chengang@emindsoft.com.cn>
To: Minchan Kim <minchan@kernel.org>
Cc: akpm@linux-foundation.org, vbabka@suse.cz,
	mgorman@techsingularity.net, mhocko@suse.com,
	gi-oh.kim@profitbricks.com, iamjoonsoo.kim@lge.com,
	hillf.zj@alibaba-inc.com, rientjes@google.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Chen Gang <gang.chen.5i5j@gmail.com>
Subject: Re: [PATCH] mm: migrate: Use bool instead of int for the return value of PageMovable
Date: Tue, 12 Jul 2016 03:47:42 +0800	[thread overview]
Message-ID: <5783F7DE.9020203@emindsoft.com.cn> (raw)
In-Reply-To: <20160711002605.GD31817@bbox>


On 7/11/16 08:26, Minchan Kim wrote:
> On Sat, Jul 09, 2016 at 11:55:04PM +0800, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> For pure bool function's return value, bool is a little better more or
>> less than int.
>>
>> And return boolean result directly, since 'if' statement is also for
>> boolean checking, and return boolean result, too.
> 
> I just wanted to consistent with other PageXXX flags functions, PageAnon,
> PageMappingFlags which returns int rather than bool. Although I agree bool
> is natural, I want to be consistent with others rather than breaking at
> the moment.
> 
> Maybe if you feel it's really helpful, you might be able to handle all
> of places I mentioned for better readability and keeping consistency.

OK, I guess, we can send another patch for include/linux/page-flags.h
for PageXXX.

> But I doubt it's worth.
> 

In our case, the 2 output size are same, but under x86_64, the insns are
different. After uses bool, it uses push/pop instead of branch, for me,
it should be a little better for catching.

The orig:

  0000000000001290 <PageMovable>:
      1290:       48 8b 47 08             mov    0x8(%rdi),%rax
      1294:       83 e0 03                and    $0x3,%eax
      1297:       48 83 f8 02             cmp    $0x2,%rax
      129b:       74 03                   je     12a0 <__SetPageMovable+0x12a0>
      129d:       31 c0                   xor    %eax,%eax
      129f:       c3                      retq
      12a0:       55                      push   %rbp
      12a1:       48 89 e5                mov    %rsp,%rbp
      12a4:       e8 00 00 00 00          callq  12a9 <__SetPageMovable+0x12a9>
      12a9:       48 85 c0                test   %rax,%rax
      12ac:       74 17                   je     12c5 <__SetPageMovable+0x12c5>
      12ae:       48 8b 50 68             mov    0x68(%rax),%rdx
      12b2:       48 85 d2                test   %rdx,%rdx
      12b5:       74 0e                   je     12c5 <__SetPageMovable+0x12c5>
      12b7:       48 83 7a 68 00          cmpq   $0x0,0x68(%rdx)
      12bc:       b8 01 00 00 00          mov    $0x1,%eax
      12c1:       74 02                   je     12c5 <__SetPageMovable+0x12c5>
      12c3:       5d                      pop    %rbp
      12c4:       c3                      retq
      12c5:       31 c0                   xor    %eax,%eax
      12c7:       5d                      pop    %rbp
      12c8:       c3                      retq
      12c9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

The new:

  0000000000001290 <PageMovable>:
      1290:       48 8b 47 08             mov    0x8(%rdi),%rax
      1294:       55                      push   %rbp
      1295:       48 89 e5                mov    %rsp,%rbp
      1298:       53                      push   %rbx
      1299:       31 db                   xor    %ebx,%ebx
      129b:       83 e0 03                and    $0x3,%eax
      129e:       48 83 f8 02             cmp    $0x2,%rax
      12a2:       74 05                   je     12a9 <__SetPageMovable+0x12a9>
      12a4:       89 d8                   mov    %ebx,%eax
      12a6:       5b                      pop    %rbx
      12a7:       5d                      pop    %rbp
      12a8:       c3                      retq
      12a9:       e8 00 00 00 00          callq  12ae <__SetPageMovable+0x12ae>
      12ae:       48 85 c0                test   %rax,%rax
      12b1:       74 f1                   je     12a4 <__SetPageMovable+0x12a4>
      12b3:       48 8b 40 68             mov    0x68(%rax),%rax
      12b7:       48 85 c0                test   %rax,%rax
      12ba:       74 e8                   je     12a4 <__SetPageMovable+0x12a4>
      12bc:       48 83 78 68 00          cmpq   $0x0,0x68(%rax)
      12c1:       0f 95 c3                setne  %bl
      12c4:       89 d8                   mov    %ebx,%eax
      12c6:       5b                      pop    %rbx
      12c7:       5d                      pop    %rbp
      12c8:       c3                      retq
      12c9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

Thanks.
-- 
Chen Gang (e??a??)

Managing Natural Environments is the Duty of Human Beings.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Chen Gang <chengang@emindsoft.com.cn>
To: Minchan Kim <minchan@kernel.org>
Cc: akpm@linux-foundation.org, vbabka@suse.cz,
	mgorman@techsingularity.net, mhocko@suse.com,
	gi-oh.kim@profitbricks.com, iamjoonsoo.kim@lge.com,
	hillf.zj@alibaba-inc.com, rientjes@google.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Chen Gang <gang.chen.5i5j@gmail.com>
Subject: Re: [PATCH] mm: migrate: Use bool instead of int for the return value of PageMovable
Date: Tue, 12 Jul 2016 03:47:42 +0800	[thread overview]
Message-ID: <5783F7DE.9020203@emindsoft.com.cn> (raw)
In-Reply-To: <20160711002605.GD31817@bbox>


On 7/11/16 08:26, Minchan Kim wrote:
> On Sat, Jul 09, 2016 at 11:55:04PM +0800, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> For pure bool function's return value, bool is a little better more or
>> less than int.
>>
>> And return boolean result directly, since 'if' statement is also for
>> boolean checking, and return boolean result, too.
> 
> I just wanted to consistent with other PageXXX flags functions, PageAnon,
> PageMappingFlags which returns int rather than bool. Although I agree bool
> is natural, I want to be consistent with others rather than breaking at
> the moment.
> 
> Maybe if you feel it's really helpful, you might be able to handle all
> of places I mentioned for better readability and keeping consistency.

OK, I guess, we can send another patch for include/linux/page-flags.h
for PageXXX.

> But I doubt it's worth.
> 

In our case, the 2 output size are same, but under x86_64, the insns are
different. After uses bool, it uses push/pop instead of branch, for me,
it should be a little better for catching.

The orig:

  0000000000001290 <PageMovable>:
      1290:       48 8b 47 08             mov    0x8(%rdi),%rax
      1294:       83 e0 03                and    $0x3,%eax
      1297:       48 83 f8 02             cmp    $0x2,%rax
      129b:       74 03                   je     12a0 <__SetPageMovable+0x12a0>
      129d:       31 c0                   xor    %eax,%eax
      129f:       c3                      retq
      12a0:       55                      push   %rbp
      12a1:       48 89 e5                mov    %rsp,%rbp
      12a4:       e8 00 00 00 00          callq  12a9 <__SetPageMovable+0x12a9>
      12a9:       48 85 c0                test   %rax,%rax
      12ac:       74 17                   je     12c5 <__SetPageMovable+0x12c5>
      12ae:       48 8b 50 68             mov    0x68(%rax),%rdx
      12b2:       48 85 d2                test   %rdx,%rdx
      12b5:       74 0e                   je     12c5 <__SetPageMovable+0x12c5>
      12b7:       48 83 7a 68 00          cmpq   $0x0,0x68(%rdx)
      12bc:       b8 01 00 00 00          mov    $0x1,%eax
      12c1:       74 02                   je     12c5 <__SetPageMovable+0x12c5>
      12c3:       5d                      pop    %rbp
      12c4:       c3                      retq
      12c5:       31 c0                   xor    %eax,%eax
      12c7:       5d                      pop    %rbp
      12c8:       c3                      retq
      12c9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

The new:

  0000000000001290 <PageMovable>:
      1290:       48 8b 47 08             mov    0x8(%rdi),%rax
      1294:       55                      push   %rbp
      1295:       48 89 e5                mov    %rsp,%rbp
      1298:       53                      push   %rbx
      1299:       31 db                   xor    %ebx,%ebx
      129b:       83 e0 03                and    $0x3,%eax
      129e:       48 83 f8 02             cmp    $0x2,%rax
      12a2:       74 05                   je     12a9 <__SetPageMovable+0x12a9>
      12a4:       89 d8                   mov    %ebx,%eax
      12a6:       5b                      pop    %rbx
      12a7:       5d                      pop    %rbp
      12a8:       c3                      retq
      12a9:       e8 00 00 00 00          callq  12ae <__SetPageMovable+0x12ae>
      12ae:       48 85 c0                test   %rax,%rax
      12b1:       74 f1                   je     12a4 <__SetPageMovable+0x12a4>
      12b3:       48 8b 40 68             mov    0x68(%rax),%rax
      12b7:       48 85 c0                test   %rax,%rax
      12ba:       74 e8                   je     12a4 <__SetPageMovable+0x12a4>
      12bc:       48 83 78 68 00          cmpq   $0x0,0x68(%rax)
      12c1:       0f 95 c3                setne  %bl
      12c4:       89 d8                   mov    %ebx,%eax
      12c6:       5b                      pop    %rbx
      12c7:       5d                      pop    %rbp
      12c8:       c3                      retq
      12c9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

  reply	other threads:[~2016-07-11 19:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-09 15:55 [PATCH] mm: migrate: Use bool instead of int for the return value of PageMovable chengang
2016-07-09 15:55 ` chengang
2016-07-11  0:26 ` Minchan Kim
2016-07-11  0:26   ` Minchan Kim
2016-07-11 19:47   ` Chen Gang [this message]
2016-07-11 19:47     ` Chen Gang
2016-07-12  7:15     ` Vlastimil Babka
2016-07-12  7:15       ` Vlastimil Babka
2016-07-12 16:42       ` Chen Gang
2016-07-12 16:42         ` Chen Gang
2016-07-12  7:48     ` Michal Hocko
2016-07-12  7:48       ` Michal Hocko
2016-07-12 16:50       ` Chen Gang
2016-07-12 16:50         ` Chen Gang
2016-07-13  7:53         ` Michal Hocko
2016-07-13  7:53           ` Michal Hocko
2016-07-17  0:51           ` Chen Gang
2016-07-17  0:51             ` Chen Gang
2016-07-13 14:26         ` 回复:[PATCH] " 陈刚

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=5783F7DE.9020203@emindsoft.com.cn \
    --to=chengang@emindsoft.com.cn \
    --cc=akpm@linux-foundation.org \
    --cc=gang.chen.5i5j@gmail.com \
    --cc=gi-oh.kim@profitbricks.com \
    --cc=hillf.zj@alibaba-inc.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --cc=minchan@kernel.org \
    --cc=rientjes@google.com \
    --cc=vbabka@suse.cz \
    /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.