2021-11-27 15:01:44

by Wei Yang

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

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 <[email protected]>
---
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

2021-11-27 15:01:49

by Wei Yang

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

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 <[email protected]>
---
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

2021-11-29 22:52:12

by Tejun Heo

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

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

2021-11-29 23:04:34

by Tejun Heo

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

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 <[email protected]>

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

Thanks.

--
tejun

2021-11-30 00:33:46

by Wei Yang

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

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