Skip to content

Commit

Permalink
mm: correctly synchronize rss-counters at exit/exec
Browse files Browse the repository at this point in the history
mm->rss_stat counters have per-task delta: task->rss_stat.  Before
changing task->mm pointer the kernel must flush this delta with
sync_mm_rss().

do_exit() already calls sync_mm_rss() to flush the rss-counters before
committing the rss statistics into task->signal->maxrss, taskstats,
audit and other stuff.  Unfortunately the kernel does this before
calling mm_release(), which can call put_user() for processing
task->clear_child_tid.  So at this point we can trigger page-faults and
task->rss_stat becomes non-zero again.  As a result mm->rss_stat becomes
inconsistent and check_mm() will print something like this:

| BUG: Bad rss-counter state mm:ffff88020813c380 idx:1 val:-1
| BUG: Bad rss-counter state mm:ffff88020813c380 idx:2 val:1

This patch moves sync_mm_rss() into mm_release(), and moves mm_release()
out of do_exit() and calls it earlier.  After mm_release() there should
be no pagefaults.

[[email protected]: tweak comment]
Signed-off-by: Konstantin Khlebnikov <[email protected]>
Reported-by: Markus Trippelsdorf <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: <[email protected]>		[3.4.x]
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
koct9i authored and torvalds committed Jun 7, 2012
1 parent 39caa09 commit 40af1bb
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
1 change: 0 additions & 1 deletion fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,6 @@ static int exec_mmap(struct mm_struct *mm)
/* Notify parent that we're no longer interested in the old VM */
tsk = current;
old_mm = current->mm;
sync_mm_rss(old_mm);
mm_release(tsk, old_mm);

if (old_mm) {
Expand Down
13 changes: 8 additions & 5 deletions kernel/exit.c
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ void daemonize(const char *name, ...)
* user space pages. We don't need them, and if we didn't close them
* they would be locked into memory.
*/
mm_release(current, current->mm);
exit_mm(current);
/*
* We don't want to get frozen, in case system-wide hibernation
Expand Down Expand Up @@ -640,7 +641,6 @@ static void exit_mm(struct task_struct * tsk)
struct mm_struct *mm = tsk->mm;
struct core_state *core_state;

mm_release(tsk, mm);
if (!mm)
return;
/*
Expand Down Expand Up @@ -960,9 +960,13 @@ void do_exit(long code)
preempt_count());

acct_update_integrals(tsk);
/* sync mm's RSS info before statistics gathering */
if (tsk->mm)
sync_mm_rss(tsk->mm);

/* Set exit_code before complete_vfork_done() in mm_release() */
tsk->exit_code = code;

/* Release mm and sync mm's RSS info before statistics gathering */
mm_release(tsk, tsk->mm);

group_dead = atomic_dec_and_test(&tsk->signal->live);
if (group_dead) {
hrtimer_cancel(&tsk->signal->real_timer);
Expand All @@ -975,7 +979,6 @@ void do_exit(long code)
tty_audit_exit();
audit_free(tsk);

tsk->exit_code = code;
taskstats_exit(tsk, group_dead);

exit_mm(tsk);
Expand Down
8 changes: 8 additions & 0 deletions kernel/fork.c
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,14 @@ void mmput(struct mm_struct *mm)
module_put(mm->binfmt->module);
mmdrop(mm);
}

/*
* Final rss-counter synchronization. After this point there must be
* no pagefaults into this mm from the current context. Otherwise
* mm->rss_stat will be inconsistent.
*/
if (mm)
sync_mm_rss(mm);
}
EXPORT_SYMBOL_GPL(mmput);

Expand Down

0 comments on commit 40af1bb

Please sign in to comment.