Thread (7 messages) 7 messages, 3 authors, 2013-12-12

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help