[PATCH] VFS: Add read-only users count to superblock

[PATCH] VFS: Add read-only users count to superblock

[Valerie Aurora]

During the last FS summit, Al Viro suggested creating a superblock
level read-only marker so that union mounts could guarantee that the
underlying fs would not become writable.  This patch implements the
VFS support, but doesn't add any users.  The patch making union mounts
use the support is in our union mounts tree.  I think we also need
some way to pass this through NFS mounts, since a read-only NFS mount
for the bottom layer of a union mount is a common use case.

-VAL

commit b0bfe1b9023467184e138c8520a084ca1e7bf8ab
Author: Valerie Aurora (Henson) <vaurora@redhat.com>
Date:   Mon Jul 13 09:30:49 2009 -0700

    VFS: Add read-only users count to superblock
    
    While we can check if a file system is currently read-only, we can't
    guarantee that it will stay read-only.  The file system can be
    remounted read-write at any time; it's also concievable that a file
    system can be mounted a second time and converted to read-write if the
    underlying fs allows it.  This is a problem for union mounts, which
    require the underlying file system be read-only.  Add a read-only
    users count and don't allow remounts to change the file system to
    read-write or read-write mounts if there are any read-only users.
    
    Signed-off-by: Valerie Aurora (Henson) <vaurora@redhat.com>

diff --git a/fs/super.c b/fs/super.c
index 2761d3e..65972df 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -553,6 +553,15 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
 	}
 	remount_rw = !(flags & MS_RDONLY) && (sb->s_flags & MS_RDONLY);
 
+	/* If we are remounting read/write, make sure that none of the
+	   users require read-only for correct operation (such as
+	   union mounts). */
+	if (remount_rw && sb->s_readonly_users) {
+		printk(KERN_INFO "%s: In use by %d read-only user(s)\n",
+		       sb->s_id, sb->s_readonly_users);
+		return -EROFS;
+	}
+
 	if (sb->s_op->remount_fs) {
 		retval = sb->s_op->remount_fs(sb, &flags, data);
 		if (retval)
@@ -889,6 +898,10 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
  	if (error)
  		goto out_sb;
 
+	error = -EROFS;
+	if (mnt->mnt_sb->s_readonly_users)
+		goto out_sb;
+
 	mnt->mnt_mountpoint = mnt->mnt_root;
 	mnt->mnt_parent = mnt;
 	up_write(&mnt->mnt_sb->s_umount);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0872372..aff3dca 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1379,6 +1379,10 @@ struct super_block {
 	 * generic_show_options()
 	 */
 	char *s_options;
+	/*
+	 * Users who require read-only access - e.g., union mounts
+	 */
+	int s_readonly_users;
 };
 
 extern struct timespec current_fs_time(struct super_block *sb);

Re: [PATCH] VFS: Add read-only users count to superblock

[Erez Zadok]

In message <20090713192743.GA27582@shell>, Valerie Aurora writes:
> During the last FS summit, Al Viro suggested creating a superblock
> level read-only marker so that union mounts could guarantee that the
> underlying fs would not become writable.  This patch implements the
> VFS support, but doesn't add any users.  The patch making union mounts
> use the support is in our union mounts tree.  I think we also need
> some way to pass this through NFS mounts, since a read-only NFS mount
> for the bottom layer of a union mount is a common use case.
> 
> -VAL
[...]

Val, I've often wondered if a generic readonly superblock solution will
obviate the need for the sb->s_vfs_rename_mutex "kludge" (as commented in
fs.h) and the whole way directory-renames are done wrt locking.  Can the
rename code be the first user of such patch, or the patch isn't quite ready
for this?

I realize that marking a whole superblock readonly during a directory rename
can hurt performance in some cases, as compared to the current way of doing
things, using lock_rename() to lock a subtree of the namespace.  But I
suspect that many concurrent directory renames are an exception, and thus
practical performance won't be hurt much with such a patch -- but result in
major benefits of code simplicity for several file systems (esp. for
unioning layers).

Erez.

Re: [PATCH] VFS: Add read-only users count to superblock

[Valerie Aurora]

On Tue, Jul 14, 2009 at 02:18:44PM -0400, Erez Zadok wrote:
> In message <20090713192743.GA27582@shell>, Valerie Aurora writes:
> > During the last FS summit, Al Viro suggested creating a superblock
> > level read-only marker so that union mounts could guarantee that the
> > underlying fs would not become writable.  This patch implements the
> > VFS support, but doesn't add any users.  The patch making union mounts
> > use the support is in our union mounts tree.  I think we also need
> > some way to pass this through NFS mounts, since a read-only NFS mount
> > for the bottom layer of a union mount is a common use case.
> > 
> > -VAL
> [...]
> 
> Val, I've often wondered if a generic readonly superblock solution will
> obviate the need for the sb->s_vfs_rename_mutex "kludge" (as commented in
> fs.h) and the whole way directory-renames are done wrt locking.  Can the
> rename code be the first user of such patch, or the patch isn't quite ready
> for this?

I'm afraid not!  With this patch, you can only mark the superblock
(and all associated mounts) read-only if no files are open for writing
- not exactly the common case during a directory rename.  So no, it
can't replace the rename mutex, at least in its current form.

-VAL