Re: [VFS] move active filesystem

Matthew Wilcox (Matthew.Wilcox@genedata.com)
Thu, 13 May 1999 22:41:04 +0200


On Thu, May 13, 1999 at 11:25:57AM -0400, Alexander Viro wrote:
> The thing that actually worries me here being that all analysis of
> potential races was done in assumption that mountpoints never move. Quite
> probably your patch *is* safe (with addition of loop check, that is). As
> for lookups - yes, you are right. It's not worse than renames (which are
> not race-free wrt lookups ;-/) and the same fix will work here too.
> Well, it's 2.3, so... Let's try it and look whether it will give
> any troubles wrt NFS. Final word here belongs to Trond and Linus, indeed.

Great. Here's the patch again, revised to include the loop check you
requested. Your check missed a boundary condition (remounting / on itself),
but it's the right idea:

--- fs/super.c.linus Wed May 12 14:10:46 1999
+++ fs/super.c Thu May 13 22:32:45 1999
@@ -49,7 +49,7 @@

extern int root_mountflags;

-static int do_remount_sb(struct super_block *sb, int flags, char * data);
+static int do_remount_sb(struct vfsmount *vfsmnt, int flags, char * data);

/* this is initialized in init/main.c */
kdev_t ROOT_DEV;
@@ -86,7 +86,6 @@
}

return ((struct vfsmount *)NULL);
- /* NOTREACHED */
}

static struct vfsmount *add_vfsmnt(struct super_block *sb,
@@ -684,8 +683,11 @@
* we just try to remount it readonly.
*/
retval = 0;
- if (!(sb->s_flags & MS_RDONLY))
- retval = do_remount_sb(sb, MS_RDONLY, 0);
+ if (!(sb->s_flags & MS_RDONLY)) {
+ struct vfsmount *vfsmnt = lookup_vfsmnt(sb->s_dev);
+ if (vfsmnt)
+ retval = do_remount_sb(vfsmnt, MS_RDONLY, 0);
+ }
return retval;
}

@@ -911,11 +913,11 @@
* FS-specific mount options can't be altered by remounting.
*/

-static int do_remount_sb(struct super_block *sb, int flags, char *data)
+static int do_remount_sb(struct vfsmount *vfsmnt, int flags, char *data)
{
int retval;
- struct vfsmount *vfsmnt;
-
+ struct super_block *sb = vfsmnt->mnt_sb;
+
/*
* Invalidate the inodes, as some mount options may be changed.
* N.B. If we are changing media, we should check the return
@@ -936,35 +938,79 @@
return retval;
}
sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK);
- vfsmnt = lookup_vfsmnt(sb->s_dev);
- if (vfsmnt)
- vfsmnt->mnt_flags = sb->s_flags;
+ vfsmnt->mnt_flags = sb->s_flags;
return 0;
}

-static int do_remount(const char *dir,int flags,char *data)
+static int do_remount(const char *dev, const char *dir, int flags, char *data)
{
- struct dentry *dentry;
+ struct dentry *dev_d, *ndir_d, *odir_d;
int retval;
+ struct vfsmount *vfsmnt;
+ struct super_block *sb, *parent_sb;

- dentry = namei(dir);
- retval = PTR_ERR(dentry);
- if (!IS_ERR(dentry)) {
- struct super_block * sb = dentry->d_inode->i_sb;
+ dev_d = namei(dev);
+ retval = PTR_ERR(dev_d);
+ if (IS_ERR(dev_d))
+ return retval;
+ vfsmnt = lookup_vfsmnt(dev_d->d_inode->i_rdev);
+ retval = -EINVAL;
+ if (!vfsmnt)
+ goto out;
+ sb = vfsmnt->mnt_sb;

- retval = -EINVAL;
- if (dentry == sb->s_root) {
- /*
- * Shrink the dcache and sync the device.
- */
- shrink_dcache_sb(sb);
- fsync_dev(sb->s_dev);
- if (flags & MS_RDONLY)
- acct_auto_close(sb->s_dev);
- retval = do_remount_sb(sb, flags, data);
- }
- dput(dentry);
+ /*
+ * Shrink the dcache and sync the device.
+ */
+ shrink_dcache_sb(sb);
+ fsync_dev(sb->s_dev);
+ if (flags & MS_RDONLY)
+ acct_auto_close(sb->s_dev);
+ retval = do_remount_sb(vfsmnt, flags, data);
+ if (retval != 0)
+ goto out;
+
+ ndir_d = namei(dir);
+ retval = PTR_ERR(ndir_d);
+ if (IS_ERR(ndir_d))
+ goto out;
+
+ /* if this is the same mount point, there's no more to do. */
+ if (ndir_d == sb->s_root)
+ goto dput_and_out;
+ /* is this a directory? */
+ retval = -ENOTDIR;
+ if (!S_ISDIR(ndir_d->d_inode->i_mode))
+ goto dput_and_out;
+ /* is something else already mounted on this directory? */
+ retval = -EBUSY;
+ if (ndir_d->d_covers != ndir_d)
+ goto dput_and_out;
+ /* or is the new directory on this filesystem? */
+ parent_sb = ndir_d->d_sb;
+ while (parent_sb != parent_sb->s_root->d_covers->d_inode->i_sb) {
+ if (parent_sb == sb)
+ goto dput_and_out;
+ parent_sb = parent_sb->s_root->d_covers->d_inode->i_sb;
}
+ if (parent_sb == sb)
+ goto dput_and_out;
+
+ retval = 0;
+ odir_d = sb->s_root->d_covers;
+ ndir_d->d_mounts = odir_d->d_mounts;
+ odir_d->d_mounts = odir_d;
+ sb->s_root->d_covers = ndir_d;
+ ndir_d = odir_d; /* free the old one instead */
+
+ kfree(vfsmnt->mnt_dirname);
+ vfsmnt->mnt_dirname = (char *) kmalloc(strlen(dir)+1, GFP_KERNEL);
+ if (vfsmnt->mnt_dirname != NULL)
+ strcpy(vfsmnt->mnt_dirname, dir);
+dput_and_out:
+ dput(ndir_d);
+out:
+ dput(dev_d);
return retval;
}

@@ -1031,7 +1077,7 @@
retval = copy_mount_options (data, &page);
if (retval < 0)
goto out;
- retval = do_remount(dir_name,
+ retval = do_remount(dev_name, dir_name,
new_flags & ~MS_MGC_MSK & ~MS_REMOUNT,
(char *) page);
free_page(page);

-- 
Matthew Wilcox <willy@bofh.ai>
"Windows and MacOS are products, contrived by engineers in the service of
specific companies. Unix, by contrast, is not so much a product as it is a
painstakingly compiled oral history of the hacker subculture." - N Stephenson

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/