[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080214060201.GA17680@infradead.org>
Date: Thu, 14 Feb 2008 01:02:01 -0500
From: Christoph Hellwig <hch@...radead.org>
To: Paul Menage <menage@...gle.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org
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@...gle.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists