Reported-by: Hugh Dickins <hugh@veritas.com>
There's a race between mm->owner assignment and try_to_unuse(). The condition
occurs when try_to_unuse() runs in parallel with an exiting task.
The race can be visualized below. To quote Hugh
"I don't think your careful alternation of CPU0/1 events at the end matters:
the swapoff CPU simply dereferences mm->owner after that task has gone"
But the alteration does help understand the race better (at-least for me :))
CPU0 CPU1
try_to_unuse
task 1 stars exiting look at mm = task1->mm
.. increment mm_users
task 1 exits
mm->owner needs to be updated, but
no new owner is found
(mm_users > 1, but no other task
has task->mm = task1->mm)
mm_update_next_owner() leaves
grace period
user count drops, call mmput(mm)
task 1 freed
dereferencing mm->owner fails
The fix is to notify the subsystem (via mm_owner_changed callback), if
no new owner is found by specifying the new task as NULL.
Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---
kernel/cgroup.c | 5 +++--
kernel/exit.c | 10 ++++++++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff -puN kernel/exit.c~mm-owner-fix-race-with-swap kernel/exit.c
--- linux-2.6.27-rc1/kernel/exit.c~mm-owner-fix-race-with-swap 2008-08-05 10:46:19.000000000 +0530
+++ linux-2.6.27-rc1-balbir/kernel/exit.c 2008-08-05 10:46:19.000000000 +0530
@@ -625,6 +625,16 @@ retry:
} while_each_thread(g, c);
read_unlock(&tasklist_lock);
+ /*
+ * We found no owner and mm_users > 1, this implies that
+ * we are most likely racing with swap (try_to_unuse())
+ * Mark owner as NULL, so that subsystems can understand
+ * the callback and take action
+ */
+ down_write(&mm->mmap_sem);
+ mm->owner = NULL;
+ cgroup_mm_owner_callbacks(mm->owner, NULL);
+ up_write(&mm->mmap_sem);
return;
assign_new_owner:
diff -L kernel/cgroup/.c -puN /dev/null /dev/null
diff -puN kernel/cgroup.c~mm-owner-fix-race-with-swap kernel/cgroup.c
--- linux-2.6.27-rc1/kernel/cgroup.c~mm-owner-fix-race-with-swap 2008-08-05 10:47:20.000000000 +0530
+++ linux-2.6.27-rc1-balbir/kernel/cgroup.c 2008-08-05 10:47:55.000000000 +0530
@@ -2740,14 +2740,15 @@ void cgroup_fork_callbacks(struct task_s
*/
void cgroup_mm_owner_callbacks(struct task_struct *old, struct task_struct *new)
{
- struct cgroup *oldcgrp, *newcgrp;
+ struct cgroup *oldcgrp, *newcgrp = NULL;
if (need_mm_owner_callback) {
int i;
for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
struct cgroup_subsys *ss = subsys[i];
oldcgrp = task_cgroup(old, ss->subsys_id);
- newcgrp = task_cgroup(new, ss->subsys_id);
+ if (new)
+ newcgrp = task_cgroup(new, ss->subsys_id);
if (oldcgrp == newcgrp)
continue;
if (ss->mm_owner_changed)
_
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
--
