lkml.org 
[lkml]   [2018]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH]cgroup: __cpuset_node_allowed return bool
Hi Tejun,

inline.

On 2018-03-26 10:12 PM, Tejun Heo wrote:
> Hello, John.
>
> On Sat, Mar 24, 2018 at 01:05:50PM +0800, yuankuiz@codeaurora.org
> wrote:
>> From 304cec1cc42255fbd9e231a810f4eea20ab74b90 Mon Sep 17 00:00:00 2001
>> From: John Zhao <yuankuiz@codeaurora.org>
>> Date: Sat, 24 Mar 2018 13:01:32 +0800
>> Subject: [PATCH] cgroup: __cpuset_node_allowed return bool
>>
>> as a bool, __cpuset_node_allowed(...) return should be bool.
>
> So, as a minor cleanup patch, this is fine but can you please soften
> the commit title / description a bit? It doesn't have to be bool.
> int is fine. bool may be marginally more readable but that's about
> it, so let's please make the commit match that.
[ZJ] In detail, Considering the conversion after it could be compiled
into asm such as: // cross compile it was done by
"arm-linux-androideabi-gcc" on ubuntu
1) return int type variable in bool function:
bool enabled()
{
int ret = 1;
return ret;
}

/**
* ... ...
* mov r3, #1
* str r3, [fp, #-8]
* ldr r3, [fp, #-8]
* cmp r3, #0
* movne r3, #1
* moveq r3, #0
* uxtb r3, r3
* ... ...
*/

2)
bool enabled()
{
bool ret = 1;
return ret;
}

/**
* ... ...
* mov r3, #1
* strb r3, [fp, #-5]
* ldrb r3, [fp, #-5] @ zero_extendqisi2
* ... ...
*/

so the #1) style function can generate significant instructions than
the #2).
While, this is happened only when "-On" is not used with *-gcc
together. Though, it is oftern there, it is best to provide this with
decoupling of which option is used for optimization.

Situation is only nice to have this change as test_bit() is interpreted
in difference way in differece arch, which is "inline int" actually in
arm-arch. Which makes the situation is not the like like the general
case but needs to be checked and continued in the general include
section.

So mark this change as nice to have to keep the thing as simple as
possible as this is what can be found under /linux/kernel related to
this point.

> Thanks.

Thanks,
BR//Zhao

\
 
 \ /
  Last update: 2018-03-26 16:21    [W:1.456 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site