]> nv-tegra.nvidia Code Review - linux-2.6.git/commitdiff
[PATCH] fix for prune_icache()/forced final iput() races
authorAlexander Viro <aviro@redhat.com>
Thu, 23 Jun 2005 07:09:01 +0000 (00:09 -0700)
committerLinus Torvalds <torvalds@ppc970.osdl.org>
Thu, 23 Jun 2005 16:45:17 +0000 (09:45 -0700)
Based on analysis and a patch from Russ Weight <rweight@us.ibm.com>

There is a race condition that can occur if an inode is allocated and then
released (using iput) during the ->fill_super functions.  The race
condition is between kswapd and mount.

For most filesystems this can only happen in an error path when kswapd is
running concurrently.  For isofs, however, the error can occur in a more
common code path (which is how the bug was found).

The logic here is "we want final iput() to free inode *now* instead of
letting it sit in cache if fs is going down or had not quite come up".  The
problem is with kswapd seeing such inodes in the middle of being killed and
happily taking over.

The clean solution would be to tell kswapd to leave those inodes alone and
let our final iput deal with them.  I.e.  add a new flag
(I_FORCED_FREEING), set it before write_inode_now() there and make
prune_icache() leave those alone.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
fs/inode.c
include/linux/fs.h

index 801fe7f362807b7d581f1ef06033184771e5129f..1f9a3a2b89bc67add68e90faa78612ebf6e884cf 100644 (file)
@@ -500,7 +500,7 @@ repeat:
                        continue;
                if (!test(inode, data))
                        continue;
                        continue;
                if (!test(inode, data))
                        continue;
-               if (inode->i_state & (I_FREEING|I_CLEAR)) {
+               if (inode->i_state & (I_FREEING|I_CLEAR|I_WILL_FREE)) {
                        __wait_on_freeing_inode(inode);
                        goto repeat;
                }
                        __wait_on_freeing_inode(inode);
                        goto repeat;
                }
@@ -525,7 +525,7 @@ repeat:
                        continue;
                if (inode->i_sb != sb)
                        continue;
                        continue;
                if (inode->i_sb != sb)
                        continue;
-               if (inode->i_state & (I_FREEING|I_CLEAR)) {
+               if (inode->i_state & (I_FREEING|I_CLEAR|I_WILL_FREE)) {
                        __wait_on_freeing_inode(inode);
                        goto repeat;
                }
                        __wait_on_freeing_inode(inode);
                        goto repeat;
                }
@@ -727,7 +727,7 @@ EXPORT_SYMBOL(iunique);
 struct inode *igrab(struct inode *inode)
 {
        spin_lock(&inode_lock);
 struct inode *igrab(struct inode *inode)
 {
        spin_lock(&inode_lock);
-       if (!(inode->i_state & I_FREEING))
+       if (!(inode->i_state & (I_FREEING|I_WILL_FREE)))
                __iget(inode);
        else
                /*
                __iget(inode);
        else
                /*
@@ -1024,17 +1024,21 @@ static void generic_forget_inode(struct inode *inode)
                if (!(inode->i_state & (I_DIRTY|I_LOCK)))
                        list_move(&inode->i_list, &inode_unused);
                inodes_stat.nr_unused++;
                if (!(inode->i_state & (I_DIRTY|I_LOCK)))
                        list_move(&inode->i_list, &inode_unused);
                inodes_stat.nr_unused++;
-               spin_unlock(&inode_lock);
-               if (!sb || (sb->s_flags & MS_ACTIVE))
+               if (!sb || (sb->s_flags & MS_ACTIVE)) {
+                       spin_unlock(&inode_lock);
                        return;
                        return;
+               }
+               inode->i_state |= I_WILL_FREE;
+               spin_unlock(&inode_lock);
                write_inode_now(inode, 1);
                spin_lock(&inode_lock);
                write_inode_now(inode, 1);
                spin_lock(&inode_lock);
+               inode->i_state &= ~I_WILL_FREE;
                inodes_stat.nr_unused--;
                hlist_del_init(&inode->i_hash);
        }
        list_del_init(&inode->i_list);
        list_del_init(&inode->i_sb_list);
                inodes_stat.nr_unused--;
                hlist_del_init(&inode->i_hash);
        }
        list_del_init(&inode->i_list);
        list_del_init(&inode->i_sb_list);
-       inode->i_state|=I_FREEING;
+       inode->i_state |= I_FREEING;
        inodes_stat.nr_inodes--;
        spin_unlock(&inode_lock);
        if (inode->i_data.nrpages)
        inodes_stat.nr_inodes--;
        spin_unlock(&inode_lock);
        if (inode->i_data.nrpages)
index e5a8db00df291ecee6365e6ff437dff3a5c82da0..3622e952e98cbda821cd858792a40036fc8947cd 100644 (file)
@@ -1025,6 +1025,7 @@ struct super_operations {
 #define I_FREEING              16
 #define I_CLEAR                        32
 #define I_NEW                  64
 #define I_FREEING              16
 #define I_CLEAR                        32
 #define I_NEW                  64
+#define I_WILL_FREE            128
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)