2008-03-13 21:28:36

by Miklos Szeredi

[permalink] [raw]
Subject: [patch 4/6] vfs: mountinfo show dominating group id

From: Miklos Szeredi <[email protected]>

Show peer group ID of nearest dominating group that has intersection
with the mount's namespace.

See the the documentation update for details.

Signed-off-by: Miklos Szeredi <[email protected]>
---
Documentation/filesystems/proc.txt | 11 ++++++++---
fs/namespace.c | 24 ++++++++++++++++--------
fs/pnode.c | 32 ++++++++++++++++++++++++++++++++
fs/pnode.h | 1 +
4 files changed, 57 insertions(+), 11 deletions(-)

Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c 2008-03-13 20:45:50.000000000 +0100
+++ linux/fs/namespace.c 2008-03-13 20:45:51.000000000 +0100
@@ -798,16 +798,24 @@ static int show_mountinfo(struct seq_fil
show_sb_opts(m, sb);
if (sb->s_op->show_options)
err = sb->s_op->show_options(m, mnt);
- if (IS_MNT_SHARED(mnt)) {
- seq_printf(m, " shared:%i", get_peer_group_id(mnt));
- if (IS_MNT_SLAVE(mnt))
- seq_printf(m, ",slave:%i", get_master_id(mnt));
- } else if (IS_MNT_SLAVE(mnt)) {
- seq_printf(m, " slave:%i", get_master_id(mnt));
+ seq_putc(m, ' ');
+ if (IS_MNT_SHARED(mnt) || IS_MNT_SLAVE(mnt)) {
+ if (IS_MNT_SHARED(mnt))
+ seq_printf(m, "shared:%i", get_peer_group_id(mnt));
+ if (IS_MNT_SLAVE(mnt)) {
+ int dominator_id = get_dominator_id_same_ns(mnt);
+
+ if (IS_MNT_SHARED(mnt))
+ seq_putc(m, ',');
+
+ seq_printf(m, "slave:%i", get_master_id(mnt));
+ if (dominator_id != -1)
+ seq_printf(m, ":%i", dominator_id);
+ }
} else if (IS_MNT_UNBINDABLE(mnt)) {
- seq_printf(m, " unbindable");
+ seq_printf(m, "unbindable");
} else {
- seq_printf(m, " private");
+ seq_printf(m, "private");
}
seq_putc(m, '\n');
return err;
Index: linux/fs/pnode.c
===================================================================
--- linux.orig/fs/pnode.c 2008-03-13 20:45:50.000000000 +0100
+++ linux/fs/pnode.c 2008-03-13 20:45:51.000000000 +0100
@@ -88,6 +88,38 @@ int get_master_id(struct vfsmount *mnt)
return id;
}

+static struct vfsmount *get_peer_in_ns(struct vfsmount *mnt,
+ struct mnt_namespace *ns)
+{
+ struct vfsmount *m = mnt;
+
+ do {
+ if (m->mnt_ns == ns)
+ return m;
+ m = next_peer(m);
+ } while (m != mnt);
+
+ return NULL;
+}
+
+int get_dominator_id_same_ns(struct vfsmount *mnt)
+{
+ int id = -1;
+ struct vfsmount *m;
+
+ spin_lock(&vfsmount_lock);
+ for (m = mnt->mnt_master; m != NULL; m = m->mnt_master) {
+ struct vfsmount *d = get_peer_in_ns(m, mnt->mnt_ns);
+ if (d) {
+ id = d->mnt_pgid;
+ break;
+ }
+ }
+ spin_unlock(&vfsmount_lock);
+
+ return id;
+}
+
static int do_make_slave(struct vfsmount *mnt)
{
struct vfsmount *peer_mnt = mnt, *master = mnt->mnt_master;
Index: linux/fs/pnode.h
===================================================================
--- linux.orig/fs/pnode.h 2008-03-13 20:45:50.000000000 +0100
+++ linux/fs/pnode.h 2008-03-13 20:45:51.000000000 +0100
@@ -33,4 +33,5 @@ int propagate_umount(struct list_head *)
int propagate_mount_busy(struct vfsmount *, int);
int get_peer_group_id(struct vfsmount *);
int get_master_id(struct vfsmount *);
+int get_dominator_id_same_ns(struct vfsmount *);
#endif /* _LINUX_PNODE_H */
Index: linux/Documentation/filesystems/proc.txt
===================================================================
--- linux.orig/Documentation/filesystems/proc.txt 2008-03-13 20:45:50.000000000 +0100
+++ linux/Documentation/filesystems/proc.txt 2008-03-13 20:45:51.000000000 +0100
@@ -2367,15 +2367,20 @@ MNTOPTS: per mount options
SBOPTS: per super block options
PROPAGATION: propagation type

-propagation type: <propagation_flag>[:<peergrpid>][,...]
+propagation type: <propagation_flag>[:<peergrpid>[:<domgrpid>]][,...]
note: 'shared' flag is followed by the id of this mount's peer group
- 'slave' flag is followed by the peer group id of its master mount
+ 'slave' flag is followed by the peer group id of its master mount,
+ optionally followed by the id of the closest dominant(*)
+ peer group in the same namespace, if one exists.
'private' flag stands by itself
'unbindable' flag stands by itself

+(*) A dominant peer group is an ancestor of this mount in the
+propagation tree, in other words, this mount receives propagation from
+the dominant peer group, but not the other way round.
+
For more information see:

Documentation/filesystems/sharedsubtree.txt

-
------------------------------------------------------------------------------

--


2008-03-19 21:19:33

by Al Viro

[permalink] [raw]
Subject: Re: [patch 4/6] vfs: mountinfo show dominating group id

On Wed, Mar 19, 2008 at 01:19:42PM +0100, Miklos Szeredi wrote:
> > So maybe some alternative, multi line format would be better?
> >
> > MountID: 99
> > ParentID: 88
> > DevID: 0:34
> > Type: foofs
> > Source: /dev/foo
> > Root: /
> > MountPoint: /mnt/foo
> > MountOpts: rw,noatime
> > Opts: rw,errors=continue
> > Propagation: shared:42
>
> Which still doesn't fully solve the problem, since ->show_options()
> can also spew newlines + MountID:. Oh well.

a) ban newlines in ->show_options(); that's a requirement that is easy
to formulate and understand, so it has a chance to survive the contact
with reality.

b) the order is all wrong - *everything* that depends on fs type should
be after fs type and everything else should be prior to it. That way
you don't need to know what the hell does this fs type spew in order to
parse type-independent information. In particular, "source" (BTW, why
do you capitalize those?) certainly has no business being in front of
fs type; as the matter of fact, I'm not at all sure that we _want_ it
separated from the rest of type-dependent options. The fact that mount(2)
gets it in a separate argument is a historical accident...

c) since you are tagging the fields anyway, why do you need newlines?
Moreover, you don't really need to tag everything - there's a well-defined
beginning and optional fields between it and (type+rest) are the only
things that needs to be tagged... BTW, why bother with Propagation: part
and gluing shared:... with slave:... into a single field? Separate them
with whitespace - you have recognizable prefixes right there.

2008-03-19 21:23:45

by Al Viro

[permalink] [raw]
Subject: Re: [patch 4/6] vfs: mountinfo show dominating group id

On Thu, Mar 13, 2008 at 10:26:45PM +0100, Miklos Szeredi wrote:
> From: Miklos Szeredi <[email protected]>
>
> Show peer group ID of nearest dominating group that has intersection
> with the mount's namespace.

There's an obvious problem here: ->show_options() can spew _anything_,
including a string that ends on " shared:42". Makes reliable parsing of
the damn thing in userland impossible. IOW, fs options should go _last_
and they should follow an unconditionally present field.

> if (sb->s_op->show_options)
> err = sb->s_op->show_options(m, mnt);
> - if (IS_MNT_SHARED(mnt)) {
> - seq_printf(m, " shared:%i", get_peer_group_id(mnt));
> - if (IS_MNT_SLAVE(mnt))
> - seq_printf(m, ",slave:%i", get_master_id(mnt));
> - } else if (IS_MNT_SLAVE(mnt)) {
> - seq_printf(m, " slave:%i", get_master_id(mnt));
> + seq_putc(m, ' ');
> + if (IS_MNT_SHARED(mnt) || IS_MNT_SLAVE(mnt)) {
> + if (IS_MNT_SHARED(mnt))
> + seq_printf(m, "shared:%i", get_peer_group_id(mnt));
> + if (IS_MNT_SLAVE(mnt)) {
> + int dominator_id = get_dominator_id_same_ns(mnt);

2008-03-20 00:18:28

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [patch 4/6] vfs: mountinfo show dominating group id

> > > So maybe some alternative, multi line format would be better?
> > >
> > > MountID: 99
> > > ParentID: 88
> > > DevID: 0:34
> > > Type: foofs
> > > Source: /dev/foo
> > > Root: /
> > > MountPoint: /mnt/foo
> > > MountOpts: rw,noatime
> > > Opts: rw,errors=continue
> > > Propagation: shared:42
> >
> > Which still doesn't fully solve the problem, since ->show_options()
> > can also spew newlines + MountID:. Oh well.
>
> a) ban newlines in ->show_options(); that's a requirement that is easy
> to formulate and understand, so it has a chance to survive the contact
> with reality.
>
> b) the order is all wrong - *everything* that depends on fs type should
> be after fs type and everything else should be prior to it. That way
> you don't need to know what the hell does this fs type spew in order to
> parse type-independent information. In particular, "source" (BTW, why
> do you capitalize those?) certainly has no business being in front of
> fs type; as the matter of fact, I'm not at all sure that we _want_ it
> separated from the rest of type-dependent options. The fact that mount(2)
> gets it in a separate argument is a historical accident...
>
> c) since you are tagging the fields anyway, why do you need newlines?
> Moreover, you don't really need to tag everything - there's a well-defined
> beginning and optional fields between it and (type+rest) are the only
> things that needs to be tagged... BTW, why bother with Propagation: part
> and gluing shared:... with slave:... into a single field? Separate them
> with whitespace - you have recognizable prefixes right there.
>

99 88 0:34 / /mnt/foo rw,noatime shared:42 slave:13 foofs /dev/foo,errors=continue

Something like that? It assumes, that fs types never have ':' in
them, but that's acceptable.

Miklos

2008-03-20 00:19:48

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [patch 4/6] vfs: mountinfo show dominating group id

> > Makes reliable parsing of
> > the damn thing in userland impossible. IOW, fs options should go _last_
> > and they should follow an unconditionally present field.
>
> But then we lose the ability to later extend the format by adding
> fields at the end. Which is one of the things that would be nice to
> have, in contrast to /proc/mounts, which we are so afraid to touch now.
>
> So maybe some alternative, multi line format would be better?
>
> MountID: 99
> ParentID: 88
> DevID: 0:34
> Type: foofs
> Source: /dev/foo
> Root: /
> MountPoint: /mnt/foo
> MountOpts: rw,noatime
> Opts: rw,errors=continue
> Propagation: shared:42

Which still doesn't fully solve the problem, since ->show_options()
can also spew newlines + MountID:. Oh well.

Miklos

2008-03-20 00:21:56

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [patch 4/6] vfs: mountinfo show dominating group id

> > From: Miklos Szeredi <[email protected]>
> >
> > Show peer group ID of nearest dominating group that has intersection
> > with the mount's namespace.
>
> There's an obvious problem here: ->show_options() can spew _anything_,
> including a string that ends on " shared:42".

Yeah, even though I'd call that very broken, I wouldn't like to go
auditing filesystems for such breakage.

> Makes reliable parsing of
> the damn thing in userland impossible. IOW, fs options should go _last_
> and they should follow an unconditionally present field.

But then we lose the ability to later extend the format by adding
fields at the end. Which is one of the things that would be nice to
have, in contrast to /proc/mounts, which we are so afraid to touch now.

So maybe some alternative, multi line format would be better?

MountID: 99
ParentID: 88
DevID: 0:34
Type: foofs
Source: /dev/foo
Root: /
MountPoint: /mnt/foo
MountOpts: rw,noatime
Opts: rw,errors=continue
Propagation: shared:42

Miklos