as a bool, __cpuset_node_allowed(...) return should be bool.
Signed-off-by: John Zhao <[email protected]>
--- 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())
From 304cec1cc42255fbd9e231a810f4eea20ab74b90 Mon Sep 17 00:00:00 2001
From: John Zhao <[email protected]>
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 <[email protected]>
---
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
Hello, John.
On Sat, Mar 24, 2018 at 01:05:50PM +0800, [email protected] wrote:
> From 304cec1cc42255fbd9e231a810f4eea20ab74b90 Mon Sep 17 00:00:00 2001
> From: John Zhao <[email protected]>
> 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
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, [email protected]
> wrote:
>> From 304cec1cc42255fbd9e231a810f4eea20ab74b90 Mon Sep 17 00:00:00 2001
>> From: John Zhao <[email protected]>
>> 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
On Mon, Mar 26, 2018 at 10:20:43PM +0800, [email protected] 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
Hi Tejun,
inline.
On 2018-03-26 10:25 PM, Tejun Heo wrote:
> On Mon, Mar 26, 2018 at 10:20:43PM +0800, [email protected]
> 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.
Hi Tejun,
Additionally,
On 2018-03-26 10:37 PM, [email protected] wrote:
> Hi Tejun,
>
> inline.
>
> On 2018-03-26 10:25 PM, Tejun Heo wrote:
>> On Mon, Mar 26, 2018 at 10:20:43PM +0800, [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
BR//Zhao
On 2018-03-26 10:12 PM, Tejun Heo wrote:
> Hello, John.
>
> On Sat, Mar 24, 2018 at 01:05:50PM +0800, [email protected]
> 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 <[email protected]>
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 <[email protected]>
---
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