2005-11-24 16:10:21

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 1/2] shared mounts: cleanup

Small cleanups in shared mounts code.

Signed-off-by: Miklos Szeredi <[email protected]>
---

Index: linux/fs/pnode.c
===================================================================
--- linux.orig/fs/pnode.c 2005-11-23 17:12:01.000000000 +0100
+++ linux/fs/pnode.c 2005-11-23 17:13:03.000000000 +0100
@@ -103,7 +103,7 @@ static struct vfsmount *propagation_next
struct vfsmount *next;
struct vfsmount *master = m->mnt_master;

- if ( master == origin->mnt_master ) {
+ if (master == origin->mnt_master) {
next = next_peer(m);
return ((next == origin) ? NULL : next);
} else if (m->mnt_slave.next != &master->mnt_slave_list)
Index: linux/include/linux/mount.h
===================================================================
--- linux.orig/include/linux/mount.h 2005-11-23 17:12:01.000000000 +0100
+++ linux/include/linux/mount.h 2005-11-23 17:13:03.000000000 +0100
@@ -22,7 +22,8 @@
#define MNT_NOEXEC 0x04
#define MNT_SHARED 0x10 /* if the vfsmount is a shared mount */
#define MNT_UNBINDABLE 0x20 /* if the vfsmount is a unbindable mount */
-#define MNT_PNODE_MASK 0x30 /* propogation flag mask */
+
+#define MNT_PNODE_MASK (MNT_SHARED | MNT_UNBINDABLE)

struct vfsmount {
struct list_head mnt_hash;
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h 2005-11-23 17:12:01.000000000 +0100
+++ linux/include/linux/fs.h 2005-11-23 17:13:03.000000000 +0100
@@ -103,11 +103,11 @@ extern int dir_notify_enable;
#define MS_MOVE 8192
#define MS_REC 16384
#define MS_VERBOSE 32768
+#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */
#define MS_UNBINDABLE (1<<17) /* change to unbindable */
#define MS_PRIVATE (1<<18) /* change to private */
#define MS_SLAVE (1<<19) /* change to slave */
#define MS_SHARED (1<<20) /* change to shared */
-#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */
#define MS_ACTIVE (1<<30)
#define MS_NOUSER (1<<31)

Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c 2005-11-23 17:12:01.000000000 +0100
+++ linux/fs/namespace.c 2005-11-23 17:13:03.000000000 +0100
@@ -451,7 +451,7 @@ EXPORT_SYMBOL(may_umount);
void release_mounts(struct list_head *head)
{
struct vfsmount *mnt;
- while(!list_empty(head)) {
+ while (!list_empty(head)) {
mnt = list_entry(head->next, struct vfsmount, mnt_hash);
list_del_init(&mnt->mnt_hash);
if (mnt->mnt_parent != mnt) {


2005-11-24 16:18:45

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 2/2] shared mounts: save mount flag space

Remaining mount flags are becoming scarce (just 11 bits)
and shared mount code uses 4 though one would suffice.

I think this should go into 2.6.15, fixing it later would be breaking
userspace ABI.

Signed-off-by: Miklos Szeredi <[email protected]>
---

Index: linux/fs/pnode.c
===================================================================
--- linux.orig/fs/pnode.c 2005-11-24 16:21:02.000000000 +0100
+++ linux/fs/pnode.c 2005-11-24 16:33:12.000000000 +0100
@@ -72,17 +72,17 @@ static int do_make_slave(struct vfsmount
return 0;
}

-void change_mnt_propagation(struct vfsmount *mnt, int type)
+void change_mnt_propagation(struct vfsmount *mnt, enum propagation_type type)
{
- if (type == MS_SHARED) {
+ if (type == PT_SHARED) {
set_mnt_shared(mnt);
return;
}
do_make_slave(mnt);
- if (type != MS_SLAVE) {
+ if (type != PT_SLAVE) {
list_del_init(&mnt->mnt_slave);
mnt->mnt_master = NULL;
- if (type == MS_UNBINDABLE)
+ if (type == PT_UNBINDABLE)
mnt->mnt_flags |= MNT_UNBINDABLE;
}
}
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h 2005-11-24 16:21:06.000000000 +0100
+++ linux/include/linux/fs.h 2005-11-24 16:36:44.000000000 +0100
@@ -104,10 +104,7 @@ extern int dir_notify_enable;
#define MS_REC 16384
#define MS_VERBOSE 32768
#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */
-#define MS_UNBINDABLE (1<<17) /* change to unbindable */
-#define MS_PRIVATE (1<<18) /* change to private */
-#define MS_SLAVE (1<<19) /* change to slave */
-#define MS_SHARED (1<<20) /* change to shared */
+#define MS_PROPAGATION (1<<17) /* change propagation property */
#define MS_ACTIVE (1<<30)
#define MS_NOUSER (1<<31)

Index: linux/fs/pnode.h
===================================================================
--- linux.orig/fs/pnode.h 2005-11-24 16:32:19.000000000 +0100
+++ linux/fs/pnode.h 2005-11-24 16:32:45.000000000 +0100
@@ -23,13 +23,20 @@
#define CL_MAKE_SHARED 0x08
#define CL_PROPAGATION 0x10

+enum propagation_type {
+ PT_UNBINDABLE,
+ PT_PRIVATE,
+ PT_SLAVE,
+ PT_SHARED,
+};
+
static inline void set_mnt_shared(struct vfsmount *mnt)
{
mnt->mnt_flags &= ~MNT_PNODE_MASK;
mnt->mnt_flags |= MNT_SHARED;
}

-void change_mnt_propagation(struct vfsmount *, int);
+void change_mnt_propagation(struct vfsmount *, enum propagation_type);
int propagate_mnt(struct vfsmount *, struct dentry *, struct vfsmount *,
struct list_head *);
int propagate_umount(struct list_head *);
Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c 2005-11-24 16:21:02.000000000 +0100
+++ linux/fs/namespace.c 2005-11-24 16:37:34.000000000 +0100
@@ -490,7 +490,7 @@ void umount_tree(struct vfsmount *mnt, i
list_del_init(&p->mnt_child);
if (p->mnt_parent != p)
mnt->mnt_mountpoint->d_mounted--;
- change_mnt_propagation(p, MS_PRIVATE);
+ change_mnt_propagation(p, PT_PRIVATE);
}
}

@@ -835,15 +835,25 @@ out_unlock:
/*
* recursively change the type of the mountpoint.
*/
-static int do_change_type(struct nameidata *nd, int flag)
+static int do_change_type(struct nameidata *nd, int recurse, char *name)
{
struct vfsmount *m, *mnt = nd->mnt;
- int recurse = flag & MS_REC;
- int type = flag & ~MS_REC;
+ enum propagation_type type;

if (nd->dentry != nd->mnt->mnt_root)
return -EINVAL;

+ if (strcmp(name, "unbindable") == 0)
+ type = PT_UNBINDABLE;
+ else if (strcmp(name, "private") == 0)
+ type = PT_PRIVATE;
+ else if (strcmp(name, "slave") == 0)
+ type = PT_SLAVE;
+ else if (strcmp(name, "shared") == 0)
+ type = PT_SHARED;
+ else
+ return -EINVAL;
+
down_write(&namespace_sem);
spin_lock(&vfsmount_lock);
for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL))
@@ -1302,8 +1312,8 @@ long do_mount(char *dev_name, char *dir_
data_page);
else if (flags & MS_BIND)
retval = do_loopback(&nd, dev_name, flags & MS_REC);
- else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
- retval = do_change_type(&nd, flags);
+ else if (flags & MS_PROPAGATION)
+ retval = do_change_type(&nd, flags & MS_REC, data_page);
else if (flags & MS_MOVE)
retval = do_move_mount(&nd, dev_name);
else

2005-11-24 16:34:27

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

Duh, last patch was missing a check against NULL. Here's the fixed
one. I promise to review patches _before_ sending, next time.


Remaining mount flags are becoming scarce (just 11 bits)
and shared mount code uses 4 though one would suffice.

I think this should go into 2.6.15, fixing it later would be breaking
userspace ABI.

Signed-off-by: Miklos Szeredi <[email protected]>
---

Index: linux/fs/pnode.c
===================================================================
--- linux.orig/fs/pnode.c 2005-11-24 16:47:13.000000000 +0100
+++ linux/fs/pnode.c 2005-11-24 16:48:14.000000000 +0100
@@ -72,17 +72,17 @@ static int do_make_slave(struct vfsmount
return 0;
}

-void change_mnt_propagation(struct vfsmount *mnt, int type)
+void change_mnt_propagation(struct vfsmount *mnt, enum propagation_type type)
{
- if (type == MS_SHARED) {
+ if (type == PT_SHARED) {
set_mnt_shared(mnt);
return;
}
do_make_slave(mnt);
- if (type != MS_SLAVE) {
+ if (type != PT_SLAVE) {
list_del_init(&mnt->mnt_slave);
mnt->mnt_master = NULL;
- if (type == MS_UNBINDABLE)
+ if (type == PT_UNBINDABLE)
mnt->mnt_flags |= MNT_UNBINDABLE;
}
}
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h 2005-11-24 16:47:13.000000000 +0100
+++ linux/include/linux/fs.h 2005-11-24 16:48:14.000000000 +0100
@@ -104,10 +104,7 @@ extern int dir_notify_enable;
#define MS_REC 16384
#define MS_VERBOSE 32768
#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */
-#define MS_UNBINDABLE (1<<17) /* change to unbindable */
-#define MS_PRIVATE (1<<18) /* change to private */
-#define MS_SLAVE (1<<19) /* change to slave */
-#define MS_SHARED (1<<20) /* change to shared */
+#define MS_PROPAGATION (1<<17) /* change propagation property */
#define MS_ACTIVE (1<<30)
#define MS_NOUSER (1<<31)

Index: linux/fs/pnode.h
===================================================================
--- linux.orig/fs/pnode.h 2005-11-24 16:47:13.000000000 +0100
+++ linux/fs/pnode.h 2005-11-24 16:48:14.000000000 +0100
@@ -23,13 +23,20 @@
#define CL_MAKE_SHARED 0x08
#define CL_PROPAGATION 0x10

+enum propagation_type {
+ PT_UNBINDABLE,
+ PT_PRIVATE,
+ PT_SLAVE,
+ PT_SHARED,
+};
+
static inline void set_mnt_shared(struct vfsmount *mnt)
{
mnt->mnt_flags &= ~MNT_PNODE_MASK;
mnt->mnt_flags |= MNT_SHARED;
}

-void change_mnt_propagation(struct vfsmount *, int);
+void change_mnt_propagation(struct vfsmount *, enum propagation_type);
int propagate_mnt(struct vfsmount *, struct dentry *, struct vfsmount *,
struct list_head *);
int propagate_umount(struct list_head *);
Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c 2005-11-24 16:47:13.000000000 +0100
+++ linux/fs/namespace.c 2005-11-24 17:24:58.000000000 +0100
@@ -490,7 +490,7 @@ void umount_tree(struct vfsmount *mnt, i
list_del_init(&p->mnt_child);
if (p->mnt_parent != p)
mnt->mnt_mountpoint->d_mounted--;
- change_mnt_propagation(p, MS_PRIVATE);
+ change_mnt_propagation(p, PT_PRIVATE);
}
}

@@ -835,15 +835,28 @@ out_unlock:
/*
* recursively change the type of the mountpoint.
*/
-static int do_change_type(struct nameidata *nd, int flag)
+static int do_change_type(struct nameidata *nd, int recurse, char *name)
{
struct vfsmount *m, *mnt = nd->mnt;
- int recurse = flag & MS_REC;
- int type = flag & ~MS_REC;
+ enum propagation_type type;

if (nd->dentry != nd->mnt->mnt_root)
return -EINVAL;

+ if (!name)
+ return -EINVAL;
+
+ if (strcmp(name, "unbindable") == 0)
+ type = PT_UNBINDABLE;
+ else if (strcmp(name, "private") == 0)
+ type = PT_PRIVATE;
+ else if (strcmp(name, "slave") == 0)
+ type = PT_SLAVE;
+ else if (strcmp(name, "shared") == 0)
+ type = PT_SHARED;
+ else
+ return -EINVAL;
+
down_write(&namespace_sem);
spin_lock(&vfsmount_lock);
for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL))
@@ -1302,8 +1315,8 @@ long do_mount(char *dev_name, char *dir_
data_page);
else if (flags & MS_BIND)
retval = do_loopback(&nd, dev_name, flags & MS_REC);
- else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
- retval = do_change_type(&nd, flags);
+ else if (flags & MS_PROPAGATION)
+ retval = do_change_type(&nd, flags & MS_REC, data_page);
else if (flags & MS_MOVE)
retval = do_move_mount(&nd, dev_name);
else

2005-11-27 05:55:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

Miklos Szeredi <[email protected]> wrote:
>
> Remaining mount flags are becoming scarce (just 11 bits)
> and shared mount code uses 4 though one would suffice.
>
> I think this should go into 2.6.15, fixing it later would be breaking
> userspace ABI.

These seem sane objectives.

> -static int do_change_type(struct nameidata *nd, int flag)
> +static int do_change_type(struct nameidata *nd, int recurse, char *name)
> {
> struct vfsmount *m, *mnt = nd->mnt;
> - int recurse = flag & MS_REC;
> - int type = flag & ~MS_REC;
> + enum propagation_type type;
>
> if (nd->dentry != nd->mnt->mnt_root)
> return -EINVAL;
>
> + if (!name)
> + return -EINVAL;
> +
> + if (strcmp(name, "unbindable") == 0)
> + type = PT_UNBINDABLE;
> + else if (strcmp(name, "private") == 0)
> + type = PT_PRIVATE;
> + else if (strcmp(name, "slave") == 0)
> + type = PT_SLAVE;
> + else if (strcmp(name, "shared") == 0)
> + type = PT_SHARED;
> + else
> + return -EINVAL;
> +
> down_write(&namespace_sem);
> spin_lock(&vfsmount_lock);
> for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL))
> @@ -1302,8 +1315,8 @@ long do_mount(char *dev_name, char *dir_
> data_page);
> else if (flags & MS_BIND)
> retval = do_loopback(&nd, dev_name, flags & MS_REC);
> - else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> - retval = do_change_type(&nd, flags);
> + else if (flags & MS_PROPAGATION)
> + retval = do_change_type(&nd, flags & MS_REC, data_page);
> else if (flags & MS_MOVE)
> retval = do_move_mount(&nd, dev_name);
> else

But I don't know how much trauma this would cause. Hasn't util-linux
already been patched with the new mount flags?

If it has, and if it uses the same names for these options, the patched
mount(8) just won't work.

The proposed new mount options should be documented somewhere.

Anyway, I'll let Ram&Al decide on this proposal.

2005-11-27 06:32:21

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

On Sat, Nov 26, 2005 at 09:55:09PM -0800, Andrew Morton wrote:
> > - else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> > - retval = do_change_type(&nd, flags);
> > + else if (flags & MS_PROPAGATION)
> > + retval = do_change_type(&nd, flags & MS_REC, data_page);
> > else if (flags & MS_MOVE)
> > retval = do_move_mount(&nd, dev_name);
> > else
>
> But I don't know how much trauma this would cause. Hasn't util-linux
> already been patched with the new mount flags?
>
> If it has, and if it uses the same names for these options, the patched
> mount(8) just won't work.
>
> The proposed new mount options should be documented somewhere.
>
> Anyway, I'll let Ram&Al decide on this proposal.

It's
a) palliative
b) ugly

Let's face it, mount(2) ABI is getting past its shelf life already.
We'll need saner replacement (not mixing action with the flags and
being really typed) anyway, so let's not introduce more kludges into
mount(2) already messy situation - it's not worth the effort.

2005-11-27 09:35:05

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

> > Anyway, I'll let Ram&Al decide on this proposal.
>
> It's
> a) palliative
> b) ugly
>
> Let's face it, mount(2) ABI is getting past its shelf life already.
> We'll need saner replacement (not mixing action with the flags and
> being really typed) anyway,

Could you please ellaborate a bit more?

Are you thinking of a separate syscall for each action?

Miklos

2005-11-28 12:14:51

by Ram Pai

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

On Sat, 2005-11-26 at 21:55, Andrew Morton wrote:
> Miklos Szeredi <[email protected]> wrote:
> >
> > Remaining mount flags are becoming scarce (just 11 bits)
> > and shared mount code uses 4 though one would suffice.
> >
> > I think this should go into 2.6.15, fixing it later would be breaking
> > userspace ABI.
>
> These seem sane objectives.
>
> > -static int do_change_type(struct nameidata *nd, int flag)
> > +static int do_change_type(struct nameidata *nd, int recurse, char *name)
> > {
> > struct vfsmount *m, *mnt = nd->mnt;
> > - int recurse = flag & MS_REC;
> > - int type = flag & ~MS_REC;
> > + enum propagation_type type;
> >
> > if (nd->dentry != nd->mnt->mnt_root)
> > return -EINVAL;
> >
> > + if (!name)
> > + return -EINVAL;
> > +
> > + if (strcmp(name, "unbindable") == 0)
> > + type = PT_UNBINDABLE;
> > + else if (strcmp(name, "private") == 0)
> > + type = PT_PRIVATE;
> > + else if (strcmp(name, "slave") == 0)
> > + type = PT_SLAVE;
> > + else if (strcmp(name, "shared") == 0)
> > + type = PT_SHARED;
> > + else
> > + return -EINVAL;
> > +
> > down_write(&namespace_sem);
> > spin_lock(&vfsmount_lock);
> > for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL))
> > @@ -1302,8 +1315,8 @@ long do_mount(char *dev_name, char *dir_
> > data_page);
> > else if (flags & MS_BIND)
> > retval = do_loopback(&nd, dev_name, flags & MS_REC);
> > - else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> > - retval = do_change_type(&nd, flags);
> > + else if (flags & MS_PROPAGATION)
> > + retval = do_change_type(&nd, flags & MS_REC, data_page);
> > else if (flags & MS_MOVE)
> > retval = do_move_mount(&nd, dev_name);
> > else
>
> But I don't know how much trauma this would cause. Hasn't util-linux
> already been patched with the new mount flags?

Andrew,
No. The new mount flags have not yet been picked up by
util-linux AFAIK.

and again with shared subtree semantics mount/umount command
can no way handle all the implicit mounts/unmounts that take
place without its knowledge. Dependence on /etc/mnttab is already
broken with namespaces. Shared-subtree adds some more misery.

I will work on that once I am back from vacation,
RP



>
> If it has, and if it uses the same names for these options, the patched
> mount(8) just won't work.
>
> The proposed new mount options should be documented somewhere.
>
> Anyway, I'll let Ram&Al decide on this proposal.

2005-11-29 12:31:11

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH 2/2] shared mounts: save mount flag space

On Saturday 26 November 2005 23:55, Andrew Morton wrote:
> The proposed new mount options should be documented somewhere.

Yeah, busybox mount would like to implement them when this all calms down.

Rob
--
Steve Ballmer: Innovation! Inigo Montoya: You keep using that word.
I do not think it means what you think it means.