[PATCH 1/2] cgroup: cgroup_subsys[ssid] is never NULL

[PATCH 1/2] cgroup: cgroup_subsys[ssid] is never NULL

[Wei Yang]

When the check, (ssid) < CGROUP_SUBSYS_COUNT, passed, it means
cgroup_subsys[ssid] is defined to its proper value. It is not
necessary to use a true to enter the loop.

Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 kernel/cgroup/cgroup-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index bfbeabc17a9d..0c5d1df6cdef 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -163,7 +163,7 @@ extern struct file_system_type cgroup_fs_type;
  */
 #define for_each_subsys(ss, ssid)					\
 	for ((ssid) = 0; (ssid) < CGROUP_SUBSYS_COUNT &&		\
-	     (((ss) = cgroup_subsys[ssid]) || true); (ssid)++)
+	     ((ss) = cgroup_subsys[ssid]); (ssid)++)
 
 static inline bool cgroup_is_dead(const struct cgroup *cgrp)
 {
-- 
2.33.1

[PATCH 2/2] cgroup: get the wrong css for css_alloc() during cgroup_init_subsys()

[Wei Yang]

css_alloc() needs the parent css, while cgroup_css() gets current
cgropu's css. So we are getting the wrong css during
cgroup_init_subsys().

Fortunately, cgrp_dfl_root.cgrp's css is not set yet, so the value we
pass to css_alloc() doesn't harm to the system.

Let's pass NULL directly during init, since we know there is no parent
yet.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 kernel/cgroup/cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index dffef5836ff7..452a723d4a36 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5709,7 +5709,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
 
 	/* Create the root cgroup state for this subsystem */
 	ss->root = &cgrp_dfl_root;
-	css = ss->css_alloc(cgroup_css(&cgrp_dfl_root.cgrp, ss));
+	css = ss->css_alloc(NULL);
 	/* We don't handle early failures gracefully */
 	BUG_ON(IS_ERR(css));
 	init_and_link_css(css, ss, &cgrp_dfl_root.cgrp);
-- 
2.33.1

Re: [PATCH 2/2] cgroup: get the wrong css for css_alloc() during cgroup_init_subsys()

[Tejun Heo]

On Sat, Nov 27, 2021 at 02:59:19PM +0000, Wei Yang wrote:
> css_alloc() needs the parent css, while cgroup_css() gets current
> cgropu's css. So we are getting the wrong css during
> cgroup_init_subsys().
> 
> Fortunately, cgrp_dfl_root.cgrp's css is not set yet, so the value we
> pass to css_alloc() doesn't harm to the system.
> 
> Let's pass NULL directly during init, since we know there is no parent
> yet.
> 
> Signed-off-by: Wei Yang <richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied to cgroup/for-5.17 w/ minor description adjustment.

Thanks.

-- 
tejun

Re: [PATCH 1/2] cgroup: cgroup_subsys[ssid] is never NULL

[Tejun Heo]

On Sat, Nov 27, 2021 at 02:59:18PM +0000, Wei Yang wrote:
> When the check, (ssid) < CGROUP_SUBSYS_COUNT, passed, it means
> cgroup_subsys[ssid] is defined to its proper value. It is not
> necessary to use a true to enter the loop.
...
>  #define for_each_subsys(ss, ssid)					\
>  	for ((ssid) = 0; (ssid) < CGROUP_SUBSYS_COUNT &&		\
> -	     (((ss) = cgroup_subsys[ssid]) || true); (ssid)++)
> +	     ((ss) = cgroup_subsys[ssid]); (ssid)++)

So, now the compiler has to test whether ss is NULL or not before each
iteration even though we know that it's never NULL. The whole point of that
"|| true" is telling the compiler that the pointer is never NULL.

Thanks.

-- 
tejun

Re: [PATCH 1/2] cgroup: cgroup_subsys[ssid] is never NULL

[Wei Yang]

On Mon, Nov 29, 2021 at 07:35:54AM -1000, Tejun Heo wrote:
>On Sat, Nov 27, 2021 at 02:59:18PM +0000, Wei Yang wrote:
>> When the check, (ssid) < CGROUP_SUBSYS_COUNT, passed, it means
>> cgroup_subsys[ssid] is defined to its proper value. It is not
>> necessary to use a true to enter the loop.
>...
>>  #define for_each_subsys(ss, ssid)					\
>>  	for ((ssid) = 0; (ssid) < CGROUP_SUBSYS_COUNT &&		\
>> -	     (((ss) = cgroup_subsys[ssid]) || true); (ssid)++)
>> +	     ((ss) = cgroup_subsys[ssid]); (ssid)++)
>
>So, now the compiler has to test whether ss is NULL or not before each
>iteration even though we know that it's never NULL. The whole point of that
>"|| true" is telling the compiler that the pointer is never NULL.
>

That's interesting. Thanks.

>Thanks.
>
>-- 
>tejun

-- 
Wei Yang
Help you, Help me