[PATCH]cgroup: __cpuset_node_allowed return bool

[PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

as a bool, __cpuset_node_allowed(...) return should be bool.

Signed-off-by: John Zhao <yuankuiz@codeaurora.org>

--- kernel/cgroup/cpuset.c.orig	2018-03-24 12:39:27.854178608 +0800
+++ kernel/cgroup/cpuset.c	2018-03-24 12:40:51.020708670 +0800
@@ -2552,7 +2552,7 @@ static struct cpuset *nearest_hardwall_a
  bool __cpuset_node_allowed(int node, gfp_t gfp_mask)
  {
  	struct cpuset *cs;		/* current cpuset ancestors */
-	int allowed;			/* is allocation in zone z allowed? */
+	bool allowed;			/* is allocation in zone z allowed? */
  	unsigned long flags;

  	if (in_interrupt())

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

 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.

Signed-off-by: John Zhao <yuankuiz@codeaurora.org>
---
  kernel/cgroup/cpuset.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b42037e..42338b7 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2552,7 +2552,7 @@ static struct cpuset 
*nearest_hardwall_ancestor(struct cpuset *cs)
  bool __cpuset_node_allowed(int node, gfp_t gfp_mask)
  {
  	struct cpuset *cs;		/* current cpuset ancestors */
-	int allowed;			/* is allocation in zone z allowed? */
+	bool allowed;			/* is allocation in zone z allowed? */
  	unsigned long flags;

  	if (in_interrupt())
-- 
2.7.4

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[Tejun Heo]

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.

Thanks.

-- 
tejun

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

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

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[Tejun Heo]

On Mon, Mar 26, 2018 at 10:20:43PM +0800, yuankuiz@codeaurora.org wrote:
>  1) return int type variable in bool function:
>  bool enabled()
>  {
>  	int ret = 1;
>  	return ret;
>  }
...
>  2)
>  bool enabled()
>  {
>  	bool ret = 1;
>  	return ret;
>  }
...
>  so the #1) style function can generate significant instructions
> than the #2).

That is a problem for the compiler, not the code.

>  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.

We don't want to dictate minute coding styles to avoid things which
are trivially optimized by compilers.

Thanks.

-- 
tejun

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

Hi Tejun,

     inline.

On 2018-03-26 10:25 PM, Tejun Heo wrote:
> On Mon, Mar 26, 2018 at 10:20:43PM +0800, yuankuiz@codeaurora.org 
> wrote:
>>  1) return int type variable in bool function:
>>  bool enabled()
>>  {
>>  	int ret = 1;
>>  	return ret;
>>  }
> ...
>>  2)
>>  bool enabled()
>>  {
>>  	bool ret = 1;
>>  	return ret;
>>  }
> ...
>>  so the #1) style function can generate significant instructions
>> than the #2).
> 
> That is a problem for the compiler, not the code.
> 
>>  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.
> 
> We don't want to dictate minute coding styles to avoid things which
> are trivially optimized by compilers.
[ZJ] Optimized by compiler is observed only. Such as it is not so big 
difference in x86-arch.

> 
> Thanks.

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

Hi Tejun,

     Additionally,

On 2018-03-26 10:37 PM, yuankuiz@codeaurora.org wrote:
> Hi Tejun,
> 
>     inline.
> 
> On 2018-03-26 10:25 PM, Tejun Heo wrote:
>> On Mon, Mar 26, 2018 at 10:20:43PM +0800, yuankuiz@codeaurora.org 
>> wrote:
>>>  1) return int type variable in bool function:
>>>  bool enabled()
>>>  {
>>>  	int ret = 1;
>>>  	return ret;
>>>  }
>> ...
>>>  2)
>>>  bool enabled()
>>>  {
>>>  	bool ret = 1;
>>>  	return ret;
>>>  }
>> ...
>>>  so the #1) style function can generate significant instructions
>>> than the #2).
>> 
>> That is a problem for the compiler, not the code.
[ZJ] Actually, it should be bool but not int. Without any optimization 
by compiler, it is the best if it is the same as returned.

>> 
>>>  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.
>> 
>> We don't want to dictate minute coding styles to avoid things which
>> are trivially optimized by compilers.
> [ZJ] Optimized by compiler is observed only. Such as it is not so big
> difference in x86-arch.
> 
>> 
>> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

     Thanks,
BR//Zhao

Re: [PATCH]cgroup: __cpuset_node_allowed return bool

[yuankuiz]

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:
>> 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] On summary, update it as below:

 From 6fd65547d4c704d86fd98cac1d9b8c74c8bee879 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: allowed of __cpuset_node_allowed to be bool

makes variable allowed returned by the __cpuset_node_allowed(...)
to be bool as it is assigned by the test_bits which always
return 0 or 1. In addition, it could save the size from
typical 4 bytes to 1 byte only. In another side, it could
save the potiential unnecessary instructionis during type conversion.

Signed-off-by: John Zhao <yuankuiz@codeaurora.org>
---
  kernel/cgroup/cpuset.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b42037e..42338b7 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2552,7 +2552,7 @@ static struct cpuset 
*nearest_hardwall_ancestor(struct cpuset *cs)
  bool __cpuset_node_allowed(int node, gfp_t gfp_mask)
  {
  	struct cpuset *cs;		/* current cpuset ancestors */
-	int allowed;			/* is allocation in zone z allowed? */
+	bool allowed;			/* is allocation in zone z allowed? */
  	unsigned long flags;

  	if (in_interrupt())
-- 
2.7.4