aboutsummaryrefslogtreecommitdiffstats
path: root/fs
diff options
context:
space:
mode:
authorAndrea Arcangeli <andrea@suse.de>2008-02-05 01:29:21 -0500
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2008-02-05 12:44:18 -0500
commit7766755a2f249e7e0dabc5255a0a3d151ff79821 (patch)
tree0f9d130d3f8107c77ed61b75f4745dc86e36d457 /fs
parent195cf453d2c3d789cbe80e3735755f860c2fb222 (diff)
Fix /proc dcache deadlock in do_exit
This patch fixes a sles9 system hang in start_this_handle from a customer with some heavy workload where all tasks are waiting on kjournald to commit the transaction, but kjournald waits on t_updates to go down to zero (it never does). This was reported as a lowmem shortage deadlock but when checking the debug data I noticed the VM wasn't under pressure at all (well it was really under vm pressure, because lots of tasks hanged in the VM prune_dcache methods trying to flush dirty inodes, but no task was hanging in GFP_NOFS mode, the holder of the journal handle should have if this was a vm issue in the first place). No task was apparently holding the leftover handle in the committing transaction, so I deduced t_updates was stuck to 1 because a journal_stop was never run by some path (this turned out to be correct). With a debug patch adding proper reverse links and stack trace logging in ext3 deployed in production, I found journal_stop is never run because mark_inode_dirty_sync is called inside release_task called by do_exit. (that was quite fun because I would have never thought about this subtleness, I thought a regular path in ext3 had a bug and it forgot to call journal_stop) do_exit->release_task->mark_inode_dirty_sync->schedule() (will never come back to run journal_stop) The reason is that shrink_dcache_parent is racy by design (feature not a bug) and it can do blocking I/O in some case, but the point is that calling shrink_dcache_parent at the last stage of do_exit isn't safe for self-reaping tasks. I guess the memory pressure of the unbalanced highmem system allowed to trigger this more easily. Now mainline doesn't have this line in iput (like sles9 has): if (inode->i_state & I_DIRTY_DELAYED) mark_inode_dirty_sync(inode); so it will probably not crash with ext3, but for example ext2 implements an I/O-blocking ext2_put_inode that will lead to similar screwups with ext2_free_blocks never coming back and it's definitely wrong to call blocking-IO paths inside do_exit. So this should fix a subtle bug in mainline too (not verified in practice though). The equivalent fix for ext3 is also not verified yet to fix the problem in sles9 but I don't have doubt it will (it usually takes days to crash, so it'll take weeks to be sure). An alternate fix would be to offload that work to a kernel thread, but I don't think a reschedule for this is worth it, the vm should be able to collect those entries for the synchronous release_task. Signed-off-by: Andrea Arcangeli <andrea@suse.de> Cc: Jan Kara <jack@ucw.cz> Cc: Ingo Molnar <mingo@elte.hu> Cc: "Eric W. Biederman" <ebiederm@xmission.com> Cc: Alexey Dobriyan <adobriyan@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/proc/base.c3
1 files changed, 2 insertions, 1 deletions
diff --git a/fs/proc/base.c b/fs/proc/base.c
index cd9f84c4bbf..c59852b3878 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2321,7 +2321,8 @@ static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
2321 name.len = snprintf(buf, sizeof(buf), "%d", pid); 2321 name.len = snprintf(buf, sizeof(buf), "%d", pid);
2322 dentry = d_hash_and_lookup(mnt->mnt_root, &name); 2322 dentry = d_hash_and_lookup(mnt->mnt_root, &name);
2323 if (dentry) { 2323 if (dentry) {
2324 shrink_dcache_parent(dentry); 2324 if (!(current->flags & PF_EXITING))
2325 shrink_dcache_parent(dentry);
2325 d_drop(dentry); 2326 d_drop(dentry);
2326 dput(dentry); 2327 dput(dentry);
2327 } 2328 }