Skip to contentSkip to conversation

cgroups: add lock for child->cgroups in cgroup_post_fork()

PATCH In review Lai Jiangshan 3 participants 3 messages Nov 21, 2008 1 message not in the archive

lore
Share & tools
lore

Lai Jiangshan

author
#0 lore raw

PATCH 1/1 cgroups: add lock for child->cgroups in cgroup_post_fork()

when cgroup_post_fork() called, child is seen by find_task_by_vpid(), so child->cgroups maybe be changed, It'll incorrect.

child->cgroups<old>'s refcnt is decreased child->cgroups<new>'s refcnt is increased
but child->cg_list is added to child->cgroups<old>'s list.

Signed-off-by Lai Jiangshan <[email protected]>

---

kernel/cgroup.c +2 -0
index 358e775..ddc10ac 100644
@@ -2823,8 +2831,10 @@ void cgroup_post_fork(struct task_struct *child)
 {
 	if (use_task_css_set_links) {
 		write_lock(&css_set_lock);
+		task_lock(child);
 		if (list_empty(&child->cg_list))
 			list_add(&child->cg_list, &child->cgroups->tasks);
+		task_unlock(child);
 		write_unlock(&css_set_lock);
 	}
 }

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by
#2 lore raw
Quoted 8 lines · Lai Jiangshan

when cgroup_post_fork() called, child is seen by find_task_by_vpid(), so child->cgroups maybe be changed, It'll incorrect.

child->cgroups<old>'s refcnt is decreased child->cgroups<new>'s refcnt is increased
but child->cg_list is added to child->cgroups<old>'s list.

Signed-off-by Lai Jiangshan <[email protected]>
Reviewed-by Paul Menage <[email protected]>

Yeah, I guess this was a place where I equated "holding css_set_lock" with "cgroups can't change". But in fact they can partially change.

Paul

Reviewed-by on v1 1/1
#3 lore raw
In reply to Lai Jiangshan
Quoted 8 lines · Lai Jiangshan

when cgroup_post_fork() called, child is seen by find_task_by_vpid(), so child->cgroups maybe be changed, It'll incorrect.

child->cgroups<old>'s refcnt is decreased child->cgroups<new>'s refcnt is increased
but child->cg_list is added to child->cgroups<old>'s list.

Signed-off-by Lai Jiangshan <[email protected]>
Reviewed-by Paul Menage <[email protected]>

Yeah, I guess this was a place where I equated "holding css_set_lock" with "cgroups can't change". But in fact they can partially change.

Paul