From b6fac63cc1f52ec27f29fe6c6c8494a2ffac33fd Mon Sep 17 00:00:00 2001 From: Wu Fengguang Date: Thu, 2 Apr 2009 16:56:34 -0700 Subject: vfs: skip I_CLEAR state inodes clear_inode() will switch inode state from I_FREEING to I_CLEAR, and do so _outside_ of inode_lock. So any I_FREEING testing is incomplete without a coupled testing of I_CLEAR. So add I_CLEAR tests to drop_pagecache_sb(), generic_sync_sb_inodes() and add_dquot_ref(). Masayoshi MIZUMA discovered the bug in drop_pagecache_sb() and Jan Kara reminds fixing the other two cases. Masayoshi MIZUMA has a nice panic flow: ===================================================================== [process A] | [process B] | | | prune_icache() | drop_pagecache() | spin_lock(&inode_lock) | drop_pagecache_sb() | inode->i_state |= I_FREEING; | | | spin_unlock(&inode_lock) | V | | | spin_lock(&inode_lock) | V | | | dispose_list() | | | list_del() | | | clear_inode() | | | inode->i_state = I_CLEAR | | | | | V | | | if (inode->i_state & (I_FREEING|I_WILL_FREE)) | | | continue; <==== NOT MATCH | | | | | | (DANGER from here on! Accessing disposing inode!) | | | | | | __iget() | | | list_move() <===== PANIC on poisoned list !! V V | (time) ===================================================================== Reported-by: Masayoshi MIZUMA Reviewed-by: Jan Kara Signed-off-by: Wu Fengguang Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fs-writeback.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/fs-writeback.c') diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index e3fe9918faaf..f81f9e71871e 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -538,7 +538,8 @@ void generic_sync_sb_inodes(struct super_block *sb, list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { struct address_space *mapping; - if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) + if (inode->i_state & + (I_FREEING|I_CLEAR|I_WILL_FREE|I_NEW)) continue; mapping = inode->i_mapping; if (mapping->nrpages == 0) -- cgit v1.2.3 From d2caa3c549c74d6476e2c29e13bd4d0e7d21c7fe Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 2 Apr 2009 16:56:37 -0700 Subject: writeback: guard against jiffies wraparound on inode->dirtied_when checks (try #3) The dirtied_when value on an inode is supposed to represent the first time that an inode has one of its pages dirtied. This value is in units of jiffies. It's used in several places in the writeback code to determine when to write out an inode. The problem is that these checks assume that dirtied_when is updated periodically. If an inode is continuously being used for I/O it can be persistently marked as dirty and will continue to age. Once the time compared to is greater than or equal to half the maximum of the jiffies type, the logic of the time_*() macros inverts and the opposite of what is needed is returned. On 32-bit architectures that's just under 25 days (assuming HZ == 1000). As the least-recently dirtied inode, it'll end up being the first one that pdflush will try to write out. sync_sb_inodes does this check: /* Was this inode dirtied after sync_sb_inodes was called? */ if (time_after(inode->dirtied_when, start)) break; ...but now dirtied_when appears to be in the future. sync_sb_inodes bails out without attempting to write any dirty inodes. When this occurs, pdflush will stop writing out inodes for this superblock. Nothing can unwedge it until jiffies moves out of the problematic window. This patch fixes this problem by changing the checks against dirtied_when to also check whether it appears to be in the future. If it does, then we consider the value to be far in the past. This should shrink the problematic window of time to such a small period (30s) as not to matter. Signed-off-by: Jeff Layton Signed-off-by: Wu Fengguang Acked-by: Ian Kent Cc: Jens Axboe Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fs-writeback.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'fs/fs-writeback.c') diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index f81f9e71871e..eed480639902 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -196,7 +196,7 @@ static void redirty_tail(struct inode *inode) struct inode *tail_inode; tail_inode = list_entry(sb->s_dirty.next, struct inode, i_list); - if (!time_after_eq(inode->dirtied_when, + if (time_before(inode->dirtied_when, tail_inode->dirtied_when)) inode->dirtied_when = jiffies; } @@ -220,6 +220,21 @@ static void inode_sync_complete(struct inode *inode) wake_up_bit(&inode->i_state, __I_SYNC); } +static bool inode_dirtied_after(struct inode *inode, unsigned long t) +{ + bool ret = time_after(inode->dirtied_when, t); +#ifndef CONFIG_64BIT + /* + * For inodes being constantly redirtied, dirtied_when can get stuck. + * It _appears_ to be in the future, but is actually in distant past. + * This test is necessary to prevent such wrapped-around relative times + * from permanently stopping the whole pdflush writeback. + */ + ret = ret && time_before_eq(inode->dirtied_when, jiffies); +#endif + return ret; +} + /* * Move expired dirty inodes from @delaying_queue to @dispatch_queue. */ @@ -231,7 +246,7 @@ static void move_expired_inodes(struct list_head *delaying_queue, struct inode *inode = list_entry(delaying_queue->prev, struct inode, i_list); if (older_than_this && - time_after(inode->dirtied_when, *older_than_this)) + inode_dirtied_after(inode, *older_than_this)) break; list_move(&inode->i_list, dispatch_queue); } @@ -492,8 +507,11 @@ void generic_sync_sb_inodes(struct super_block *sb, continue; /* blockdev has wrong queue */ } - /* Was this inode dirtied after sync_sb_inodes was called? */ - if (time_after(inode->dirtied_when, start)) + /* + * Was this inode dirtied after sync_sb_inodes was called? + * This keeps sync from extra jobs and livelock. + */ + if (inode_dirtied_after(inode, start)) break; /* Is another pdflush already flushing this queue? */ -- cgit v1.2.3