lkml.org 
[lkml]   [2010]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] fs: fix filesystem_sync vs write race on rw=>ro remount
    Date
    Currently on rw=>ro remount we have following race
    | mount /mnt -oremount,ro | write-task |
    |-------------------------+------------|
    | | open(RDWR) |
    | shrink_dcache_sb(sb); | |
    | sync_filesystem(sb); | |
    | | write() |
    | | close() |
    | fs_may_remount_ro(sb) | |
    | sb->s_flags = new_flags | |
    Later writeback or sync() will result in error due to MS_RDONLY flag
    In case of ext4 this result in jbd2_start failure on writeback
    ext4_da_writepages: jbd2_start: 1024 pages, ino 1431; err -30
    In fact all others are affected by this error but it is not visible
    because the skip s_flags check on writeback. For example ext3 check
    (s_flags & MS_RDONLY) only if page has no buffers during journal start.

    In order to prevent the race we have to block new writers before
    fs_may_remount_ro() and sync_filesystem(). Let's introduce new
    sb->s_flags MS_RO_REMOUNT flag for this purpose. But suddenly we have
    no available space in MS_XXX bits, let's share this bit with MS_REMOUNT.
    This is possible because MS_REMOUNT used only for passing arguments
    from flags to sys_mount() and never used in sb->s_flags.

    ##TESTCASE_BEGIN:
    #! /bin/bash -x
    DEV=/dev/sdb5
    FSTYPE=ext4
    BINDIR=/home/dmon
    MNTOPT="data=ordered"
    umount /mnt
    mkfs.${FSTYPE} ${DEV} || exit 1
    mount ${DEV} /mnt -o${MNTOPT} || exit 1
    ${BINDIR}/fsstress -p1 -l999999999 -n9999999999 -d /mnt/test &
    sleep 15
    mount /mnt -oremount,ro,${MNTOPT}
    sleep 1
    killall -9 fsstress
    sync
    # after this you may get following message in dmesg
    # "ext4_da_writepages: jbd2_start: 1024 pages, ino 1431; err -30"
    ##TESTCASE_END

    Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
    --
    diff --git a/fs/namespace.c b/fs/namespace.c
    index c768f73..a216fb3 100644
    --- a/fs/namespace.c
    +++ b/fs/namespace.c
    @@ -194,7 +194,7 @@ int __mnt_is_readonly(struct vfsmount *mnt)
    {
    if (mnt->mnt_flags & MNT_READONLY)
    return 1;
    - if (mnt->mnt_sb->s_flags & MS_RDONLY)
    + if (mnt->mnt_sb->s_flags & (MS_RDONLY| MS_RO_REMOUNT))
    return 1;
    return 0;
    }
    diff --git a/fs/super.c b/fs/super.c
    index aff046b..756fe88 100644
    --- a/fs/super.c
    +++ b/fs/super.c
    @@ -569,42 +569,51 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
    {
    int retval;
    int remount_rw;
    + int remount_ro;

    if (sb->s_frozen != SB_UNFROZEN)
    return -EBUSY;
    -
    + remount_ro = (flags & MS_RDONLY) && !(sb->s_flags & MS_RDONLY);
    #ifdef CONFIG_BLOCK
    if (!(flags & MS_RDONLY) && bdev_read_only(sb->s_bdev))
    return -EACCES;
    #endif
    -
    if (flags & MS_RDONLY)
    acct_auto_close(sb);
    - shrink_dcache_sb(sb);
    - sync_filesystem(sb);

    /* If we are remounting RDONLY and current sb is read/write,
    make sure there are no rw files opened */
    - if ((flags & MS_RDONLY) && !(sb->s_flags & MS_RDONLY)) {
    + retval = -EBUSY;
    + if (remount_ro) {
    + /* Prevent new writers before check */
    + sb->s_flags |= MS_RO_REMOUNT;
    if (force)
    mark_files_ro(sb);
    else if (!fs_may_remount_ro(sb))
    - return -EBUSY;
    + goto out;
    + }
    + shrink_dcache_sb(sb);
    + sync_filesystem(sb);
    +
    + if (remount_ro) {
    retval = vfs_dq_off(sb, 1);
    if (retval < 0 && retval != -ENOSYS)
    - return -EBUSY;
    + goto out;
    }
    remount_rw = !(flags & MS_RDONLY) && (sb->s_flags & MS_RDONLY);

    if (sb->s_op->remount_fs) {
    retval = sb->s_op->remount_fs(sb, &flags, data);
    if (retval)
    - return retval;
    + goto out;
    }
    sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK);
    if (remount_rw)
    vfs_dq_quota_on_remount(sb);
    - return 0;
    +out:
    + if (remount_ro)
    + sb->s_flags = (sb->s_flags & ~MS_RO_REMOUNT);
    + return retval;
    }

    static void do_emergency_remount(struct work_struct *work)
    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index b1bcb27..a613875 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -208,6 +208,9 @@ struct inodes_stat_t {
    #define MS_STRICTATIME (1<<24) /* Always perform atime updates */
    #define MS_ACTIVE (1<<30)
    #define MS_NOUSER (1<<31)
    +#define MS_RO_REMOUNT MS_REMOUNT /* Alter flags from rw=>ro of mounted FS.
    + Not conflicting with MS_REMOUNT because
    + it never stored in sb->s_flags */

    /*
    * Superblock flags that can be altered by MS_REMOUNT
    \
     
     \ /
      Last update: 2010-01-24 12:43    [W:0.025 / U:58.912 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site