lkml.org 
[lkml]   [2010]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[patch 2/7] fs: simple fsync race fix
    It is incorrect to test inode dirty bits without participating in the inode
    writeback protocol. Inode writeback sets I_SYNC and clears I_DIRTY_?, then
    writes out the particular bits, then clears I_SYNC when it is done. BTW. it
    may not completely write all pages out, so I_DIRTY_PAGES would get set
    again.

    This is a standard pattern used throughout the kernel's writeback caches
    (I_SYNC ~= I_WRITEBACK, if that makes it clearer).

    And so it is not possible to determine an inode's dirty status just by
    checking I_DIRTY bits. Especially not for the purpose of data integrity
    syncs.

    Missing the check for these bits means that fsync can complete while
    writeback to the inode is underway. Inode writeback functions get this
    right, so call into them rather than try to shortcut things by testing
    dirty state improperly.

    Signed-off-by: Nick Piggin <npiggin@kernel.dk>


    Index: linux-2.6/fs/libfs.c
    ===================================================================
    --- linux-2.6.orig/fs/libfs.c 2010-11-19 16:44:39.000000000 +1100
    +++ linux-2.6/fs/libfs.c 2010-11-19 16:49:42.000000000 +1100
    @@ -895,11 +895,6 @@ int generic_file_fsync(struct file *file
    int ret;

    ret = sync_mapping_buffers(inode->i_mapping);
    - if (!(inode->i_state & I_DIRTY))
    - return ret;
    - if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
    - return ret;
    -
    err = sync_inode_metadata(inode, 1);
    if (ret == 0)
    ret = err;
    Index: linux-2.6/fs/exofs/file.c
    ===================================================================
    --- linux-2.6.orig/fs/exofs/file.c 2010-11-19 16:50:00.000000000 +1100
    +++ linux-2.6/fs/exofs/file.c 2010-11-19 16:50:07.000000000 +1100
    @@ -48,11 +48,6 @@ static int exofs_file_fsync(struct file
    struct inode *inode = filp->f_mapping->host;
    struct super_block *sb;

    - if (!(inode->i_state & I_DIRTY))
    - return 0;
    - if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
    - return 0;
    -
    ret = sync_inode_metadata(inode, 1);

    /* This is a good place to write the sb */



    \
     
     \ /
      Last update: 2010-11-23 15:13    [W:0.024 / U:30.688 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site