[RFC PATCH] mountinfo: show only reachable mounts

[RFC PATCH] mountinfo: show only reachable mounts

[Miklos Szeredi]

[One more RFC for mountinfo, and then I'll really send them to
Andrew.]

Two changes:

1. Only show reachable mounts in /proc/<pid>/mountinfo, this makes
mountpoints unambiguous for chrooted processes.

2. Instead of showing mountpoints relative to the current root, always
show them relative to the queried task's root.

This means, that a particular mountinfo file will always have the same
contents, regardless of which process is reading the file.  Which is a
lot more consistent, than the current behavior of /proc/<pid>/mounts.

Comments?

Thanks,
Miklos

Index: linux/fs/dcache.c
===================================================================
--- linux.orig/fs/dcache.c	2008-03-11 12:40:55.000000000 +0100
+++ linux/fs/dcache.c	2008-03-11 16:22:08.000000000 +0100
@@ -1774,9 +1774,10 @@ static int prepend(char **buffer, int *b
  *
  * "buflen" should be positive. Caller holds the dcache_lock.
  */
-static char *__d_path(struct dentry *dentry, struct vfsmount *vfsmnt,
-		       struct path *root, char *buffer, int buflen)
+char *__d_path(struct path *path, struct path *root, char *buffer, int buflen)
 {
+	struct dentry *dentry = path->dentry;
+	struct vfsmount *vfsmnt = path->mnt;
 	char * end = buffer+buflen;
 	char * retval;
 
@@ -1863,7 +1864,7 @@ char *d_path(struct path *path, char *bu
 	path_get(&current->fs->root);
 	read_unlock(&current->fs->lock);
 	spin_lock(&dcache_lock);
-	res = __d_path(path->dentry, path->mnt, &root, buf, buflen);
+	res = __d_path(path, &root, buf, buflen);
 	spin_unlock(&dcache_lock);
 	path_put(&root);
 	return res;
@@ -1976,7 +1977,7 @@ asmlinkage long sys_getcwd(char __user *
 		unsigned long len;
 		char * cwd;
 
-		cwd = __d_path(pwd.dentry, pwd.mnt, &root, page, PAGE_SIZE);
+		cwd = __d_path(&pwd, &root, page, PAGE_SIZE);
 		spin_unlock(&dcache_lock);
 
 		error = PTR_ERR(cwd);
Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c	2008-03-11 12:39:15.000000000 +0100
+++ linux/fs/namespace.c	2008-03-11 17:00:11.000000000 +0100
@@ -683,17 +683,17 @@ EXPORT_SYMBOL(save_mount_options);
 /* iterator */
 static void *m_start(struct seq_file *m, loff_t *pos)
 {
-	struct mnt_namespace *n = m->private;
+	struct proc_mounts *p = m->private;
 
 	down_read(&namespace_sem);
-	return seq_list_start(&n->list, *pos);
+	return seq_list_start(&p->ns->list, *pos);
 }
 
 static void *m_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct mnt_namespace *n = m->private;
+	struct proc_mounts *p = m->private;
 
-	return seq_list_next(v, &n->list, pos);
+	return seq_list_next(v, &p->ns->list, pos);
 }
 
 static void m_stop(struct seq_file *m, void *v)
@@ -777,13 +777,52 @@ const struct seq_operations mounts_op = 
 	.show	= show_vfsmnt
 };
 
+/*
+ * Return true if 'path' is reachable from 'root'
+ */
+static bool is_path_reachable(const struct path *path, const struct path *root)
+{
+	struct dentry *dentry = path->dentry;
+	struct vfsmount *mnt = path->mnt;
+	bool res = false;
+
+	spin_lock(&dcache_lock);
+	for (;;) {
+		if (dentry == root->dentry && mnt == root->mnt) {
+			res = true;
+			break;
+		}
+		if (dentry == mnt->mnt_root || IS_ROOT(dentry)) {
+			/* Global root? */
+			spin_lock(&vfsmount_lock);
+			if (mnt->mnt_parent == mnt) {
+				spin_unlock(&vfsmount_lock);
+				break;
+			}
+			dentry = mnt->mnt_mountpoint;
+			mnt = mnt->mnt_parent;
+			spin_unlock(&vfsmount_lock);
+			continue;
+		}
+		dentry = dentry->d_parent;
+	}
+	spin_unlock(&dcache_lock);
+
+	return res;
+}
+
 static int show_mountinfo(struct seq_file *m, void *v)
 {
+	struct proc_mounts *p = m->private;
 	struct vfsmount *mnt = list_entry(v, struct vfsmount, mnt_list);
 	struct super_block *sb = mnt->mnt_sb;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	int err = 0;
 
+	/* Skip unreachable mounts */
+	if (!is_path_reachable(&mnt_path, &p->root))
+		return 0;
+
 	seq_printf(m, "%i %i %u:%u ", mnt->mnt_id, mnt->mnt_parent->mnt_id,
 		   MAJOR(sb->s_dev), MINOR(sb->s_dev));
 	show_type(m, sb);
@@ -792,7 +831,7 @@ static int show_mountinfo(struct seq_fil
 	seq_putc(m, ' ');
 	seq_dentry(m, mnt->mnt_root, " \t\n\\");
 	seq_putc(m, ' ');
-	seq_path(m, &mnt_path, " \t\n\\");
+	seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
 	seq_puts(m, mnt->mnt_flags & MNT_READONLY ? " ro" : " rw");
 	show_mnt_opts(m, mnt);
 	seq_puts(m, sb->s_flags & MS_RDONLY ? " ro" : " rw");
Index: linux/fs/proc/base.c
===================================================================
--- linux.orig/fs/proc/base.c	2008-03-11 12:38:48.000000000 +0100
+++ linux/fs/proc/base.c	2008-03-11 16:22:08.000000000 +0100
@@ -502,17 +502,14 @@ static const struct inode_operations pro
 	.setattr	= proc_setattr,
 };
 
-struct proc_mounts {
-	struct seq_file m;
-	int event;
-};
-
 static int mounts_open_common(struct inode *inode, struct file *file,
 			      const struct seq_operations *op)
 {
 	struct task_struct *task = get_proc_task(inode);
 	struct nsproxy *nsp;
 	struct mnt_namespace *ns = NULL;
+	struct fs_struct *fs = NULL;
+	struct path root;
 	struct proc_mounts *p;
 	int ret = -EINVAL;
 
@@ -525,40 +522,61 @@ static int mounts_open_common(struct ino
 				get_mnt_ns(ns);
 		}
 		rcu_read_unlock();
-
+		if (ns)
+			fs = get_fs_struct(task);
 		put_task_struct(task);
 	}
 
-	if (ns) {
-		ret = -ENOMEM;
-		p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);
-		if (p) {
-			file->private_data = &p->m;
-			ret = seq_open(file, op);
-			if (!ret) {
-				p->m.private = ns;
-				p->event = ns->event;
-				return 0;
-			}
-			kfree(p);
-		}
-		put_mnt_ns(ns);
-	}
+	if (!ns)
+		goto err;
+	if (!fs)
+		goto err_put_ns;
+
+	read_lock(&fs->lock);
+	root = fs->root;
+	path_get(&root);
+	read_unlock(&fs->lock);
+	put_fs_struct(fs);
+
+	ret = -ENOMEM;
+	p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);
+	if (!p)
+		goto err_put_path;
+
+	file->private_data = &p->m;
+	ret = seq_open(file, op);
+	if (ret)
+		goto err_free;
+
+	p->m.private = p;
+	p->ns = ns;
+	p->root = root;
+	p->event = ns->event;
+
+	return 0;
+
+ err_free:
+	kfree(p);
+ err_put_path:
+	path_put(&root);
+ err_put_ns:
+	put_mnt_ns(ns);
+ err:
 	return ret;
 }
 
 static int mounts_release(struct inode *inode, struct file *file)
 {
-	struct seq_file *m = file->private_data;
-	struct mnt_namespace *ns = m->private;
-	put_mnt_ns(ns);
+	struct proc_mounts *p = file->private_data;
+	path_put(&p->root);
+	put_mnt_ns(p->ns);
 	return seq_release(inode, file);
 }
 
 static unsigned mounts_poll(struct file *file, poll_table *wait)
 {
 	struct proc_mounts *p = file->private_data;
-	struct mnt_namespace *ns = p->m.private;
+	struct mnt_namespace *ns = p->ns;
 	unsigned res = 0;
 
 	poll_wait(file, &ns->poll, wait);
Index: linux/fs/seq_file.c
===================================================================
--- linux.orig/fs/seq_file.c	2008-03-11 12:39:15.000000000 +0100
+++ linux/fs/seq_file.c	2008-03-11 16:22:08.000000000 +0100
@@ -392,6 +392,29 @@ int seq_path(struct seq_file *m, struct 
 EXPORT_SYMBOL(seq_path);
 
 #ifdef CONFIG_PROC_FS
+int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
+		  char *esc)
+{
+	if (m->count < m->size) {
+		char *s = m->buf + m->count;
+		char *p;
+
+		spin_lock(&dcache_lock);
+		p = __d_path(path, root, s, m->size - m->count);
+		spin_unlock(&dcache_lock);
+		if (!IS_ERR(p)) {
+			s = mangle_path(s, p, esc);
+			if (s) {
+				p = m->buf + m->count;
+				m->count = s - m->buf;
+				return s - p;
+			}
+		}
+	}
+	m->count = m->size;
+	return -1;
+}
+
 /*
  * returns the path of the 'dentry' from the root of its filesystem.
  */
Index: linux/include/linux/dcache.h
===================================================================
--- linux.orig/include/linux/dcache.h	2008-03-11 12:39:15.000000000 +0100
+++ linux/include/linux/dcache.h	2008-03-11 16:22:08.000000000 +0100
@@ -302,6 +302,7 @@ extern int d_validate(struct dentry *, s
  */
 extern char *dynamic_dname(struct dentry *, char *, int, const char *, ...);
 
+extern char *__d_path(struct path *path, struct path *root, char *, int);
 extern char *d_path(struct path *, char *, int);
 
 #ifdef CONFIG_PROC_FS
Index: linux/include/linux/mnt_namespace.h
===================================================================
--- linux.orig/include/linux/mnt_namespace.h	2008-03-11 12:38:48.000000000 +0100
+++ linux/include/linux/mnt_namespace.h	2008-03-11 16:22:08.000000000 +0100
@@ -5,6 +5,7 @@
 #include <linux/mount.h>
 #include <linux/sched.h>
 #include <linux/nsproxy.h>
+#include <linux/seq_file.h>
 
 struct mnt_namespace {
 	atomic_t		count;
@@ -14,6 +15,13 @@ struct mnt_namespace {
 	int event;
 };
 
+struct proc_mounts {
+	struct seq_file m; /* must be the first element */
+	struct mnt_namespace *ns;
+	struct path root;
+	int event;
+};
+
 extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *,
 		struct fs_struct *);
 extern void __put_mnt_ns(struct mnt_namespace *ns);
Index: linux/include/linux/seq_file.h
===================================================================
--- linux.orig/include/linux/seq_file.h	2008-03-11 12:39:15.000000000 +0100
+++ linux/include/linux/seq_file.h	2008-03-11 16:22:08.000000000 +0100
@@ -46,6 +46,8 @@ int seq_printf(struct seq_file *, const 
 int seq_path(struct seq_file *, struct path *, char *);
 
 #ifdef CONFIG_PROC_FS
+int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
+		  char *esc);
 int seq_dentry(struct seq_file *, struct dentry *, char *);
 #endif /* CONFIG_PROC_FS */
 

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Jan Blunck]

On Tue, 11 Mar 2008 17:17:37 +0100, Miklos Szeredi wrote:

> +/*
> + * Return true if 'path' is reachable from 'root' + */
> +static bool is_path_reachable(const struct path *path, const struct
> path *root) +{
> +	struct dentry *dentry = path->dentry; +	struct vfsmount *mnt =
> path->mnt;
> +	bool res = false;
> +
> +	spin_lock(&dcache_lock);
> +	for (;;) {
> +		if (dentry == root->dentry && mnt == root->mnt) 
{ +			res = true;
> +			break;
> +		}
> +		if (dentry == mnt->mnt_root || IS_ROOT(dentry)) 
{ +			/* Global root?
> */
> +			spin_lock(&vfsmount_lock);
> +			if (mnt->mnt_parent == mnt) {
> +				spin_unlock(&vfsmount_lock);
> +				break;
> +			}
> +			dentry = mnt->mnt_mountpoint;
> +			mnt = mnt->mnt_parent;
> +			spin_unlock(&vfsmount_lock);
> +			continue;
> +		}
> +		dentry = dentry->d_parent;
> +	}
> +	spin_unlock(&dcache_lock);
> +
> +	return res;
> +}
> +

Hmm, this may hold dcache_lock for some time. Isn't it enough to use 
rcu_readlock() and read_seqbegin(&rename_lock) ?

Cheers,
Jan

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Miklos Szeredi]

> > +/*
> > + * Return true if 'path' is reachable from 'root' + */
> > +static bool is_path_reachable(const struct path *path, const struct
> > path *root) +{
> > +	struct dentry *dentry = path->dentry; +	struct vfsmount *mnt =
> > path->mnt;
> > +	bool res = false;
> > +
> > +	spin_lock(&dcache_lock);
> > +	for (;;) {
> > +		if (dentry == root->dentry && mnt == root->mnt) 
> { +			res = true;
> > +			break;
> > +		}
> > +		if (dentry == mnt->mnt_root || IS_ROOT(dentry)) 
> { +			/* Global root?
> > */
> > +			spin_lock(&vfsmount_lock);
> > +			if (mnt->mnt_parent == mnt) {
> > +				spin_unlock(&vfsmount_lock);
> > +				break;
> > +			}
> > +			dentry = mnt->mnt_mountpoint;
> > +			mnt = mnt->mnt_parent;
> > +			spin_unlock(&vfsmount_lock);
> > +			continue;
> > +		}
> > +		dentry = dentry->d_parent;
> > +	}
> > +	spin_unlock(&dcache_lock);
> > +
> > +	return res;
> > +}
> > +
> 
> Hmm, this may hold dcache_lock for some time. Isn't it enough to use 
> rcu_readlock() and read_seqbegin(&rename_lock) ?

Maybe.  I just blindly copied the d_path() code without thinking much.
Is this worth optimizing?  I mean, getting RCU wrong is easy, and it
makes my head hurt to think about it.  And dcache_lock shouldn't be
used in any performance critical code paths anyway.

Thanks,
Miklos

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Miklos Szeredi]

> > Hmm, this may hold dcache_lock for some time. Isn't it enough to use 
> > rcu_readlock() and read_seqbegin(&rename_lock) ?
> 
> Maybe.  I just blindly copied the d_path() code without thinking much.
> Is this worth optimizing?  I mean, getting RCU wrong is easy, and it
> makes my head hurt to think about it.  And dcache_lock shouldn't be
> used in any performance critical code paths anyway.

But I'm an idiot.  This is a much simpler and faster implementation
that uses is_subdir(), which essentially does what you suggested:

/*
 * Return true if 'path' is reachable from 'root'
 */
static bool is_path_reachable(const struct path *path, const struct path *root)
{
	struct dentry *dentry = path->dentry;
	struct vfsmount *mnt = path->mnt;
	bool res = false;

	spin_lock(&vfsmount_lock);
	while (mnt != root->mnt && mnt->mnt_parent != mnt) {
		dentry = mnt->mnt_mountpoint;
		mnt = mnt->mnt_parent;
	}
	if (mnt == root->mnt && is_subdir(dentry, root->dentry))
		res = true;
	spin_unlock(&vfsmount_lock);

	return res;
}

Miklos

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Jan Blunck]

On Thu, Mar 13, Miklos Szeredi wrote:

> But I'm an idiot.  This is a much simpler and faster implementation
> that uses is_subdir(), which essentially does what you suggested:
> 
> /*
>  * Return true if 'path' is reachable from 'root'
>  */
> static bool is_path_reachable(const struct path *path, const struct path *root)
> {
> 	struct dentry *dentry = path->dentry;
> 	struct vfsmount *mnt = path->mnt;
> 	bool res = false;
> 
> 	spin_lock(&vfsmount_lock);
> 	while (mnt != root->mnt && mnt->mnt_parent != mnt) {
> 		dentry = mnt->mnt_mountpoint;
> 		mnt = mnt->mnt_parent;
> 	}
> 	if (mnt == root->mnt && is_subdir(dentry, root->dentry))
> 		res = true;
> 	spin_unlock(&vfsmount_lock);
> 
> 	return res;
> }

Very well.

BTW, I have a patch rotting in my quilt stack to remove
lives_below_in_same_fs() since is_subdir() provides the same
functionality.

Cheers,
Jan

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Ram Pai]

On Tue, 2008-03-11 at 17:17 +0100, Miklos Szeredi wrote:
> [One more RFC for mountinfo, and then I'll really send them to
> Andrew.]
> 
> Two changes:
> 
> 1. Only show reachable mounts in /proc/<pid>/mountinfo, this makes
> mountpoints unambiguous for chrooted processes.
> 
> 2. Instead of showing mountpoints relative to the current root, always
> show them relative to the queried task's root.
> 
> This means, that a particular mountinfo file will always have the same
> contents, regardless of which process is reading the file.  Which is a
> lot more consistent, than the current behavior of /proc/<pid>/mounts.


What about the propagation aspect?  Won't you have to show only those
master mount pgid's that have a mounts within the same chrooted tree?

RP


> 
> Comments?
> 
> Thanks,
> Miklos
> 
> Index: linux/fs/dcache.c
> ===================================================================
> --- linux.orig/fs/dcache.c	2008-03-11 12:40:55.000000000 +0100
> +++ linux/fs/dcache.c	2008-03-11 16:22:08.000000000 +0100
> @@ -1774,9 +1774,10 @@ static int prepend(char **buffer, int *b
>   *
>   * "buflen" should be positive. Caller holds the dcache_lock.
>   */
> -static char *__d_path(struct dentry *dentry, struct vfsmount *vfsmnt,
> -		       struct path *root, char *buffer, int buflen)
> +char *__d_path(struct path *path, struct path *root, char *buffer, int buflen)
>  {
> +	struct dentry *dentry = path->dentry;
> +	struct vfsmount *vfsmnt = path->mnt;
>  	char * end = buffer+buflen;
>  	char * retval;
> 
> @@ -1863,7 +1864,7 @@ char *d_path(struct path *path, char *bu
>  	path_get(&current->fs->root);
>  	read_unlock(&current->fs->lock);
>  	spin_lock(&dcache_lock);
> -	res = __d_path(path->dentry, path->mnt, &root, buf, buflen);
> +	res = __d_path(path, &root, buf, buflen);
>  	spin_unlock(&dcache_lock);
>  	path_put(&root);
>  	return res;
> @@ -1976,7 +1977,7 @@ asmlinkage long sys_getcwd(char __user *
>  		unsigned long len;
>  		char * cwd;
> 
> -		cwd = __d_path(pwd.dentry, pwd.mnt, &root, page, PAGE_SIZE);
> +		cwd = __d_path(&pwd, &root, page, PAGE_SIZE);
>  		spin_unlock(&dcache_lock);
> 
>  		error = PTR_ERR(cwd);
> Index: linux/fs/namespace.c
> ===================================================================
> --- linux.orig/fs/namespace.c	2008-03-11 12:39:15.000000000 +0100
> +++ linux/fs/namespace.c	2008-03-11 17:00:11.000000000 +0100
> @@ -683,17 +683,17 @@ EXPORT_SYMBOL(save_mount_options);
>  /* iterator */
>  static void *m_start(struct seq_file *m, loff_t *pos)
>  {
> -	struct mnt_namespace *n = m->private;
> +	struct proc_mounts *p = m->private;
> 
>  	down_read(&namespace_sem);
> -	return seq_list_start(&n->list, *pos);
> +	return seq_list_start(&p->ns->list, *pos);
>  }
> 
>  static void *m_next(struct seq_file *m, void *v, loff_t *pos)
>  {
> -	struct mnt_namespace *n = m->private;
> +	struct proc_mounts *p = m->private;
> 
> -	return seq_list_next(v, &n->list, pos);
> +	return seq_list_next(v, &p->ns->list, pos);
>  }
> 
>  static void m_stop(struct seq_file *m, void *v)
> @@ -777,13 +777,52 @@ const struct seq_operations mounts_op = 
>  	.show	= show_vfsmnt
>  };
> 
> +/*
> + * Return true if 'path' is reachable from 'root'
> + */
> +static bool is_path_reachable(const struct path *path, const struct path *root)
> +{
> +	struct dentry *dentry = path->dentry;
> +	struct vfsmount *mnt = path->mnt;
> +	bool res = false;
> +
> +	spin_lock(&dcache_lock);
> +	for (;;) {
> +		if (dentry == root->dentry && mnt == root->mnt) {
> +			res = true;
> +			break;
> +		}
> +		if (dentry == mnt->mnt_root || IS_ROOT(dentry)) {
> +			/* Global root? */
> +			spin_lock(&vfsmount_lock);
> +			if (mnt->mnt_parent == mnt) {
> +				spin_unlock(&vfsmount_lock);
> +				break;
> +			}
> +			dentry = mnt->mnt_mountpoint;
> +			mnt = mnt->mnt_parent;
> +			spin_unlock(&vfsmount_lock);
> +			continue;
> +		}
> +		dentry = dentry->d_parent;
> +	}
> +	spin_unlock(&dcache_lock);
> +
> +	return res;
> +}
> +
>  static int show_mountinfo(struct seq_file *m, void *v)
>  {
> +	struct proc_mounts *p = m->private;
>  	struct vfsmount *mnt = list_entry(v, struct vfsmount, mnt_list);
>  	struct super_block *sb = mnt->mnt_sb;
>  	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
>  	int err = 0;
> 
> +	/* Skip unreachable mounts */
> +	if (!is_path_reachable(&mnt_path, &p->root))
> +		return 0;
> +
>  	seq_printf(m, "%i %i %u:%u ", mnt->mnt_id, mnt->mnt_parent->mnt_id,
>  		   MAJOR(sb->s_dev), MINOR(sb->s_dev));
>  	show_type(m, sb);
> @@ -792,7 +831,7 @@ static int show_mountinfo(struct seq_fil
>  	seq_putc(m, ' ');
>  	seq_dentry(m, mnt->mnt_root, " \t\n\\");
>  	seq_putc(m, ' ');
> -	seq_path(m, &mnt_path, " \t\n\\");
> +	seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
>  	seq_puts(m, mnt->mnt_flags & MNT_READONLY ? " ro" : " rw");
>  	show_mnt_opts(m, mnt);
>  	seq_puts(m, sb->s_flags & MS_RDONLY ? " ro" : " rw");
> Index: linux/fs/proc/base.c
> ===================================================================
> --- linux.orig/fs/proc/base.c	2008-03-11 12:38:48.000000000 +0100
> +++ linux/fs/proc/base.c	2008-03-11 16:22:08.000000000 +0100
> @@ -502,17 +502,14 @@ static const struct inode_operations pro
>  	.setattr	= proc_setattr,
>  };
> 
> -struct proc_mounts {
> -	struct seq_file m;
> -	int event;
> -};
> -
>  static int mounts_open_common(struct inode *inode, struct file *file,
>  			      const struct seq_operations *op)
>  {
>  	struct task_struct *task = get_proc_task(inode);
>  	struct nsproxy *nsp;
>  	struct mnt_namespace *ns = NULL;
> +	struct fs_struct *fs = NULL;
> +	struct path root;
>  	struct proc_mounts *p;
>  	int ret = -EINVAL;
> 
> @@ -525,40 +522,61 @@ static int mounts_open_common(struct ino
>  				get_mnt_ns(ns);
>  		}
>  		rcu_read_unlock();
> -
> +		if (ns)
> +			fs = get_fs_struct(task);
>  		put_task_struct(task);
>  	}
> 
> -	if (ns) {
> -		ret = -ENOMEM;
> -		p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);
> -		if (p) {
> -			file->private_data = &p->m;
> -			ret = seq_open(file, op);
> -			if (!ret) {
> -				p->m.private = ns;
> -				p->event = ns->event;
> -				return 0;
> -			}
> -			kfree(p);
> -		}
> -		put_mnt_ns(ns);
> -	}
> +	if (!ns)
> +		goto err;
> +	if (!fs)
> +		goto err_put_ns;
> +
> +	read_lock(&fs->lock);
> +	root = fs->root;
> +	path_get(&root);
> +	read_unlock(&fs->lock);
> +	put_fs_struct(fs);
> +
> +	ret = -ENOMEM;
> +	p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);
> +	if (!p)
> +		goto err_put_path;
> +
> +	file->private_data = &p->m;
> +	ret = seq_open(file, op);
> +	if (ret)
> +		goto err_free;
> +
> +	p->m.private = p;
> +	p->ns = ns;
> +	p->root = root;
> +	p->event = ns->event;
> +
> +	return 0;
> +
> + err_free:
> +	kfree(p);
> + err_put_path:
> +	path_put(&root);
> + err_put_ns:
> +	put_mnt_ns(ns);
> + err:
>  	return ret;
>  }
> 
>  static int mounts_release(struct inode *inode, struct file *file)
>  {
> -	struct seq_file *m = file->private_data;
> -	struct mnt_namespace *ns = m->private;
> -	put_mnt_ns(ns);
> +	struct proc_mounts *p = file->private_data;
> +	path_put(&p->root);
> +	put_mnt_ns(p->ns);
>  	return seq_release(inode, file);
>  }
> 
>  static unsigned mounts_poll(struct file *file, poll_table *wait)
>  {
>  	struct proc_mounts *p = file->private_data;
> -	struct mnt_namespace *ns = p->m.private;
> +	struct mnt_namespace *ns = p->ns;
>  	unsigned res = 0;
> 
>  	poll_wait(file, &ns->poll, wait);
> Index: linux/fs/seq_file.c
> ===================================================================
> --- linux.orig/fs/seq_file.c	2008-03-11 12:39:15.000000000 +0100
> +++ linux/fs/seq_file.c	2008-03-11 16:22:08.000000000 +0100
> @@ -392,6 +392,29 @@ int seq_path(struct seq_file *m, struct 
>  EXPORT_SYMBOL(seq_path);
> 
>  #ifdef CONFIG_PROC_FS
> +int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
> +		  char *esc)
> +{
> +	if (m->count < m->size) {
> +		char *s = m->buf + m->count;
> +		char *p;
> +
> +		spin_lock(&dcache_lock);
> +		p = __d_path(path, root, s, m->size - m->count);
> +		spin_unlock(&dcache_lock);
> +		if (!IS_ERR(p)) {
> +			s = mangle_path(s, p, esc);
> +			if (s) {
> +				p = m->buf + m->count;
> +				m->count = s - m->buf;
> +				return s - p;
> +			}
> +		}
> +	}
> +	m->count = m->size;
> +	return -1;
> +}
> +
>  /*
>   * returns the path of the 'dentry' from the root of its filesystem.
>   */
> Index: linux/include/linux/dcache.h
> ===================================================================
> --- linux.orig/include/linux/dcache.h	2008-03-11 12:39:15.000000000 +0100
> +++ linux/include/linux/dcache.h	2008-03-11 16:22:08.000000000 +0100
> @@ -302,6 +302,7 @@ extern int d_validate(struct dentry *, s
>   */
>  extern char *dynamic_dname(struct dentry *, char *, int, const char *, ...);
> 
> +extern char *__d_path(struct path *path, struct path *root, char *, int);
>  extern char *d_path(struct path *, char *, int);
> 
>  #ifdef CONFIG_PROC_FS
> Index: linux/include/linux/mnt_namespace.h
> ===================================================================
> --- linux.orig/include/linux/mnt_namespace.h	2008-03-11 12:38:48.000000000 +0100
> +++ linux/include/linux/mnt_namespace.h	2008-03-11 16:22:08.000000000 +0100
> @@ -5,6 +5,7 @@
>  #include <linux/mount.h>
>  #include <linux/sched.h>
>  #include <linux/nsproxy.h>
> +#include <linux/seq_file.h>
> 
>  struct mnt_namespace {
>  	atomic_t		count;
> @@ -14,6 +15,13 @@ struct mnt_namespace {
>  	int event;
>  };
> 
> +struct proc_mounts {
> +	struct seq_file m; /* must be the first element */
> +	struct mnt_namespace *ns;
> +	struct path root;
> +	int event;
> +};
> +
>  extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *,
>  		struct fs_struct *);
>  extern void __put_mnt_ns(struct mnt_namespace *ns);
> Index: linux/include/linux/seq_file.h
> ===================================================================
> --- linux.orig/include/linux/seq_file.h	2008-03-11 12:39:15.000000000 +0100
> +++ linux/include/linux/seq_file.h	2008-03-11 16:22:08.000000000 +0100
> @@ -46,6 +46,8 @@ int seq_printf(struct seq_file *, const 
>  int seq_path(struct seq_file *, struct path *, char *);
> 
>  #ifdef CONFIG_PROC_FS
> +int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
> +		  char *esc);
>  int seq_dentry(struct seq_file *, struct dentry *, char *);
>  #endif /* CONFIG_PROC_FS */

Re: [RFC PATCH] mountinfo: show only reachable mounts

[Miklos Szeredi]

> On Tue, 2008-03-11 at 17:17 +0100, Miklos Szeredi wrote:
> > [One more RFC for mountinfo, and then I'll really send them to
> > Andrew.]
> > 
> > Two changes:
> > 
> > 1. Only show reachable mounts in /proc/<pid>/mountinfo, this makes
> > mountpoints unambiguous for chrooted processes.
> > 
> > 2. Instead of showing mountpoints relative to the current root, always
> > show them relative to the queried task's root.
> > 
> > This means, that a particular mountinfo file will always have the same
> > contents, regardless of which process is reading the file.  Which is a
> > lot more consistent, than the current behavior of /proc/<pid>/mounts.
> 
> 
> What about the propagation aspect?  Won't you have to show only those
> master mount pgid's that have a mounts within the same chrooted tree?

You mean, the dominating mount's pgid?  Yes, it would make sense to
show only the one which is under the same root.  I'm a bit sceptical
about the worth of this dominating mount pgid thing, but Al thinks
it's a good idea, so...

About the immediate master's pgid, I think we decided, that it's not a
problem to show it, even if there's no representative mount of it
under the current namespace (or root).

Miklos