Skip to contentSkip to conversation

cgroup: fix spurious lockdep warning in cgroup_exit()

PATCH In review Li Zefan 2 participants 3 messages Mar 28, 2014 – Mar 29, 2014

Subsystems (4)
MM core Scheduler exec and binfmt cgroup
lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 cgroup: fix spurious lockdep warning in cgroup_exit()

cgroup_exit() is called in fork and exit path. If it's called in the failure path during fork, PF_EXITING isn't set, and then lockdep will complain.

Fix this by removing cgroup_exit() in that failure path. cgroup_fork() does nothing that needs cleanup.

Reported-by Sasha Levin <[email protected]>
Signed-off-by Li Zefan <[email protected]>
---
 kernel/cgroup.c | 3 +--
 kernel/fork.c   | 5 ++---
 2 files changed, 3 insertions(+), 5 deletions(-)
kernel/cgroup.c +1 -2
index 8f4ddbe..a53dbc6 100644
@@ -4339,8 +4339,7 @@ void cgroup_post_fork(struct task_struct *child)
  * notify_on_release(), then leave the task attached to the root cgroup in
  * each hierarchy for the remainder of its exit.  No need to bother with
  * init_css_set refcnting.  init_css_set never goes away and we can't race
- * with migration path - either PF_EXITING is visible to migration path or
- * @tsk never got on the tasklist.
+ * with migration path - PF_EXITING is visible to migration path.
  */
 void cgroup_exit(struct task_struct *tsk, int run_callbacks)
 {
kernel/fork.c +2 -3
index 3e02737..0477657 100644
@@ -1274,7 +1274,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	if (IS_ERR(p->mempolicy)) {
 		retval = PTR_ERR(p->mempolicy);
 		p->mempolicy = NULL;
-		goto bad_fork_cleanup_cgroup;
+		goto bad_fork_cleanup_threadgroup_lock;
 	}
 #endif
 #ifdef CONFIG_CPUSETS

@@ -1526,11 +1526,10 @@ bad_fork_cleanup_policy:
 	perf_event_free_task(p);
 #ifdef CONFIG_NUMA
 	mpol_put(p->mempolicy);
-bad_fork_cleanup_cgroup:
+bad_fork_cleanup_threadgroup_lock:
 #endif
 	if (clone_flags & CLONE_THREAD)
 		threadgroup_change_end(current);
-	cgroup_exit(p, 0);
 	delayacct_tsk_free(p);
 	module_put(task_thread_info(p)->exec_domain->module);
 bad_fork_cleanup_count:
Signature · 1 line
1.8.0.2
author
#1 lore raw

PATCH 2/2 cgroup: remove useless argument from cgroup_exit()

Signed-off-by Li Zefan <[email protected]>
---
 include/linux/cgroup.h | 4 ++--
 kernel/cgroup.c        | 5 ++---
 kernel/exit.c          | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)
include/linux/cgroup.h +2 -2
index acbb9a4..13ffb06 100644
@@ -33,7 +33,7 @@ extern int cgroup_init_early(void);
 extern int cgroup_init(void);
 extern void cgroup_fork(struct task_struct *p);
 extern void cgroup_post_fork(struct task_struct *p);
-extern void cgroup_exit(struct task_struct *p, int run_callbacks);
+extern void cgroup_exit(struct task_struct *p);
 extern int cgroupstats_build(struct cgroupstats *stats,
 				struct dentry *dentry);
 

@@ -848,7 +848,7 @@ static inline int cgroup_init_early(void) { return 0; }
 static inline int cgroup_init(void) { return 0; }
 static inline void cgroup_fork(struct task_struct *p) {}
 static inline void cgroup_post_fork(struct task_struct *p) {}
-static inline void cgroup_exit(struct task_struct *p, int callbacks) {}
+static inline void cgroup_exit(struct task_struct *p) {}
 
 static inline int cgroupstats_build(struct cgroupstats *stats,
 					struct dentry *dentry)
kernel/cgroup.c +2 -3
index a53dbc6..775a90d 100644
@@ -4324,7 +4324,6 @@ void cgroup_post_fork(struct task_struct *child)
 /**
  * cgroup_exit - detach cgroup from exiting task
  * @tsk: pointer to task_struct of exiting process
- * @run_callback: run exit callbacks?
  *
  * Description: Detach cgroup from @tsk and release it.
  *

@@ -4341,7 +4340,7 @@ void cgroup_post_fork(struct task_struct *child)
  * init_css_set refcnting.  init_css_set never goes away and we can't race
  * with migration path - PF_EXITING is visible to migration path.
  */
-void cgroup_exit(struct task_struct *tsk, int run_callbacks)
+void cgroup_exit(struct task_struct *tsk)
 {
 	struct cgroup_subsys *ss;
 	struct css_set *cset;

@@ -4363,7 +4362,7 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
 	cset = task_css_set(tsk);
 	RCU_INIT_POINTER(tsk->cgroups, &init_css_set);
 
-	if (run_callbacks && need_forkexit_callback) {
+	if (need_forkexit_callback) {
 		/* see cgroup_post_fork() for details */
 		for_each_subsys(ss, i) {
 			if (ss->exit) {
kernel/exit.c +1 -1
index c3af836..c2ae892 100644
@@ -797,7 +797,7 @@ void do_exit(long code)
 	 */
 	perf_event_exit_task(tsk);
 
-	cgroup_exit(tsk, 1);
+	cgroup_exit(tsk);
 
 	if (group_dead)
 		disassociate_ctty(1);
Signature · 1 line
1.8.0.2
maintainer
#2 lore raw
In reply to Li Zefan
Quoted 9 lines · Li Zefan

cgroup_exit() is called in fork and exit path. If it's called in the failure path during fork, PF_EXITING isn't set, and then lockdep will complain.

Fix this by removing cgroup_exit() in that failure path. cgroup_fork() does nothing that needs cleanup.

Reported-by Sasha Levin <[email protected]>
Signed-off-by Li Zefan <[email protected]>

Applied 1-2 to cgroup/for-3.15.

Thanks!

Signature · 1 line
tejun