Skip to content
  • Chris Mason's avatar
    ext3: Add locking to ext3_do_update_inode · 4f003fd3
    Chris Mason authored
    
    
    I've been struggling with this off and on while I've been testing the
    data=guarded work.  The symptom is corrupted orphan lists and inodes
    with the wrong i_size stored on disk.  I was convinced the
    data=guarded code was just missing a call to ext3_mark_inode_dirty, but
    tracing showed the i_disksize I was sending to ext3_mark_inode_dirty
    wasn't actually making it to the drive.
    
    ext3_mark_inode_dirty can be called without locks held (atime updates
    and a few others), so the data=guarded code uses locks while updating
    the in-memory inode, and then calls ext3_mark_inode_dirty
    without any locks held.
    
    But, ext3_mark_inode_dirty has no internal locking to make sure that
    only one CPU is updating the buffer head at a time.  Generally this
    works out ok because everyone that changes the inode then calls
    ext3_mark_inode_dirty themselves.  Even though it races, eventually
    someone updates the buffer heads and things move on.
    
    But there is still a risk of the wrong values getting in, and the
    data=guarded code seems to hit the race very often.
    
    Since everyone that changes the inode also logs it, it should be
    possible to fix this with some memory barriers.  I'll leave that as an
    exercise to the reader and lock the buffer head instead.
    
    It it probably a good idea to have a different patch series for lockless
    bit flipping on the ext3 i_state field.  ext3_do_update_inode &= clears
    EXT3_STATE_NEW without any locks held.
    
    Signed-off-by: default avatarChris Mason <chris.mason@oracle.com>
    Signed-off-by: default avatarJan Kara <jack@suse.cz>
    4f003fd3