* [PATCH]cgroup: __cpuset_node_allowed return bool @ 2018-03-24 4:56 yuankuiz 2018-03-24 5:05 ` yuankuiz 0 siblings, 1 reply; 8+ messages in thread From: yuankuiz @ 2018-03-24 4:56 UTC (permalink / raw) To: cgroups, tj, lizefan, hannes; +Cc: linux-kernel, pkondeti 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()) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-24 4:56 [PATCH]cgroup: __cpuset_node_allowed return bool yuankuiz @ 2018-03-24 5:05 ` yuankuiz 2018-03-26 14:12 ` Tejun Heo 0 siblings, 1 reply; 8+ messages in thread From: yuankuiz @ 2018-03-24 5:05 UTC (permalink / raw) To: cgroups, tj, lizefan, hannes; +Cc: linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-24 5:05 ` yuankuiz @ 2018-03-26 14:12 ` Tejun Heo 2018-03-26 14:20 ` yuankuiz 2018-03-27 16:45 ` yuankuiz 0 siblings, 2 replies; 8+ messages in thread From: Tejun Heo @ 2018-03-26 14:12 UTC (permalink / raw) To: yuankuiz; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-26 14:12 ` Tejun Heo @ 2018-03-26 14:20 ` yuankuiz 2018-03-26 14:25 ` Tejun Heo 2018-03-27 16:45 ` yuankuiz 1 sibling, 1 reply; 8+ messages in thread From: yuankuiz @ 2018-03-26 14:20 UTC (permalink / raw) To: Tejun Heo; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-26 14:20 ` yuankuiz @ 2018-03-26 14:25 ` Tejun Heo 2018-03-26 14:37 ` yuankuiz 0 siblings, 1 reply; 8+ messages in thread From: Tejun Heo @ 2018-03-26 14:25 UTC (permalink / raw) To: yuankuiz; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-26 14:25 ` Tejun Heo @ 2018-03-26 14:37 ` yuankuiz 2018-03-26 14:41 ` yuankuiz 0 siblings, 1 reply; 8+ messages in thread From: yuankuiz @ 2018-03-26 14:37 UTC (permalink / raw) To: Tejun Heo; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-26 14:37 ` yuankuiz @ 2018-03-26 14:41 ` yuankuiz 0 siblings, 0 replies; 8+ messages in thread From: yuankuiz @ 2018-03-26 14:41 UTC (permalink / raw) To: Tejun Heo; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]cgroup: __cpuset_node_allowed return bool 2018-03-26 14:12 ` Tejun Heo 2018-03-26 14:20 ` yuankuiz @ 2018-03-27 16:45 ` yuankuiz 1 sibling, 0 replies; 8+ messages in thread From: yuankuiz @ 2018-03-27 16:45 UTC (permalink / raw) To: Tejun Heo; +Cc: cgroups, lizefan, hannes, linux-kernel, pkondeti, cgroups-owner 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-27 16:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-24 4:56 [PATCH]cgroup: __cpuset_node_allowed return bool yuankuiz 2018-03-24 5:05 ` yuankuiz 2018-03-26 14:12 ` Tejun Heo 2018-03-26 14:20 ` yuankuiz 2018-03-26 14:25 ` Tejun Heo 2018-03-26 14:37 ` yuankuiz 2018-03-26 14:41 ` yuankuiz 2018-03-27 16:45 ` yuankuiz
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.