• Nick Piggin's avatar
    fs: new inode i_state corruption fix · 7ef0d737
    Nick Piggin authored
    There was a report of a data corruption
    http://lkml.org/lkml/2008/11/14/121.  There is a script included to
    reproduce the problem.
    
    During testing, I encountered a number of strange things with ext3, so I
    tried ext2 to attempt to reduce complexity of the problem.  I found that
    fsstress would quickly hang in wait_on_inode, waiting for I_LOCK to be
    cleared, even though instrumentation showed that unlock_new_inode had
    already been called for that inode.  This points to memory scribble, or
    synchronisation problme.
    
    i_state of I_NEW inodes is not protected by inode_lock because other
    processes are not supposed to touch them until I_LOCK (and I_NEW) is
    cleared.  Adding WARN_ON(inode->i_state & I_NEW) to sites where we modify
    i_state revealed that generic_sync_sb_inodes is picking up new inodes from
    the inode lists and passing them to __writeback_single_inode without
    waiting for I_NEW.  Subsequently modifying i_state causes corruption.  In
    my case it would look like this:
    
    CPU0                            CPU1
    unlock_new_inode()              __sync_single_inode()
     reg <- inode->i_state
     reg -> reg & ~(I_LOCK|I_NEW)   reg <- inode->i_state
     reg -> inode->i_state          reg -> reg | I_SYNC
                                    reg -> inode->i_state
    
    Non-atomic RMW on CPU1 overwrites CPU0 store and sets I_LOCK|I_NEW again.
    
    Fix for this is rather than wait for I_NEW inodes, just skip over them:
    inodes concurrently being created are not subject to data integrity
    operations, and should not significantly contribute to dirty memory
    either.
    
    After this change, I'm unable to reproduce any of the added warnings or
    hangs after ~1hour of running.  Previously, the new warnings would start
    immediately and hang would happen in under 5 minutes.
    
    I'm also testing on ext3 now, and so far no problems there either.  I
    don't know whether this fixes the problem reported above, but it fixes a
    real problem for me.
    
    Cc: "Jorge Boncompte [DTI2]" <jorge@dti2.net>
    Reported-by: default avatarAdrian Hunter <ext-adrian.hunter@nokia.com>
    Cc: Jan Kara <jack@suse.cz>
    Cc: <stable@kernel.org>
    Signed-off-by: default avatarNick Piggin <npiggin@suse.de>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    7ef0d737
inode.c 40.4 KB