[PATCH 0/3] make struct nameidata private to namei.c

[PATCH 0/3] make struct nameidata private to namei.c

[Christoph Hellwig]

This series stops exposing struct nameidata internals to filesystems.
It is on top of vfs.git#master plus the two fixes for follow_link()
which I sent earlier.

[PATCH 1/3] fs: move path_put on failure out of ->follow_link

[Christoph Hellwig]

[-- Attachment #1: fs-follow_link-dont-put --]
[-- Type: text/plain, Size: 2010 bytes --]

Currently the non-nd_set_link based versions of ->follow_link are expected
to do a path_put(&nd->path).  This calling convention is unexpected,
undocumented and doesn't match what the nd_set_link-based instance do.

Move the path_put out of the only non-nd_set_link based ->follow_link
instance into the caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>

---
 fs/namei.c     |    3 +--
 fs/proc/base.c |   12 ++++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2012-06-18 12:46:47.560143542 +0200
+++ linux-2.6/fs/namei.c	2012-06-18 12:47:02.424143920 +0200
@@ -615,7 +615,7 @@ follow_link(struct path *link, struct na
 	*p = dentry->d_inode->i_op->follow_link(dentry, nd);
 	error = PTR_ERR(*p);
 	if (IS_ERR(*p))
-		goto out_put_link;
+		goto out_put_nd_path;
 
 	error = 0;
 	s = nd_get_link(nd);
@@ -640,7 +640,6 @@ follow_link(struct path *link, struct na
 
 out_put_nd_path:
 	path_put(&nd->path);
-out_put_link:
 	path_put(link);
 	return error;
 }
Index: linux-2.6/fs/proc/base.c
===================================================================
--- linux-2.6.orig/fs/proc/base.c	2012-06-18 12:46:47.560143542 +0200
+++ linux-2.6/fs/proc/base.c	2012-06-18 12:47:02.424143920 +0200
@@ -1427,16 +1427,20 @@ static int proc_exe_link(struct dentry *
 static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
 	struct inode *inode = dentry->d_inode;
+	struct path path;
 	int error = -EACCES;
 
-	/* We don't need a base pointer in the /proc filesystem */
-	path_put(&nd->path);
-
 	/* Are we allowed to snoop on the tasks file descriptors? */
 	if (!proc_fd_access_allowed(inode))
 		goto out;
 
-	error = PROC_I(inode)->op.proc_get_link(dentry, &nd->path);
+	error = PROC_I(inode)->op.proc_get_link(dentry, &path);
+	if (error)
+		goto out;
+
+	path_put(&nd->path);
+	nd->path = path;
+	return NULL;
 out:
 	return ERR_PTR(error);
 }

[PATCH 2/3] fs: add nd_jump_link

[Christoph Hellwig]

[-- Attachment #1: fs-change-proc-symlinks --]
[-- Type: text/plain, Size: 2901 bytes --]

Add a helper that abstracts out the jump to an already parsed struct path
from ->follow_link operation from procfs.  Not only does this clean up
the code by moving the two sides of this game into a single helper, but
it also prepares for making struct nameidata private to namei.c

Signed-off-by: Christoph Hellwig <hch@lst.de>

---
 fs/namei.c            |   27 +++++++++++++++++----------
 fs/proc/base.c        |    3 +--
 include/linux/namei.h |    2 ++
 3 files changed, 20 insertions(+), 12 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2012-06-18 15:04:54.004355716 +0200
+++ linux-2.6/fs/namei.c	2012-06-18 15:08:29.572361238 +0200
@@ -586,6 +586,21 @@ static inline void path_to_nameidata(con
 	nd->path.dentry = path->dentry;
 }
 
+/*
+ * Helper to directly jump to a known parsed path from ->follow_link,
+ * caller must have taken a reference to path beforehand.
+ */
+void nd_jump_link(struct nameidata *nd, struct path *path)
+{
+	path_put(&nd->path);
+
+	nd->path = *path;
+	nd->inode = nd->path.dentry->d_inode;
+	nd->flags |= LOOKUP_JUMPED;
+
+	BUG_ON(nd->inode->i_op->follow_link);
+}
+
 static inline void put_link(struct nameidata *nd, struct path *link, void *cookie)
 {
 	struct inode *inode = link->dentry->d_inode;
@@ -630,17 +645,9 @@ follow_link(struct path *link, struct na
 	s = nd_get_link(nd);
 	if (s) {
 		error = __vfs_follow_link(nd, s);
-	} else if (nd->last_type == LAST_BIND) {
-		nd->flags |= LOOKUP_JUMPED;
-		nd->inode = nd->path.dentry->d_inode;
-		if (nd->inode->i_op->follow_link) {
-			/* stepped on a _really_ weird one */
-			path_put(&nd->path);
-			error = -ELOOP;
-		}
+		if (unlikely(error))
+			put_link(nd, link, *p);
 	}
-	if (unlikely(error))
-		put_link(nd, link, *p);
 
 	return error;
 
Index: linux-2.6/include/linux/namei.h
===================================================================
--- linux-2.6.orig/include/linux/namei.h	2012-06-18 15:04:54.004355716 +0200
+++ linux-2.6/include/linux/namei.h	2012-06-18 15:05:41.292356924 +0200
@@ -80,6 +80,8 @@ extern int follow_up(struct path *);
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
+extern void nd_jump_link(struct nameidata *nd, struct path *path);
+
 static inline void nd_set_link(struct nameidata *nd, char *path)
 {
 	nd->saved_names[nd->depth] = path;
Index: linux-2.6/fs/proc/base.c
===================================================================
--- linux-2.6.orig/fs/proc/base.c	2012-06-18 15:04:54.004355716 +0200
+++ linux-2.6/fs/proc/base.c	2012-06-18 15:05:41.296356925 +0200
@@ -1438,8 +1438,7 @@ static void *proc_pid_follow_link(struct
 	if (error)
 		goto out;
 
-	path_put(&nd->path);
-	nd->path = path;
+	nd_jump_link(nd, &path);
 	return NULL;
 out:
 	return ERR_PTR(error);

[PATCH 3/3] fs: make struct nameidata private to namei.c

[Christoph Hellwig]

[-- Attachment #1: fs-make-nameidata-private --]
[-- Type: text/plain, Size: 4245 bytes --]

By moving nd_set_link and nd_get_link out of line we can hide the internals
of struct nameidata now.

Signed-off-by: Christoph Hellwig <hch@lst.de>

---
 Documentation/filesystems/porting |    6 ++++++
 fs/namei.c                        |   31 +++++++++++++++++++++++++++++++
 include/linux/namei.h             |   38 ++++++--------------------------------
 3 files changed, 43 insertions(+), 32 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2012-06-18 15:15:13.000371564 +0200
+++ linux-2.6/fs/namei.c	2012-06-18 15:20:46.920380115 +0200
@@ -110,6 +110,25 @@
  * any extra contention...
  */
 
+enum { MAX_NESTED_LINKS = 8 };
+
+/*
+ * Type of the last component on LOOKUP_PARENT
+ */
+enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
+
+struct nameidata {
+	struct path	path;
+	struct qstr	last;
+	struct path	root;
+	struct inode	*inode; /* path.dentry.d_inode */
+	unsigned int	flags;
+	unsigned	seq;
+	int		last_type;
+	unsigned	depth;
+	char *saved_names[MAX_NESTED_LINKS + 1];
+};
+
 /* In order to reduce some races, while at the same time doing additional
  * checking and hopefully speeding things up, we copy filenames to the
  * kernel data space before using them..
@@ -586,6 +605,18 @@ static inline void path_to_nameidata(con
 	nd->path.dentry = path->dentry;
 }
 
+void nd_set_link(struct nameidata *nd, char *path)
+{
+	nd->saved_names[nd->depth] = path;
+}
+EXPORT_SYMBOL(nd_set_link);
+
+char *nd_get_link(struct nameidata *nd)
+{
+	return nd->saved_names[nd->depth];
+}
+EXPORT_SYMBOL(nd_get_link);
+
 /*
  * Helper to directly jump to a known parsed path from ->follow_link,
  * caller must have taken a reference to path beforehand.
Index: linux-2.6/include/linux/namei.h
===================================================================
--- linux-2.6.orig/include/linux/namei.h	2012-06-18 15:15:13.000371564 +0200
+++ linux-2.6/include/linux/namei.h	2012-06-18 15:20:46.920380115 +0200
@@ -1,31 +1,13 @@
 #ifndef _LINUX_NAMEI_H
 #define _LINUX_NAMEI_H
 
-#include <linux/dcache.h>
-#include <linux/linkage.h>
-#include <linux/path.h>
+#include <linux/kernel.h>
 
+struct dentry;
+struct nameidata;
+struct path;
 struct vfsmount;
 
-enum { MAX_NESTED_LINKS = 8 };
-
-struct nameidata {
-	struct path	path;
-	struct qstr	last;
-	struct path	root;
-	struct inode	*inode; /* path.dentry.d_inode */
-	unsigned int	flags;
-	unsigned	seq;
-	int		last_type;
-	unsigned	depth;
-	char *saved_names[MAX_NESTED_LINKS + 1];
-};
-
-/*
- * Type of the last component on LOOKUP_PARENT
- */
-enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
-
 /*
  * The bitmask for a lookup event:
  *  - follow links at the end
@@ -80,18 +62,10 @@ extern int follow_up(struct path *);
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
+extern void nd_set_link(struct nameidata *nd, char *path);
+extern char *nd_get_link(struct nameidata *nd);
 extern void nd_jump_link(struct nameidata *nd, struct path *path);
 
-static inline void nd_set_link(struct nameidata *nd, char *path)
-{
-	nd->saved_names[nd->depth] = path;
-}
-
-static inline char *nd_get_link(struct nameidata *nd)
-{
-	return nd->saved_names[nd->depth];
-}
-
 static inline void nd_terminate_link(void *name, size_t len, size_t maxlen)
 {
 	((char *) name)[min(len, maxlen)] = '\0';
Index: linux-2.6/Documentation/filesystems/porting
===================================================================
--- linux-2.6.orig/Documentation/filesystems/porting	2012-06-18 15:15:13.000371564 +0200
+++ linux-2.6/Documentation/filesystems/porting	2012-06-18 15:20:46.920380115 +0200
@@ -442,3 +442,9 @@ d_make_root() drops the reference to ino
 two, it gets "is it an O_EXCL or equivalent?" boolean argument.  Note that
 local filesystems can ignore tha argument - they are guaranteed that the
 object doesn't exist.  It's remote/distributed ones that might care...
+
+--
+[mandatory]
+	struct nameidata is private to fs/namei.c now.  All ->follow_link
+	must now either use nd_set_link to set a link path to iterate on,
+	or nd_jump_link to jump to a pre-parsed struct path directly.