Messages in this thread | | | Date | Thu, 14 Feb 2008 01:02:01 -0500 | From | Christoph Hellwig <> | Subject | Re: [PATCH] Add MS_BIND_FLAGS mount flag |
| |
On Tue, Feb 12, 2008 at 09:45:15PM -0800, Paul Menage wrote: > From: Paul Menage <menage@google.com> > > Add a new mount() flag, MS_BIND_FLAGS. > > MS_BIND_FLAGS indicates that a bind mount should take its per-mount flags > from the arguments passed to mount() rather than from the source > mountpoint. > > This flag allows you to create a bind mount with the desired per-mount > flags in a single operation, rather than having to do a bind mount > followed by a remount, which is fiddly and can block for non-trivial > periods of time (on sb->s_umount?). > > For recursive bind mounts, only the root of the tree being bound > inherits the per-mount flags from the mount() arguments; sub-mounts > inherit their per-mount flags from the source tree as usual.
I think this concept is reasonable, but I don't think MS_BIND_FLAGS is a descriptive name for this flag. MS_EXPLICIT_FLAGS might be better but still isn't optimal.
> +static struct vfsmount * > +__copy_tree(struct vfsmount *mnt, struct dentry *dentry, > + int flag, int mnt_flags)
> +struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry, > + int flag) > +{ > + return __copy_tree(mnt, dentry, flag, mnt->mnt_flags); > +}
Please just change copy_tree to have an additional parameter. There's just four callers of it in the tree, and three of them want the new parameter.
> + /* Use the source mount flags unless the user passed MS_BIND_FLAGS */
I think this comment could be made a little more explicit.
/* * If MS_EXPLICIT_FLAGS is passed in we will take the paramters * the user has specified. The default behaviour for bind * mounts is however to take the flags from existing mount * instances for the same superblock. */ > #define MS_RELATIME (1<<21) /* Update atime relative to mtime/ctime. */ > #define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */ > #define MS_I_VERSION (1<<23) /* Update inode I_version field */ > +#define MS_BIND_FLAGS (1<<24) /* For MS_BIND, take mnt_flags from > + * args, not from source mountpoint */ > #define MS_ACTIVE (1<<30) > #define MS_NOUSER (1<<31)
this looks like whitespace damage to me.
| |