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 <[EMAIL PROTECTED]> "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