Re: [PATCH] cgroup: fix fail path in cgroup_load_subsys()
From: Vladimir Davydov <hidden>
Date: 2013-12-12 18:37:41
Also in:
lkml
Subsystem:
the rest · Maintainer:
Linus Torvalds
On 12/12/2013 07:55 PM, Tejun Heo wrote:
On Thu, Dec 12, 2013 at 10:35:36AM -0500, Tejun Heo wrote:quoted
On Sat, Dec 07, 2013 at 11:58:05AM +0400, Vladimir Davydov wrote:quoted
We should not call cgroup_unload_subsys() if online_css() fails, because online_css() does not assign css to cgroup on failure, while offline_css() called from cgroup_unload_subsys() expects it is assigned. So let's call everything we need to rollback inline without involving cgroup_unload_subsys(). Signed-off-by: Vladimir Davydov <redacted> Cc: Tejun Heo <redacted> Cc: Li Zefan <redacted>Applied to cgroup/for-3.13-fixes.Scrap that. I don't think we can fail css_online for the root css in 3.13-fixes and this would cause conflict with 3.14, so let's not commit it to 3.13-fixes. Vladimir, can you please respin the patch on top of cgroup/for-3.14
No problem.
but make cgroup_unload_subsys() check whether css is actually there before calling offline so that it matches the usual offline path better?
You want me to add something like this to the patch, don't you?
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 402f7aa..bc03b97 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c@@ -4680,6 +4680,7 @@ EXPORT_SYMBOL_GPL(cgroup_load_subsys); void cgroup_unload_subsys(struct cgroup_subsys *ss) { struct cgrp_cset_link *link; + struct cgroup_subsys_state *css; BUG_ON(ss->module == NULL);
@@ -4693,7 +4694,9 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss) mutex_lock(&cgroup_mutex); mutex_lock(&cgroup_root_mutex); - offline_css(cgroup_css(cgroup_dummy_top, ss)); + css = cgroup_css(cgroup_dummy_top, ss); + if (css) + offline_css(css); /* deassign the subsys_id */ cgroup_subsys[ss->subsys_id] = NULL;
If so, I don't mind, but why? cgroup_unload_subsys() should never be called without properly initialized css. Thanks.