* [PATCH 0/4] read-only remount fixes
@ 2010-10-26 14:16 Miklos Szeredi
2010-10-26 14:16 ` [PATCH 1/4] vfs: ignore error on forced remount Miklos Szeredi
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Miklos Szeredi @ 2010-10-26 14:16 UTC (permalink / raw)
To: viro; +Cc: dave, akpm, linux-fsdevel, linux-kernel
Al,
Here's the series redone to address your comments.
Please apply.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/4] vfs: ignore error on forced remount 2010-10-26 14:16 [PATCH 0/4] read-only remount fixes Miklos Szeredi @ 2010-10-26 14:16 ` Miklos Szeredi 2010-10-26 14:16 ` [PATCH 2/4] vfs: keep list of mounts for each superblock Miklos Szeredi ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-10-26 14:16 UTC (permalink / raw) To: viro; +Cc: dave, akpm, linux-fsdevel, linux-kernel [-- Attachment #1: vfs-fix-force-remount-readonly.patch --] [-- Type: text/plain, Size: 843 bytes --] From: Miklos Szeredi <mszeredi@suse.cz> On emergency remount we want to force MS_RDONLY on the super block even if ->remount_fs() failed for some reason. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: linux-2.6/fs/super.c =================================================================== --- linux-2.6.orig/fs/super.c 2010-10-26 14:26:57.000000000 +0200 +++ linux-2.6/fs/super.c 2010-10-26 14:27:01.000000000 +0200 @@ -579,7 +579,8 @@ int do_remount_sb(struct super_block *sb if (sb->s_op->remount_fs) { retval = sb->s_op->remount_fs(sb, &flags, data); - if (retval) + /* If forced remount, go ahead despite any errors */ + if (retval && !force) return retval; } sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK); -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/4] vfs: keep list of mounts for each superblock 2010-10-26 14:16 [PATCH 0/4] read-only remount fixes Miklos Szeredi 2010-10-26 14:16 ` [PATCH 1/4] vfs: ignore error on forced remount Miklos Szeredi @ 2010-10-26 14:16 ` Miklos Szeredi 2010-10-26 14:16 ` [PATCH 3/4] vfs: protect remounting superblock read-only Miklos Szeredi 2010-10-26 14:16 ` [PATCH 4/4] vfs: fs_may_remount_ro: turn unnecessary check into a WARN_ON Miklos Szeredi 3 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-10-26 14:16 UTC (permalink / raw) To: viro; +Cc: dave, akpm, linux-fsdevel, linux-kernel [-- Attachment #1: vfs-mount-list.patch --] [-- Type: text/plain, Size: 3403 bytes --] From: Miklos Szeredi <mszeredi@suse.cz> Keep track of vfsmounts belonging to a superblock. List is protected by vfsmount_lock. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/namespace.c | 5 +++++ fs/super.c | 7 +++++++ include/linux/fs.h | 1 + include/linux/mount.h | 1 + 4 files changed, 14 insertions(+) Index: linux-2.6/include/linux/fs.h =================================================================== --- linux-2.6.orig/include/linux/fs.h 2010-10-26 14:26:57.000000000 +0200 +++ linux-2.6/include/linux/fs.h 2010-10-26 14:27:02.000000000 +0200 @@ -1348,6 +1348,7 @@ struct super_block { #else struct list_head s_files; #endif + struct list_head s_mounts; /* list of mounts */ /* s_dentry_lru and s_nr_dentry_unused are protected by dcache_lock */ struct list_head s_dentry_lru; /* unused dentry lru */ int s_nr_dentry_unused; /* # of dentry on lru */ Index: linux-2.6/include/linux/mount.h =================================================================== --- linux-2.6.orig/include/linux/mount.h 2010-10-26 14:26:57.000000000 +0200 +++ linux-2.6/include/linux/mount.h 2010-10-26 14:27:02.000000000 +0200 @@ -54,6 +54,7 @@ struct vfsmount { struct super_block *mnt_sb; /* pointer to superblock */ struct list_head mnt_mounts; /* list of children, anchored here */ struct list_head mnt_child; /* and going through their mnt_child */ + struct list_head mnt_instance; /* mount instance on sb->s_mounts */ int mnt_flags; /* 4 bytes hole on 64bits arches without fsnotify */ #ifdef CONFIG_FSNOTIFY Index: linux-2.6/fs/namespace.c =================================================================== --- linux-2.6.orig/fs/namespace.c 2010-10-26 14:27:01.000000000 +0200 +++ linux-2.6/fs/namespace.c 2010-10-26 14:27:02.000000000 +0200 @@ -622,6 +622,10 @@ static struct vfsmount *clone_mnt(struct if (!list_empty(&old->mnt_expire)) list_add(&mnt->mnt_expire, &old->mnt_expire); } + + br_write_lock(vfsmount_lock); + list_add_tail(&mnt->mnt_instance, &mnt->mnt_sb->s_mounts); + br_write_unlock(vfsmount_lock); } return mnt; @@ -661,6 +665,7 @@ void mntput_no_expire(struct vfsmount *m return; } if (likely(!mnt->mnt_pinned)) { + list_del(&mnt->mnt_instance); br_write_unlock(vfsmount_lock); __mntput(mnt); return; Index: linux-2.6/fs/super.c =================================================================== --- linux-2.6.orig/fs/super.c 2010-10-26 14:27:01.000000000 +0200 +++ linux-2.6/fs/super.c 2010-10-26 14:27:02.000000000 +0200 @@ -74,6 +74,7 @@ static struct super_block *alloc_super(s INIT_HLIST_HEAD(&s->s_anon); INIT_LIST_HEAD(&s->s_inodes); INIT_LIST_HEAD(&s->s_dentry_lru); + INIT_LIST_HEAD(&s->s_mounts); init_rwsem(&s->s_umount); mutex_init(&s->s_lock); lockdep_set_class(&s->s_umount, &type->s_umount_key); @@ -127,6 +128,7 @@ static inline void destroy_super(struct free_percpu(s->s_files); #endif security_sb_free(s); + WARN_ON(!list_empty(&s->s_mounts)); kfree(s->s_subtype); kfree(s->s_options); kfree(s); @@ -968,6 +970,11 @@ vfs_kern_mount(struct file_system_type * mnt->mnt_parent = mnt; up_write(&mnt->mnt_sb->s_umount); free_secdata(secdata); + + br_write_lock(vfsmount_lock); + list_add_tail(&mnt->mnt_instance, &mnt->mnt_sb->s_mounts); + br_write_unlock(vfsmount_lock); + return mnt; out_sb: dput(mnt->mnt_root); -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] vfs: protect remounting superblock read-only 2010-10-26 14:16 [PATCH 0/4] read-only remount fixes Miklos Szeredi 2010-10-26 14:16 ` [PATCH 1/4] vfs: ignore error on forced remount Miklos Szeredi 2010-10-26 14:16 ` [PATCH 2/4] vfs: keep list of mounts for each superblock Miklos Szeredi @ 2010-10-26 14:16 ` Miklos Szeredi 2010-10-26 14:16 ` [PATCH 4/4] vfs: fs_may_remount_ro: turn unnecessary check into a WARN_ON Miklos Szeredi 3 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-10-26 14:16 UTC (permalink / raw) To: viro; +Cc: dave, akpm, linux-fsdevel, linux-kernel [-- Attachment #1: vfs-protect-sb-remount.patch --] [-- Type: text/plain, Size: 5473 bytes --] From: Miklos Szeredi <mszeredi@suse.cz> Currently remouting superblock read-only is racy in a major way. With the per mount read-only infrastructure it is now possible to prevent most races, which this patch attempts. Before starting the remount read-only, iterate through all mounts belonging to the superblock and if none of them have any pending writes, set sb->s_readonly_remount. This indicates that remount is in progress and no further write requests are allowed. If the remount succeeds set MS_RDONLY and reset s_readonly_remount. If the remounting is unsuccessful just reset s_readonly_remount. This can result in transient EROFS errors, despite the fact the remount failed. Unfortunately hodling off writes is difficult as remount itself may touch the filesystem (e.g. through load_nls()) which would deadlock. Small races still remain because delayed writes due to nlink going to zero but inode still having a reference are not dealt with by this patch. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/internal.h | 1 + fs/namespace.c | 40 +++++++++++++++++++++++++++++++++++++++- fs/super.c | 22 ++++++++++++++++++---- include/linux/fs.h | 3 +++ 4 files changed, 61 insertions(+), 5 deletions(-) Index: linux-2.6/fs/internal.h =================================================================== --- linux-2.6.orig/fs/internal.h 2010-10-26 14:26:57.000000000 +0200 +++ linux-2.6/fs/internal.h 2010-10-26 14:27:03.000000000 +0200 @@ -69,6 +69,7 @@ extern void mnt_set_mountpoint(struct vf extern void release_mounts(struct list_head *); extern void umount_tree(struct vfsmount *, int, struct list_head *); extern struct vfsmount *copy_tree(struct vfsmount *, struct dentry *, int); +extern int sb_prepare_remount_readonly(struct super_block *); extern void __init mnt_init(void); Index: linux-2.6/fs/namespace.c =================================================================== --- linux-2.6.orig/fs/namespace.c 2010-10-26 14:27:02.000000000 +0200 +++ linux-2.6/fs/namespace.c 2010-10-26 14:27:03.000000000 +0200 @@ -251,6 +251,15 @@ static unsigned int count_mnt_writers(st #endif } +static int mnt_is_readonly(struct vfsmount *mnt) +{ + if (mnt->mnt_sb->s_readonly_remount) + return 1; + /* Order wrt setting s_flags/s_readonly_remount in do_remount() */ + smp_rmb(); + return __mnt_is_readonly(mnt); +} + /* * Most r/o checks on a fs are for operations that take * discrete amounts of time, like a write() or unlink(). @@ -289,7 +298,7 @@ int mnt_want_write(struct vfsmount *mnt) * MNT_WRITE_HOLD is cleared. */ smp_rmb(); - if (__mnt_is_readonly(mnt)) { + if (mnt_is_readonly(mnt)) { dec_mnt_writers(mnt); ret = -EROFS; goto out; @@ -406,6 +415,35 @@ static void __mnt_unmake_readonly(struct br_write_unlock(vfsmount_lock); } +int sb_prepare_remount_readonly(struct super_block *sb) +{ + struct vfsmount *mnt; + int err = 0; + + br_write_lock(vfsmount_lock); + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (!(mnt->mnt_flags & MNT_READONLY)) { + mnt->mnt_flags |= MNT_WRITE_HOLD; + smp_mb(); + if (count_mnt_writers(mnt) > 0) { + err = -EBUSY; + break; + } + } + } + if (!err) { + sb->s_readonly_remount = 1; + smp_wmb(); + } + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (mnt->mnt_flags & MNT_WRITE_HOLD) + mnt->mnt_flags &= ~MNT_WRITE_HOLD; + } + br_write_unlock(vfsmount_lock); + + return err; +} + void simple_set_mnt(struct vfsmount *mnt, struct super_block *sb) { mnt->mnt_sb = sb; Index: linux-2.6/fs/super.c =================================================================== --- linux-2.6.orig/fs/super.c 2010-10-26 14:27:02.000000000 +0200 +++ linux-2.6/fs/super.c 2010-10-26 14:27:03.000000000 +0200 @@ -573,19 +573,29 @@ int do_remount_sb(struct super_block *sb /* If we are remounting RDONLY and current sb is read/write, make sure there are no rw files opened */ if (remount_ro) { - if (force) + if (force) { mark_files_ro(sb); - else if (!fs_may_remount_ro(sb)) - return -EBUSY; + } else { + retval = sb_prepare_remount_readonly(sb); + if (retval) + return retval; + + retval = -EBUSY; + if (!fs_may_remount_ro(sb)) + goto cancel_readonly; + } } if (sb->s_op->remount_fs) { retval = sb->s_op->remount_fs(sb, &flags, data); /* If forced remount, go ahead despite any errors */ if (retval && !force) - return retval; + goto cancel_readonly; } sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK); + /* Needs to be ordered wrt mnt_is_readonly() */ + smp_wmb(); + sb->s_readonly_remount = 0; /* * Some filesystems modify their metadata via some other path than the @@ -598,6 +608,10 @@ int do_remount_sb(struct super_block *sb if (remount_ro && sb->s_bdev) invalidate_bdev(sb->s_bdev); return 0; + +cancel_readonly: + sb->s_readonly_remount = 0; + return retval; } static void do_emergency_remount(struct work_struct *work) Index: linux-2.6/include/linux/fs.h =================================================================== --- linux-2.6.orig/include/linux/fs.h 2010-10-26 14:27:02.000000000 +0200 +++ linux-2.6/include/linux/fs.h 2010-10-26 16:06:30.000000000 +0200 @@ -1388,6 +1388,9 @@ struct super_block { * generic_show_options() */ char __rcu *s_options; + + /* Being remounted read-only */ + int s_readonly_remount; }; extern struct timespec current_fs_time(struct super_block *sb); -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/4] vfs: fs_may_remount_ro: turn unnecessary check into a WARN_ON 2010-10-26 14:16 [PATCH 0/4] read-only remount fixes Miklos Szeredi ` (2 preceding siblings ...) 2010-10-26 14:16 ` [PATCH 3/4] vfs: protect remounting superblock read-only Miklos Szeredi @ 2010-10-26 14:16 ` Miklos Szeredi 3 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2010-10-26 14:16 UTC (permalink / raw) To: viro; +Cc: dave, akpm, linux-fsdevel, linux-kernel [-- Attachment #1: vfs-remove-unnecessary-remount-check.patch --] [-- Type: text/plain, Size: 1017 bytes --] From: Miklos Szeredi <mszeredi@suse.cz> Now a successful sb_prepare_remount_readonly() should ensure that no writable files remain for this superblock. So turn this check into a WARN_ON. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/file_table.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) Index: linux-2.6/fs/file_table.c =================================================================== --- linux-2.6.orig/fs/file_table.c 2010-10-26 14:26:57.000000000 +0200 +++ linux-2.6/fs/file_table.c 2010-10-26 14:27:05.000000000 +0200 @@ -437,8 +437,11 @@ int fs_may_remount_ro(struct super_block if (inode->i_nlink == 0) goto too_bad; - /* Writeable file? */ - if (S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE)) + /* + * Writable file? + * Should be caught by sb_prepare_remount_readonly(). + */ + if (WARN_ON(S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE))) goto too_bad; } while_file_list_for_each_entry; lg_global_unlock(files_lglock); -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 0/4] read-only remount race fix
@ 2011-08-03 10:48 Miklos Szeredi
2011-08-03 10:48 ` [PATCH 3/4] vfs: protect remounting superblock read-only Miklos Szeredi
0 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2011-08-03 10:48 UTC (permalink / raw)
To: viro
Cc: linux-fsdevel, linux-kernel, jack, hch, akpm, toshi.okajima, mszeredi
Al,
Here's an update of the read-only remount race fixes.
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git read-only-remount-fixes.v5
Please apply.
Thanks,
Miklos
---
Miklos Szeredi (4):
vfs: ignore error on forced remount
vfs: keep list of mounts for each superblock
vfs: protect remounting superblock read-only
vfs: fs_may_remount_ro: turn unnecessary check into a WARN_ON
---
fs/file_table.c | 7 ++++-
fs/internal.h | 1 +
fs/namespace.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-
fs/super.c | 27 +++++++++++++++++++++----
include/linux/fs.h | 4 +++
include/linux/mount.h | 1 +
6 files changed, 82 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/4] vfs: protect remounting superblock read-only 2011-08-03 10:48 [PATCH v5 0/4] read-only remount race fix Miklos Szeredi @ 2011-08-03 10:48 ` Miklos Szeredi 0 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2011-08-03 10:48 UTC (permalink / raw) To: viro Cc: linux-fsdevel, linux-kernel, jack, hch, akpm, toshi.okajima, mszeredi From: Miklos Szeredi <mszeredi@suse.cz> Currently remouting superblock read-only is racy in a major way. With the per mount read-only infrastructure it is now possible to prevent most races, which this patch attempts. Before starting the remount read-only, iterate through all mounts belonging to the superblock and if none of them have any pending writes, set sb->s_readonly_remount. This indicates that remount is in progress and no further write requests are allowed. If the remount succeeds set MS_RDONLY and reset s_readonly_remount. If the remounting is unsuccessful just reset s_readonly_remount. This can result in transient EROFS errors, despite the fact the remount failed. Unfortunately hodling off writes is difficult as remount itself may touch the filesystem (e.g. through load_nls()) which would deadlock. Small races still remain because delayed writes due to nlink going to zero but inode still having a reference are not dealt with by this patch. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/internal.h | 1 + fs/namespace.c | 40 +++++++++++++++++++++++++++++++++++++++- fs/super.c | 22 ++++++++++++++++++---- include/linux/fs.h | 3 +++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/fs/internal.h b/fs/internal.h index fe327c2..f925271 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -74,6 +74,7 @@ extern int finish_automount(struct vfsmount *, struct path *); extern void mnt_make_longterm(struct vfsmount *); extern void mnt_make_shortterm(struct vfsmount *); +extern int sb_prepare_remount_readonly(struct super_block *); extern void __init mnt_init(void); diff --git a/fs/namespace.c b/fs/namespace.c index c369c32..263201c 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -311,6 +311,15 @@ static unsigned int mnt_get_writers(struct vfsmount *mnt) #endif } +static int mnt_is_readonly(struct vfsmount *mnt) +{ + if (mnt->mnt_sb->s_readonly_remount) + return 1; + /* Order wrt setting s_flags/s_readonly_remount in do_remount() */ + smp_rmb(); + return __mnt_is_readonly(mnt); +} + /* * Most r/o checks on a fs are for operations that take * discrete amounts of time, like a write() or unlink(). @@ -349,7 +358,7 @@ int mnt_want_write(struct vfsmount *mnt) * MNT_WRITE_HOLD is cleared. */ smp_rmb(); - if (__mnt_is_readonly(mnt)) { + if (mnt_is_readonly(mnt)) { mnt_dec_writers(mnt); ret = -EROFS; goto out; @@ -466,6 +475,35 @@ static void __mnt_unmake_readonly(struct vfsmount *mnt) br_write_unlock(vfsmount_lock); } +int sb_prepare_remount_readonly(struct super_block *sb) +{ + struct vfsmount *mnt; + int err = 0; + + br_write_lock(vfsmount_lock); + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (!(mnt->mnt_flags & MNT_READONLY)) { + mnt->mnt_flags |= MNT_WRITE_HOLD; + smp_mb(); + if (mnt_get_writers(mnt) > 0) { + err = -EBUSY; + break; + } + } + } + if (!err) { + sb->s_readonly_remount = 1; + smp_wmb(); + } + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (mnt->mnt_flags & MNT_WRITE_HOLD) + mnt->mnt_flags &= ~MNT_WRITE_HOLD; + } + br_write_unlock(vfsmount_lock); + + return err; +} + static void free_vfsmnt(struct vfsmount *mnt) { kfree(mnt->mnt_devname); diff --git a/fs/super.c b/fs/super.c index 994003b..62c6aa9 100644 --- a/fs/super.c +++ b/fs/super.c @@ -721,19 +721,29 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force) /* If we are remounting RDONLY and current sb is read/write, make sure there are no rw files opened */ if (remount_ro) { - if (force) + if (force) { mark_files_ro(sb); - else if (!fs_may_remount_ro(sb)) - return -EBUSY; + } else { + retval = sb_prepare_remount_readonly(sb); + if (retval) + return retval; + + retval = -EBUSY; + if (!fs_may_remount_ro(sb)) + goto cancel_readonly; + } } if (sb->s_op->remount_fs) { retval = sb->s_op->remount_fs(sb, &flags, data); /* If forced remount, go ahead despite any errors */ if (retval && !force) - return retval; + goto cancel_readonly; } sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK); + /* Needs to be ordered wrt mnt_is_readonly() */ + smp_wmb(); + sb->s_readonly_remount = 0; /* * Some filesystems modify their metadata via some other path than the @@ -746,6 +756,10 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force) if (remount_ro && sb->s_bdev) invalidate_bdev(sb->s_bdev); return 0; + +cancel_readonly: + sb->s_readonly_remount = 0; + return retval; } static void do_emergency_remount(struct work_struct *work) diff --git a/include/linux/fs.h b/include/linux/fs.h index ca046b4..9d2dba5 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1449,6 +1449,9 @@ struct super_block { int cleancache_poolid; struct shrinker s_shrink; /* per-sb shrinker handle */ + + /* Being remounted read-only */ + int s_readonly_remount; }; /* superblock cache pruning functions */ -- 1.7.3.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <1314191791-29218-1-git-send-email-miklos@szeredi.hu>]
* [PATCH 3/4] vfs: protect remounting superblock read-only [not found] <1314191791-29218-1-git-send-email-miklos@szeredi.hu> @ 2011-08-24 13:22 ` Miklos Szeredi 0 siblings, 0 replies; 7+ messages in thread From: Miklos Szeredi @ 2011-08-24 13:22 UTC (permalink / raw) To: viro Cc: linux-fsdevel, linux-kernel, jack, hch, akpm, toshi.okajima, mszeredi From: Miklos Szeredi <mszeredi@suse.cz> Currently remouting superblock read-only is racy in a major way. With the per mount read-only infrastructure it is now possible to prevent most races, which this patch attempts. Before starting the remount read-only, iterate through all mounts belonging to the superblock and if none of them have any pending writes, set sb->s_readonly_remount. This indicates that remount is in progress and no further write requests are allowed. If the remount succeeds set MS_RDONLY and reset s_readonly_remount. If the remounting is unsuccessful just reset s_readonly_remount. This can result in transient EROFS errors, despite the fact the remount failed. Unfortunately hodling off writes is difficult as remount itself may touch the filesystem (e.g. through load_nls()) which would deadlock. Small races still remain because delayed writes due to nlink going to zero but inode still having a reference are not dealt with by this patch. Signed-off-by: Miklos Szeredi <mszeredi@suse.cz> --- fs/internal.h | 1 + fs/namespace.c | 40 +++++++++++++++++++++++++++++++++++++++- fs/super.c | 22 ++++++++++++++++++---- include/linux/fs.h | 3 +++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/fs/internal.h b/fs/internal.h index fe327c2..f925271 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -74,6 +74,7 @@ extern int finish_automount(struct vfsmount *, struct path *); extern void mnt_make_longterm(struct vfsmount *); extern void mnt_make_shortterm(struct vfsmount *); +extern int sb_prepare_remount_readonly(struct super_block *); extern void __init mnt_init(void); diff --git a/fs/namespace.c b/fs/namespace.c index c369c32..263201c 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -311,6 +311,15 @@ static unsigned int mnt_get_writers(struct vfsmount *mnt) #endif } +static int mnt_is_readonly(struct vfsmount *mnt) +{ + if (mnt->mnt_sb->s_readonly_remount) + return 1; + /* Order wrt setting s_flags/s_readonly_remount in do_remount() */ + smp_rmb(); + return __mnt_is_readonly(mnt); +} + /* * Most r/o checks on a fs are for operations that take * discrete amounts of time, like a write() or unlink(). @@ -349,7 +358,7 @@ int mnt_want_write(struct vfsmount *mnt) * MNT_WRITE_HOLD is cleared. */ smp_rmb(); - if (__mnt_is_readonly(mnt)) { + if (mnt_is_readonly(mnt)) { mnt_dec_writers(mnt); ret = -EROFS; goto out; @@ -466,6 +475,35 @@ static void __mnt_unmake_readonly(struct vfsmount *mnt) br_write_unlock(vfsmount_lock); } +int sb_prepare_remount_readonly(struct super_block *sb) +{ + struct vfsmount *mnt; + int err = 0; + + br_write_lock(vfsmount_lock); + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (!(mnt->mnt_flags & MNT_READONLY)) { + mnt->mnt_flags |= MNT_WRITE_HOLD; + smp_mb(); + if (mnt_get_writers(mnt) > 0) { + err = -EBUSY; + break; + } + } + } + if (!err) { + sb->s_readonly_remount = 1; + smp_wmb(); + } + list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) { + if (mnt->mnt_flags & MNT_WRITE_HOLD) + mnt->mnt_flags &= ~MNT_WRITE_HOLD; + } + br_write_unlock(vfsmount_lock); + + return err; +} + static void free_vfsmnt(struct vfsmount *mnt) { kfree(mnt->mnt_devname); diff --git a/fs/super.c b/fs/super.c index e8aad34..94c8ace 100644 --- a/fs/super.c +++ b/fs/super.c @@ -721,23 +721,33 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force) /* If we are remounting RDONLY and current sb is read/write, make sure there are no rw files opened */ if (remount_ro) { - if (force) + if (force) { mark_files_ro(sb); - else if (!fs_may_remount_ro(sb)) - return -EBUSY; + } else { + retval = sb_prepare_remount_readonly(sb); + if (retval) + return retval; + + retval = -EBUSY; + if (!fs_may_remount_ro(sb)) + goto cancel_readonly; + } } if (sb->s_op->remount_fs) { retval = sb->s_op->remount_fs(sb, &flags, data); if (retval) { if(!force) - return retval; + goto cancel_readonly; /* If forced remount, go ahead despite any errors */ WARN(1, "forced remount of a %s fs returned %i\n", sb->s_type->name, retval); } } sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK); + /* Needs to be ordered wrt mnt_is_readonly() */ + smp_wmb(); + sb->s_readonly_remount = 0; /* * Some filesystems modify their metadata via some other path than the @@ -750,6 +760,10 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force) if (remount_ro && sb->s_bdev) invalidate_bdev(sb->s_bdev); return 0; + +cancel_readonly: + sb->s_readonly_remount = 0; + return retval; } static void do_emergency_remount(struct work_struct *work) diff --git a/include/linux/fs.h b/include/linux/fs.h index e2a67e6..e820478 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1464,6 +1464,9 @@ struct super_block { int cleancache_poolid; struct shrinker s_shrink; /* per-sb shrinker handle */ + + /* Being remounted read-only */ + int s_readonly_remount; }; /* superblock cache pruning functions */ -- 1.7.3.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-08-24 13:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-26 14:16 [PATCH 0/4] read-only remount fixes Miklos Szeredi
2010-10-26 14:16 ` [PATCH 1/4] vfs: ignore error on forced remount Miklos Szeredi
2010-10-26 14:16 ` [PATCH 2/4] vfs: keep list of mounts for each superblock Miklos Szeredi
2010-10-26 14:16 ` [PATCH 3/4] vfs: protect remounting superblock read-only Miklos Szeredi
2010-10-26 14:16 ` [PATCH 4/4] vfs: fs_may_remount_ro: turn unnecessary check into a WARN_ON Miklos Szeredi
2011-08-03 10:48 [PATCH v5 0/4] read-only remount race fix Miklos Szeredi
2011-08-03 10:48 ` [PATCH 3/4] vfs: protect remounting superblock read-only Miklos Szeredi
[not found] <1314191791-29218-1-git-send-email-miklos@szeredi.hu>
2011-08-24 13:22 ` Miklos Szeredi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®