All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link()
@ 2010-07-22 17:58 ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:58 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells

Add a dentry op (d_automount) to handle automounting directories rather than
abusing the follow_link() inode operation.  The operation is keyed off a new
inode flag (S_AUTOMOUNT).

This makes it easier to add an AT_ flag to suppress terminal segment automount
during pathwalk.  It should also remove the need for the kludge code in the
pathwalk algorithm to handle directories with follow_link() semantics.

I've only changed __follow_mount() to handle automount points, but it might be
necessary to change follow_mount() too.  The latter is only used from
follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
a child of it.

Note that autofs4's use of follow_mount() will need examining if this patch is
committed.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 Documentation/filesystems/Locking |    2 +
 Documentation/filesystems/vfs.txt |   13 ++++++
 fs/namei.c                        |   85 +++++++++++++++++++++++++++++--------
 include/linux/dcache.h            |    5 ++
 include/linux/fs.h                |    2 +
 5 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 96d4293..ccbfa98 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -16,6 +16,7 @@ prototypes:
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
+	struct vfsmount *(*d_automount)(struct path *path);
 
 locking rules:
 	none have BKL
@@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
 d_release:	no		no		no		yes
 d_iput:		no		no		no		yes
 d_dname:	no		no		no		no
+d_automount:	no		no		no		yes
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 94677e7..31a9e8f 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -851,6 +851,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -885,6 +886,18 @@ struct dentry_operations {
 	at the end of the buffer, and returns a pointer to the first char.
 	dynamic_dname() helper function is provided to take care of this.
 
+  d_automount: called when an automount dentry is to be traversed (optional).
+	This should create a new VFS mount record, mount it on the directory
+	and return the record to the caller.  The caller is supplied with a
+	path parameter giving the automount directory to describe the automount
+	target and the parent VFS mount record to provide inheritable mount
+	parameters.  NULL should be returned if someone else managed to make
+	the automount first.  If the automount failed, then an error code
+	should be returned.
+
+	This function is only used if S_AUTOMOUNT is set on the inode to which
+	the dentry refers.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
diff --git a/fs/namei.c b/fs/namei.c
index 868d0cb..fcec3c6 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -617,24 +617,71 @@ int follow_up(struct path *path)
 	return 1;
 }
 
+/*
+ * Perform an automount
+ */
+static int follow_automount(struct path *path, int res)
+{
+	struct vfsmount *mnt;
+
+	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
+		return -EREMOTE;
+
+	current->total_link_count++;
+	if (current->total_link_count >= 40)
+		return -ELOOP;
+
+	mnt = path->dentry->d_op->d_automount(path);
+	if (IS_ERR(mnt))
+		return PTR_ERR(mnt);
+	if (!mnt) /* mount collision */
+		return 0;
+
+	if (mnt->mnt_sb == path->mnt->mnt_sb &&
+	    mnt->mnt_root == path->dentry) {
+		mntput(mnt);
+		return -ELOOP;
+	}
+
+	dput(path->dentry);
+	if (res)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	return 0;
+}
+
 /* no need for dcache_lock, as serialization is taken care in
  * namespace.c
  */
-static int __follow_mount(struct path *path)
+static int __follow_mount(struct path *path, unsigned nofollow)
 {
-	int res = 0;
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
+	struct vfsmount *mounted;
+	int ret, res = 0;
+	for (;;) {
+		while (d_mountpoint(path->dentry)) {
+			if (nofollow)
+				return -ELOOP;
+			mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			if (res)
+				mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			res = 1;
+		}
+		if (!d_automount_point(path->dentry))
 			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		if (nofollow)
+			return -ELOOP;
+		ret = follow_automount(path, res);
+		if (ret < 0)
+			return ret;
 		res = 1;
 	}
-	return res;
+	return 0;
 }
 
 static void follow_mount(struct path *path)
@@ -702,6 +749,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent;
 	struct inode *dir;
+	int ret;
+
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
@@ -720,8 +769,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done:
 	path->mnt = mnt;
 	path->dentry = dentry;
-	__follow_mount(path);
-	return 0;
+	ret = __follow_mount(path, 0);
+	if (unlikely(ret < 0))
+		path_put(path);
+	return ret;
 
 need_lookup:
 	parent = nd->path.dentry;
@@ -1721,11 +1772,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	if (__follow_mount(path)) {
-		error = -ELOOP;
-		if (open_flag & O_NOFOLLOW)
-			goto exit_dput;
-	}
+	error = __follow_mount(path, open_flag & O_NOFOLLOW);
+	if (error < 0)
+		goto exit_dput;
 
 	error = -ENOENT;
 	if (!path->dentry->d_inode)
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index eebb617..5380bff 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -139,6 +139,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
 /* the dentry parameter passed to d_hash and d_compare is the parent
@@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
 d_delete:	no		yes		no       no
 d_release:	no		no		no       yes
 d_iput:		no		no		no       yes
+d_automount:	no		no		no	 yes
  */
 
 /* d_flags entries */
@@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
 	return dentry->d_mounted;
 }
 
+#define d_automount_point(dentry) \
+	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
+
 extern struct vfsmount *lookup_mnt(struct path *);
 extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 68ca1b0..a83fc81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -235,6 +235,7 @@ struct inodes_stat_t {
 #define S_NOCMTIME	128	/* Do not update file c/mtime */
 #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	512	/* Inode is fs-internal */
+#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -269,6 +270,7 @@ struct inodes_stat_t {
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
 #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
+#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
 
 /* the read-only stuff doesn't really belong here, but any other place is
    probably as bad and I don't want to create yet another include file. */

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link()
@ 2010-07-22 17:58 ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:58 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells

Add a dentry op (d_automount) to handle automounting directories rather than
abusing the follow_link() inode operation.  The operation is keyed off a new
inode flag (S_AUTOMOUNT).

This makes it easier to add an AT_ flag to suppress terminal segment automount
during pathwalk.  It should also remove the need for the kludge code in the
pathwalk algorithm to handle directories with follow_link() semantics.

I've only changed __follow_mount() to handle automount points, but it might be
necessary to change follow_mount() too.  The latter is only used from
follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
a child of it.

Note that autofs4's use of follow_mount() will need examining if this patch is
committed.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/filesystems/Locking |    2 +
 Documentation/filesystems/vfs.txt |   13 ++++++
 fs/namei.c                        |   85 +++++++++++++++++++++++++++++--------
 include/linux/dcache.h            |    5 ++
 include/linux/fs.h                |    2 +
 5 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 96d4293..ccbfa98 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -16,6 +16,7 @@ prototypes:
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
+	struct vfsmount *(*d_automount)(struct path *path);
 
 locking rules:
 	none have BKL
@@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
 d_release:	no		no		no		yes
 d_iput:		no		no		no		yes
 d_dname:	no		no		no		no
+d_automount:	no		no		no		yes
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 94677e7..31a9e8f 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -851,6 +851,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -885,6 +886,18 @@ struct dentry_operations {
 	at the end of the buffer, and returns a pointer to the first char.
 	dynamic_dname() helper function is provided to take care of this.
 
+  d_automount: called when an automount dentry is to be traversed (optional).
+	This should create a new VFS mount record, mount it on the directory
+	and return the record to the caller.  The caller is supplied with a
+	path parameter giving the automount directory to describe the automount
+	target and the parent VFS mount record to provide inheritable mount
+	parameters.  NULL should be returned if someone else managed to make
+	the automount first.  If the automount failed, then an error code
+	should be returned.
+
+	This function is only used if S_AUTOMOUNT is set on the inode to which
+	the dentry refers.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
diff --git a/fs/namei.c b/fs/namei.c
index 868d0cb..fcec3c6 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -617,24 +617,71 @@ int follow_up(struct path *path)
 	return 1;
 }
 
+/*
+ * Perform an automount
+ */
+static int follow_automount(struct path *path, int res)
+{
+	struct vfsmount *mnt;
+
+	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
+		return -EREMOTE;
+
+	current->total_link_count++;
+	if (current->total_link_count >= 40)
+		return -ELOOP;
+
+	mnt = path->dentry->d_op->d_automount(path);
+	if (IS_ERR(mnt))
+		return PTR_ERR(mnt);
+	if (!mnt) /* mount collision */
+		return 0;
+
+	if (mnt->mnt_sb == path->mnt->mnt_sb &&
+	    mnt->mnt_root == path->dentry) {
+		mntput(mnt);
+		return -ELOOP;
+	}
+
+	dput(path->dentry);
+	if (res)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	return 0;
+}
+
 /* no need for dcache_lock, as serialization is taken care in
  * namespace.c
  */
-static int __follow_mount(struct path *path)
+static int __follow_mount(struct path *path, unsigned nofollow)
 {
-	int res = 0;
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
+	struct vfsmount *mounted;
+	int ret, res = 0;
+	for (;;) {
+		while (d_mountpoint(path->dentry)) {
+			if (nofollow)
+				return -ELOOP;
+			mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			if (res)
+				mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			res = 1;
+		}
+		if (!d_automount_point(path->dentry))
 			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		if (nofollow)
+			return -ELOOP;
+		ret = follow_automount(path, res);
+		if (ret < 0)
+			return ret;
 		res = 1;
 	}
-	return res;
+	return 0;
 }
 
 static void follow_mount(struct path *path)
@@ -702,6 +749,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent;
 	struct inode *dir;
+	int ret;
+
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
@@ -720,8 +769,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done:
 	path->mnt = mnt;
 	path->dentry = dentry;
-	__follow_mount(path);
-	return 0;
+	ret = __follow_mount(path, 0);
+	if (unlikely(ret < 0))
+		path_put(path);
+	return ret;
 
 need_lookup:
 	parent = nd->path.dentry;
@@ -1721,11 +1772,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	if (__follow_mount(path)) {
-		error = -ELOOP;
-		if (open_flag & O_NOFOLLOW)
-			goto exit_dput;
-	}
+	error = __follow_mount(path, open_flag & O_NOFOLLOW);
+	if (error < 0)
+		goto exit_dput;
 
 	error = -ENOENT;
 	if (!path->dentry->d_inode)
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index eebb617..5380bff 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -139,6 +139,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
 /* the dentry parameter passed to d_hash and d_compare is the parent
@@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
 d_delete:	no		yes		no       no
 d_release:	no		no		no       yes
 d_iput:		no		no		no       yes
+d_automount:	no		no		no	 yes
  */
 
 /* d_flags entries */
@@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
 	return dentry->d_mounted;
 }
 
+#define d_automount_point(dentry) \
+	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
+
 extern struct vfsmount *lookup_mnt(struct path *);
 extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 68ca1b0..a83fc81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -235,6 +235,7 @@ struct inodes_stat_t {
 #define S_NOCMTIME	128	/* Do not update file c/mtime */
 #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	512	/* Inode is fs-internal */
+#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -269,6 +270,7 @@ struct inodes_stat_t {
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
 #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
+#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
 
 /* the read-only stuff doesn't really belong here, but any other place is
    probably as bad and I don't want to create yet another include file. */


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 2/6] AFS: Use d_automount() rather than abusing follow_link()
  2010-07-22 17:58 ` David Howells
  (?)
@ 2010-07-22 17:58 ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:58 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells

Make AFS use the new d_automount() dentry operation rather than abusing
follow_link() on directories.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/dir.c      |    1 +
 fs/afs/internal.h |    1 +
 fs/afs/mntpt.c    |   46 +++++++++++++++-------------------------------
 3 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index b42d5cc..5366f9b 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -65,6 +65,7 @@ static const struct dentry_operations afs_fs_dentry_operations = {
 	.d_revalidate	= afs_d_revalidate,
 	.d_delete	= afs_d_delete,
 	.d_release	= afs_d_release,
+	.d_automount	= afs_d_automount,
 };
 
 #define AFS_DIR_HASHTBL_SIZE	128
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 5f679b7..2c700dc 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -583,6 +583,7 @@ extern int afs_abort_to_error(u32);
 extern const struct inode_operations afs_mntpt_inode_operations;
 extern const struct file_operations afs_mntpt_file_operations;
 
+extern struct vfsmount *afs_d_automount(struct path *);
 extern int afs_mntpt_check_symlink(struct afs_vnode *, struct key *);
 extern void afs_mntpt_kill_timer(void);
 
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index a9e2303..ea9cfee 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -24,7 +24,6 @@ static struct dentry *afs_mntpt_lookup(struct inode *dir,
 				       struct dentry *dentry,
 				       struct nameidata *nd);
 static int afs_mntpt_open(struct inode *inode, struct file *file);
-static void *afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd);
 static void afs_mntpt_expiry_timed_out(struct work_struct *work);
 
 const struct file_operations afs_mntpt_file_operations = {
@@ -33,7 +32,6 @@ const struct file_operations afs_mntpt_file_operations = {
 
 const struct inode_operations afs_mntpt_inode_operations = {
 	.lookup		= afs_mntpt_lookup,
-	.follow_link	= afs_mntpt_follow_link,
 	.readlink	= page_readlink,
 	.getattr	= afs_getattr,
 };
@@ -82,6 +80,7 @@ int afs_mntpt_check_symlink(struct afs_vnode *vnode, struct key *key)
 		_debug("symlink is a mountpoint");
 		spin_lock(&vnode->lock);
 		set_bit(AFS_VNODE_MOUNTPOINT, &vnode->flags);
+		vnode->vfs_inode.i_flags |= S_AUTOMOUNT;
 		spin_unlock(&vnode->lock);
 	}
 
@@ -205,52 +204,37 @@ error_no_devname:
 }
 
 /*
- * follow a link from a mountpoint directory, thus causing it to be mounted
+ * handle an automount point
  */
-static void *afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd)
+struct vfsmount *afs_d_automount(struct path *path)
 {
 	struct vfsmount *newmnt;
 	int err;
 
-	_enter("%p{%s},{%s:%p{%s},}",
-	       dentry,
-	       dentry->d_name.name,
-	       nd->path.mnt->mnt_devname,
-	       dentry,
-	       nd->path.dentry->d_name.name);
-
-	dput(nd->path.dentry);
-	nd->path.dentry = dget(dentry);
+	_enter("{%s,%s}", path->mnt->mnt_devname, path->dentry->d_name.name);
 
-	newmnt = afs_mntpt_do_automount(nd->path.dentry);
-	if (IS_ERR(newmnt)) {
-		path_put(&nd->path);
-		return (void *)newmnt;
-	}
+	newmnt = afs_mntpt_do_automount(path->dentry);
+	if (IS_ERR(newmnt))
+		return newmnt;
 
 	mntget(newmnt);
-	err = do_add_mount(newmnt, &nd->path, MNT_SHRINKABLE, &afs_vfsmounts);
+	err = do_add_mount(newmnt, path, MNT_SHRINKABLE, &afs_vfsmounts);
 	switch (err) {
 	case 0:
-		path_put(&nd->path);
-		nd->path.mnt = newmnt;
-		nd->path.dentry = dget(newmnt->mnt_root);
 		schedule_delayed_work(&afs_mntpt_expiry_timer,
 				      afs_mntpt_expiry_timeout * HZ);
-		break;
+		_leave(" = %p {%s}", newmnt, newmnt->mnt_devname);
+		return newmnt;
 	case -EBUSY:
 		/* someone else made a mount here whilst we were busy */
-		while (d_mountpoint(nd->path.dentry) &&
-		       follow_down(&nd->path))
-			;
-		err = 0;
+		mntput(newmnt);
+		_leave(" = NULL [EBUSY]");
+		return NULL;
 	default:
 		mntput(newmnt);
-		break;
+		_leave(" = %d", err);
+		return ERR_PTR(err);
 	}
-
-	_leave(" = %d", err);
-	return ERR_PTR(err);
 }
 
 /*

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 3/6] NFS: Use d_automount() rather than abusing follow_link()
  2010-07-22 17:58 ` David Howells
@ 2010-07-22 17:58     ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:58 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	Trond Myklebust

Make NFS use the new d_automount() dentry operation rather than abusing
follow_link() on directories.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Acked-by: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
---

 fs/nfs/dir.c           |    4 ++
 fs/nfs/inode.c         |    4 +-
 fs/nfs/internal.h      |    1 +
 fs/nfs/namespace.c     |   87 ++++++++++++++++++++++--------------------------
 include/linux/nfs_fs.h |    1 -
 5 files changed, 46 insertions(+), 51 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e60416d..9ce04af 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -727,7 +727,7 @@ int nfs_lookup_verify_inode(struct inode *inode, struct nameidata *nd)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
 
-	if (test_bit(NFS_INO_MOUNTPOINT, &NFS_I(inode)->flags))
+	if (IS_AUTOMOUNT(inode))
 		return 0;
 	if (nd != NULL) {
 		/* VFS wants an on-the-wire revalidation */
@@ -927,6 +927,7 @@ const struct dentry_operations nfs_dentry_operations = {
 	.d_revalidate	= nfs_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
+	.d_automount	= nfs_d_automount,
 };
 
 static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd)
@@ -1002,6 +1003,7 @@ const struct dentry_operations nfs4_dentry_operations = {
 	.d_revalidate	= nfs_open_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
+	.d_automount	= nfs_d_automount,
 };
 
 /*
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 099b351..848abe8 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -295,7 +295,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 				else
 					inode->i_op = &nfs_mountpoint_inode_operations;
 				inode->i_fop = NULL;
-				set_bit(NFS_INO_MOUNTPOINT, &nfsi->flags);
+				inode->i_flags |= S_AUTOMOUNT;
 			}
 		} else if (S_ISLNK(inode->i_mode))
 			inode->i_op = &nfs_symlink_inode_operations;
@@ -1138,7 +1138,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	/* Update the fsid? */
 	if (S_ISDIR(inode->i_mode) && (fattr->valid & NFS_ATTR_FATTR_FSID) &&
 			!nfs_fsid_equal(&server->fsid, &fattr->fsid) &&
-			!test_bit(NFS_INO_MOUNTPOINT, &nfsi->flags))
+			!IS_AUTOMOUNT(inode))
 		server->fsid = fattr->fsid;
 
 	/*
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index e70f44b..f597aac 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -239,6 +239,7 @@ extern char *nfs_path(const char *base,
 		      const struct dentry *droot,
 		      const struct dentry *dentry,
 		      char *buffer, ssize_t buflen);
+extern struct vfsmount *nfs_d_automount(struct path *path);
 
 /* getroot.c */
 extern struct dentry *nfs_get_root(struct super_block *, struct nfs_fh *);
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index db6aa36..bf80079 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -88,9 +88,8 @@ Elong:
 }
 
 /*
- * nfs_follow_mountpoint - handle crossing a mountpoint on the server
- * @dentry - dentry of mountpoint
- * @nd - nameidata info
+ * nfs_d_automount - Handle crossing a mountpoint on the server
+ * @path - The mountpoint
  *
  * When we encounter a mountpoint on the server, we want to set up
  * a mountpoint on the client too, to prevent inode numbers from
@@ -100,87 +99,81 @@ Elong:
  * situation, and that different filesystems may want to use
  * different security flavours.
  */
-static void * nfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
+struct vfsmount *nfs_d_automount(struct path *path)
 {
 	struct vfsmount *mnt;
-	struct nfs_server *server = NFS_SERVER(dentry->d_inode);
+	struct nfs_server *server = NFS_SERVER(path->dentry->d_inode);
 	struct dentry *parent;
 	struct nfs_fh *fh = NULL;
 	struct nfs_fattr *fattr = NULL;
 	int err;
 
-	dprintk("--> nfs_follow_mountpoint()\n");
+	dprintk("--> nfs_d_automount()\n");
 
-	err = -ESTALE;
-	if (IS_ROOT(dentry))
-		goto out_err;
+	mnt = ERR_PTR(-ESTALE);
+	if (IS_ROOT(path->dentry))
+		goto out_nofree;
 
-	err = -ENOMEM;
+	mnt = ERR_PTR(-ENOMEM);
 	fh = nfs_alloc_fhandle();
 	fattr = nfs_alloc_fattr();
 	if (fh == NULL || fattr == NULL)
-		goto out_err;
+		goto out;
 
 	dprintk("%s: enter\n", __func__);
-	dput(nd->path.dentry);
-	nd->path.dentry = dget(dentry);
 
-	/* Look it up again */
-	parent = dget_parent(nd->path.dentry);
+	/* Look it up again to get its attributes */
+	parent = dget_parent(path->dentry);
 	err = server->nfs_client->rpc_ops->lookup(parent->d_inode,
-						  &nd->path.dentry->d_name,
+						  &path->dentry->d_name,
 						  fh, fattr);
 	dput(parent);
-	if (err != 0)
-		goto out_err;
+	if (err != 0) {
+		mnt = ERR_PTR(err);
+		goto out;
+	}
 
 	if (fattr->valid & NFS_ATTR_FATTR_V4_REFERRAL)
-		mnt = nfs_do_refmount(nd->path.mnt, nd->path.dentry);
+		mnt = nfs_do_refmount(path->mnt, path->dentry);
 	else
-		mnt = nfs_do_submount(nd->path.mnt, nd->path.dentry, fh,
-				      fattr);
-	err = PTR_ERR(mnt);
+		mnt = nfs_do_submount(path->mnt, path->dentry, fh, fattr);
 	if (IS_ERR(mnt))
-		goto out_err;
+		goto out;
 
 	mntget(mnt);
-	err = do_add_mount(mnt, &nd->path, nd->path.mnt->mnt_flags|MNT_SHRINKABLE,
+	err = do_add_mount(mnt, path, path->mnt->mnt_flags | MNT_SHRINKABLE,
 			   &nfs_automount_list);
-	if (err < 0) {
+	switch (err) {
+	case 0:
+		dprintk("%s: done, success\n", __func__);
+		schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
+		break;
+	case -EBUSY:
+		/* someone else made a mount here whilst we were busy */
 		mntput(mnt);
-		if (err == -EBUSY)
-			goto out_follow;
-		goto out_err;
+		dprintk("%s: done, collision\n", __func__);
+		mnt = NULL;
+		break;
+	default:
+		mntput(mnt);
+		dprintk("%s: done, error %d\n", __func__, err);
+		mnt = ERR_PTR(err);
+		break;
 	}
-	path_put(&nd->path);
-	nd->path.mnt = mnt;
-	nd->path.dentry = dget(mnt->mnt_root);
-	schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
+
 out:
 	nfs_free_fattr(fattr);
 	nfs_free_fhandle(fh);
-	dprintk("%s: done, returned %d\n", __func__, err);
-
-	dprintk("<-- nfs_follow_mountpoint() = %d\n", err);
-	return ERR_PTR(err);
-out_err:
-	path_put(&nd->path);
-	goto out;
-out_follow:
-	while (d_mountpoint(nd->path.dentry) &&
-	       follow_down(&nd->path))
-		;
-	err = 0;
-	goto out;
+out_nofree:
+	dprintk("<-- nfs_follow_mountpoint() = %p\n", mnt);
+	return mnt;
 }
 
 const struct inode_operations nfs_mountpoint_inode_operations = {
-	.follow_link	= nfs_follow_mountpoint,
 	.getattr	= nfs_getattr,
 };
 
 const struct inode_operations nfs_referral_inode_operations = {
-	.follow_link	= nfs_follow_mountpoint,
 };
 
 static void nfs_expire_automounts(struct work_struct *work)
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 77c2ae5..4511220 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -205,7 +205,6 @@ struct nfs_inode {
 #define NFS_INO_ADVISE_RDPLUS	(0)		/* advise readdirplus */
 #define NFS_INO_STALE		(1)		/* possible stale inode */
 #define NFS_INO_ACL_LRU_SET	(2)		/* Inode is on the LRU list */
-#define NFS_INO_MOUNTPOINT	(3)		/* inode is remote mountpoint */
 #define NFS_INO_FLUSHING	(4)		/* inode is flushing out data */
 #define NFS_INO_FSCACHE		(5)		/* inode can be cached by FS-Cache */
 #define NFS_INO_FSCACHE_LOCK	(6)		/* FS-Cache cookie management lock */

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 3/6] NFS: Use d_automount() rather than abusing follow_link()
@ 2010-07-22 17:58     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:58 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells, Trond Myklebust

Make NFS use the new d_automount() dentry operation rather than abusing
follow_link() on directories.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c           |    4 ++
 fs/nfs/inode.c         |    4 +-
 fs/nfs/internal.h      |    1 +
 fs/nfs/namespace.c     |   87 ++++++++++++++++++++++--------------------------
 include/linux/nfs_fs.h |    1 -
 5 files changed, 46 insertions(+), 51 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e60416d..9ce04af 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -727,7 +727,7 @@ int nfs_lookup_verify_inode(struct inode *inode, struct nameidata *nd)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
 
-	if (test_bit(NFS_INO_MOUNTPOINT, &NFS_I(inode)->flags))
+	if (IS_AUTOMOUNT(inode))
 		return 0;
 	if (nd != NULL) {
 		/* VFS wants an on-the-wire revalidation */
@@ -927,6 +927,7 @@ const struct dentry_operations nfs_dentry_operations = {
 	.d_revalidate	= nfs_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
+	.d_automount	= nfs_d_automount,
 };
 
 static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd)
@@ -1002,6 +1003,7 @@ const struct dentry_operations nfs4_dentry_operations = {
 	.d_revalidate	= nfs_open_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
+	.d_automount	= nfs_d_automount,
 };
 
 /*
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 099b351..848abe8 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -295,7 +295,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 				else
 					inode->i_op = &nfs_mountpoint_inode_operations;
 				inode->i_fop = NULL;
-				set_bit(NFS_INO_MOUNTPOINT, &nfsi->flags);
+				inode->i_flags |= S_AUTOMOUNT;
 			}
 		} else if (S_ISLNK(inode->i_mode))
 			inode->i_op = &nfs_symlink_inode_operations;
@@ -1138,7 +1138,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	/* Update the fsid? */
 	if (S_ISDIR(inode->i_mode) && (fattr->valid & NFS_ATTR_FATTR_FSID) &&
 			!nfs_fsid_equal(&server->fsid, &fattr->fsid) &&
-			!test_bit(NFS_INO_MOUNTPOINT, &nfsi->flags))
+			!IS_AUTOMOUNT(inode))
 		server->fsid = fattr->fsid;
 
 	/*
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index e70f44b..f597aac 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -239,6 +239,7 @@ extern char *nfs_path(const char *base,
 		      const struct dentry *droot,
 		      const struct dentry *dentry,
 		      char *buffer, ssize_t buflen);
+extern struct vfsmount *nfs_d_automount(struct path *path);
 
 /* getroot.c */
 extern struct dentry *nfs_get_root(struct super_block *, struct nfs_fh *);
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index db6aa36..bf80079 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -88,9 +88,8 @@ Elong:
 }
 
 /*
- * nfs_follow_mountpoint - handle crossing a mountpoint on the server
- * @dentry - dentry of mountpoint
- * @nd - nameidata info
+ * nfs_d_automount - Handle crossing a mountpoint on the server
+ * @path - The mountpoint
  *
  * When we encounter a mountpoint on the server, we want to set up
  * a mountpoint on the client too, to prevent inode numbers from
@@ -100,87 +99,81 @@ Elong:
  * situation, and that different filesystems may want to use
  * different security flavours.
  */
-static void * nfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
+struct vfsmount *nfs_d_automount(struct path *path)
 {
 	struct vfsmount *mnt;
-	struct nfs_server *server = NFS_SERVER(dentry->d_inode);
+	struct nfs_server *server = NFS_SERVER(path->dentry->d_inode);
 	struct dentry *parent;
 	struct nfs_fh *fh = NULL;
 	struct nfs_fattr *fattr = NULL;
 	int err;
 
-	dprintk("--> nfs_follow_mountpoint()\n");
+	dprintk("--> nfs_d_automount()\n");
 
-	err = -ESTALE;
-	if (IS_ROOT(dentry))
-		goto out_err;
+	mnt = ERR_PTR(-ESTALE);
+	if (IS_ROOT(path->dentry))
+		goto out_nofree;
 
-	err = -ENOMEM;
+	mnt = ERR_PTR(-ENOMEM);
 	fh = nfs_alloc_fhandle();
 	fattr = nfs_alloc_fattr();
 	if (fh == NULL || fattr == NULL)
-		goto out_err;
+		goto out;
 
 	dprintk("%s: enter\n", __func__);
-	dput(nd->path.dentry);
-	nd->path.dentry = dget(dentry);
 
-	/* Look it up again */
-	parent = dget_parent(nd->path.dentry);
+	/* Look it up again to get its attributes */
+	parent = dget_parent(path->dentry);
 	err = server->nfs_client->rpc_ops->lookup(parent->d_inode,
-						  &nd->path.dentry->d_name,
+						  &path->dentry->d_name,
 						  fh, fattr);
 	dput(parent);
-	if (err != 0)
-		goto out_err;
+	if (err != 0) {
+		mnt = ERR_PTR(err);
+		goto out;
+	}
 
 	if (fattr->valid & NFS_ATTR_FATTR_V4_REFERRAL)
-		mnt = nfs_do_refmount(nd->path.mnt, nd->path.dentry);
+		mnt = nfs_do_refmount(path->mnt, path->dentry);
 	else
-		mnt = nfs_do_submount(nd->path.mnt, nd->path.dentry, fh,
-				      fattr);
-	err = PTR_ERR(mnt);
+		mnt = nfs_do_submount(path->mnt, path->dentry, fh, fattr);
 	if (IS_ERR(mnt))
-		goto out_err;
+		goto out;
 
 	mntget(mnt);
-	err = do_add_mount(mnt, &nd->path, nd->path.mnt->mnt_flags|MNT_SHRINKABLE,
+	err = do_add_mount(mnt, path, path->mnt->mnt_flags | MNT_SHRINKABLE,
 			   &nfs_automount_list);
-	if (err < 0) {
+	switch (err) {
+	case 0:
+		dprintk("%s: done, success\n", __func__);
+		schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
+		break;
+	case -EBUSY:
+		/* someone else made a mount here whilst we were busy */
 		mntput(mnt);
-		if (err == -EBUSY)
-			goto out_follow;
-		goto out_err;
+		dprintk("%s: done, collision\n", __func__);
+		mnt = NULL;
+		break;
+	default:
+		mntput(mnt);
+		dprintk("%s: done, error %d\n", __func__, err);
+		mnt = ERR_PTR(err);
+		break;
 	}
-	path_put(&nd->path);
-	nd->path.mnt = mnt;
-	nd->path.dentry = dget(mnt->mnt_root);
-	schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
+
 out:
 	nfs_free_fattr(fattr);
 	nfs_free_fhandle(fh);
-	dprintk("%s: done, returned %d\n", __func__, err);
-
-	dprintk("<-- nfs_follow_mountpoint() = %d\n", err);
-	return ERR_PTR(err);
-out_err:
-	path_put(&nd->path);
-	goto out;
-out_follow:
-	while (d_mountpoint(nd->path.dentry) &&
-	       follow_down(&nd->path))
-		;
-	err = 0;
-	goto out;
+out_nofree:
+	dprintk("<-- nfs_follow_mountpoint() = %p\n", mnt);
+	return mnt;
 }
 
 const struct inode_operations nfs_mountpoint_inode_operations = {
-	.follow_link	= nfs_follow_mountpoint,
 	.getattr	= nfs_getattr,
 };
 
 const struct inode_operations nfs_referral_inode_operations = {
-	.follow_link	= nfs_follow_mountpoint,
 };
 
 static void nfs_expire_automounts(struct work_struct *work)
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 77c2ae5..4511220 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -205,7 +205,6 @@ struct nfs_inode {
 #define NFS_INO_ADVISE_RDPLUS	(0)		/* advise readdirplus */
 #define NFS_INO_STALE		(1)		/* possible stale inode */
 #define NFS_INO_ACL_LRU_SET	(2)		/* Inode is on the LRU list */
-#define NFS_INO_MOUNTPOINT	(3)		/* inode is remote mountpoint */
 #define NFS_INO_FLUSHING	(4)		/* inode is flushing out data */
 #define NFS_INO_FSCACHE		(5)		/* inode can be cached by FS-Cache */
 #define NFS_INO_FSCACHE_LOCK	(6)		/* FS-Cache cookie management lock */


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 4/6] CIFS: Use d_automount() rather than abusing follow_link()
  2010-07-22 17:58 ` David Howells
@ 2010-07-22 17:59     ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:59 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Steve French

Make CIFS use the new d_automount() dentry operation rather than abusing
follow_link() on directories.

[NOTE: THIS IS UNTESTED!]

[Question:  Why does cifs_dfs_do_refmount() when the caller has already done
	    that and could pass the result through?]

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---

 fs/cifs/cifs_dfs_ref.c |  145 +++++++++++++++++++++++-------------------------
 fs/cifs/cifsfs.h       |    6 ++
 fs/cifs/dir.c          |    2 +
 fs/cifs/inode.c        |    8 ++-
 4 files changed, 83 insertions(+), 78 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 37543e8..bc094f4 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -230,8 +230,8 @@ compose_mount_options_err:
 }
 
 
-static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
-		struct dentry *dentry, const struct dfs_info3_param *ref)
+static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt,
+					     const struct dfs_info3_param *ref)
 {
 	struct cifs_sb_info *cifs_sb;
 	struct vfsmount *mnt;
@@ -239,12 +239,12 @@ static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
 	char *devname = NULL;
 	char *fullpath;
 
-	cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
+	cifs_sb = CIFS_SB(mntpt->d_inode->i_sb);
 	/*
 	 * this function gives us a path with a double backslash prefix. We
 	 * require a single backslash for DFS.
 	 */
-	fullpath = build_path_from_dentry(dentry);
+	fullpath = build_path_from_dentry(mntpt);
 	if (!fullpath)
 		return ERR_PTR(-ENOMEM);
 
@@ -262,35 +262,6 @@ static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
 
 }
 
-static int add_mount_helper(struct vfsmount *newmnt, struct nameidata *nd,
-				struct list_head *mntlist)
-{
-	/* stolen from afs code */
-	int err;
-
-	mntget(newmnt);
-	err = do_add_mount(newmnt, &nd->path, nd->path.mnt->mnt_flags | MNT_SHRINKABLE, mntlist);
-	switch (err) {
-	case 0:
-		path_put(&nd->path);
-		nd->path.mnt = newmnt;
-		nd->path.dentry = dget(newmnt->mnt_root);
-		schedule_delayed_work(&cifs_dfs_automount_task,
-				      cifs_dfs_mountpoint_expiry_timeout);
-		break;
-	case -EBUSY:
-		/* someone else made a mount here whilst we were busy */
-		while (d_mountpoint(nd->path.dentry) &&
-		       follow_down(&nd->path))
-			;
-		err = 0;
-	default:
-		mntput(newmnt);
-		break;
-	}
-	return err;
-}
-
 static void dump_referral(const struct dfs_info3_param *ref)
 {
 	cFYI(1, "DFS: ref path: %s", ref->path_name);
@@ -300,34 +271,30 @@ static void dump_referral(const struct dfs_info3_param *ref)
 				ref->path_consumed);
 }
 
-
-static void*
-cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
+/*
+ * Create a vfsmount that we can automount
+ */
+static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
 {
 	struct dfs_info3_param *referrals = NULL;
 	unsigned int num_referrals = 0;
 	struct cifs_sb_info *cifs_sb;
 	struct cifsSesInfo *ses;
-	char *full_path = NULL;
+	char *full_path;
 	int xid, i;
-	int rc = 0;
-	struct vfsmount *mnt = ERR_PTR(-ENOENT);
+	int rc;
+	struct vfsmount *mnt;
 
 	cFYI(1, "in %s", __func__);
-	BUG_ON(IS_ROOT(dentry));
+	BUG_ON(IS_ROOT(mntpt));
 
 	xid = GetXid();
 
-	dput(nd->path.dentry);
-	nd->path.dentry = dget(dentry);
-
-	cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
+	cifs_sb = CIFS_SB(mntpt->d_inode->i_sb);
+	mnt = ERR_PTR(-EINVAL);
 	ses = cifs_sb->tcon->ses;
-
-	if (!ses) {
-		rc = -EINVAL;
-		goto out_err;
-	}
+	if (!ses)
+		goto free_xid;
 
 	/*
 	 * The MSDFS spec states that paths in DFS referral requests and
@@ -335,56 +302,84 @@ cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
 	 * the double backslashes usually used in the UNC. This function
 	 * gives us the latter, so we must adjust the result.
 	 */
-	full_path = build_path_from_dentry(dentry);
-	if (full_path == NULL) {
-		rc = -ENOMEM;
-		goto out_err;
-	}
+	mnt = ERR_PTR(-ENOMEM);
+	full_path = build_path_from_dentry(mntpt);
+	if (full_path == NULL)
+		goto free_xid;
 
 	rc = get_dfs_path(xid, ses , full_path + 1, cifs_sb->local_nls,
 		&num_referrals, &referrals,
 		cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 
+	mnt = ERR_PTR(-ENOENT);
 	for (i = 0; i < num_referrals; i++) {
 		int len;
-		dump_referral(referrals+i);
+		dump_referral(referrals + i);
 		/* connect to a node */
 		len = strlen(referrals[i].node_name);
 		if (len < 2) {
 			cERROR(1, "%s: Net Address path too short: %s",
 					__func__, referrals[i].node_name);
-			rc = -EINVAL;
-			goto out_err;
+			mnt = ERR_PTR(-EINVAL);
+			break;
 		}
-		mnt = cifs_dfs_do_refmount(nd->path.mnt,
-				nd->path.dentry, referrals + i);
+		mnt = cifs_dfs_do_refmount(mntpt, referrals + i);
 		cFYI(1, "%s: cifs_dfs_do_refmount:%s , mnt:%p", __func__,
 					referrals[i].node_name, mnt);
-
-		/* complete mount procedure if we accured submount */
 		if (!IS_ERR(mnt))
-			break;
+			goto success;
 	}
 
-	/* we need it cause for() above could exit without valid submount */
-	rc = PTR_ERR(mnt);
-	if (IS_ERR(mnt))
-		goto out_err;
-
-	rc = add_mount_helper(mnt, nd, &cifs_dfs_automount_list);
+	/* no valid submounts were found; return error from get_dfs_path() by
+	 * preference */
+	if (rc != 0)
+		mnt = ERR_PTR(rc);
 
-out:
-	FreeXid(xid);
+success:
 	free_dfs_info_array(referrals, num_referrals);
 	kfree(full_path);
+free_xid:
+	FreeXid(xid);
 	cFYI(1, "leaving %s" , __func__);
 	return ERR_PTR(rc);
-out_err:
-	path_put(&nd->path);
-	goto out;
+}
+
+/*
+ * Attempt to automount the referral
+ */
+struct vfsmount *cifs_dfs_d_automount(struct path *path)
+{
+	struct vfsmount *newmnt;
+	int err;
+
+	cFYI(1, "in %s", __func__);
+
+	newmnt = cifs_dfs_do_automount(path->dentry);
+	if (IS_ERR(newmnt)) {
+		cFYI(1, "leaving %s [automount failed]" , __func__);
+		return newmnt;
+	}
+
+	mntget(newmnt);
+	err = do_add_mount(newmnt, path, MNT_SHRINKABLE,
+			   &cifs_dfs_automount_list);
+	switch (err) {
+	case 0:
+		schedule_delayed_work(&cifs_dfs_automount_task,
+				      cifs_dfs_mountpoint_expiry_timeout);
+		cFYI(1, "leaving %s [ok]" , __func__);
+		return newmnt;
+	case -EBUSY:
+		/* someone else made a mount here whilst we were busy */
+		mntput(newmnt);
+		cFYI(1, "leaving %s [EBUSY]" , __func__);
+		return NULL;
+	default:
+		mntput(newmnt);
+		cFYI(1, "leaving %s [error %d]" , __func__, err);
+		return ERR_PTR(err);
+	}
 }
 
 const struct inode_operations cifs_dfs_referral_inode_operations = {
-	.follow_link = cifs_dfs_follow_mountpoint,
 };
-
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index a7eb65c..d74bda0 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -95,6 +95,12 @@ extern int cifs_readdir(struct file *file, void *direntry, filldir_t filldir);
 extern const struct dentry_operations cifs_dentry_ops;
 extern const struct dentry_operations cifs_ci_dentry_ops;
 
+#ifdef CONFIG_CIFS_DFS_UPCALL
+extern struct vfsmount *cifs_dfs_d_automount(struct path *path);
+#else
+#define cifs_dfs_d_automount NULL
+#endif
+
 /* Functions related to symlinks */
 extern void *cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
 extern void cifs_put_link(struct dentry *direntry,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index e7ae78b..b147905 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -804,6 +804,7 @@ cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd)
 
 const struct dentry_operations cifs_dentry_ops = {
 	.d_revalidate = cifs_d_revalidate,
+	.d_automount = cifs_dfs_d_automount,
 /* d_delete:       cifs_d_delete,      */ /* not needed except for debugging */
 };
 
@@ -844,4 +845,5 @@ const struct dentry_operations cifs_ci_dentry_ops = {
 	.d_revalidate = cifs_d_revalidate,
 	.d_hash = cifs_ci_hash,
 	.d_compare = cifs_ci_compare,
+	.d_automount = cifs_dfs_d_automount,
 };
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 6f0683c..011100f 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -31,7 +31,7 @@
 #include "cifs_fs_sb.h"
 
 
-static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
+static void cifs_set_ops(struct inode *inode)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
@@ -59,7 +59,7 @@ static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
 		break;
 	case S_IFDIR:
 #ifdef CONFIG_CIFS_DFS_UPCALL
-		if (is_dfs_referral) {
+		if (IS_AUTOMOUNT(inode)) {
 			inode->i_op = &cifs_dfs_referral_inode_operations;
 		} else {
 #else /* NO DFS support, treat as a directory */
@@ -166,7 +166,9 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
 	}
 	spin_unlock(&inode->i_lock);
 
-	cifs_set_ops(inode, fattr->cf_flags & CIFS_FATTR_DFS_REFERRAL);
+	if (fattr->cf_flags & CIFS_FATTR_DFS_REFERRAL)
+		inode->i_flags |= S_AUTOMOUNT;
+	cifs_set_ops(inode);
 }
 
 void

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 4/6] CIFS: Use d_automount() rather than abusing follow_link()
@ 2010-07-22 17:59     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:59 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells, Steve French

Make CIFS use the new d_automount() dentry operation rather than abusing
follow_link() on directories.

[NOTE: THIS IS UNTESTED!]

[Question:  Why does cifs_dfs_do_refmount() when the caller has already done
	    that and could pass the result through?]

Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Steve French <sfrench@samba.org>
---

 fs/cifs/cifs_dfs_ref.c |  145 +++++++++++++++++++++++-------------------------
 fs/cifs/cifsfs.h       |    6 ++
 fs/cifs/dir.c          |    2 +
 fs/cifs/inode.c        |    8 ++-
 4 files changed, 83 insertions(+), 78 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 37543e8..bc094f4 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -230,8 +230,8 @@ compose_mount_options_err:
 }
 
 
-static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
-		struct dentry *dentry, const struct dfs_info3_param *ref)
+static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt,
+					     const struct dfs_info3_param *ref)
 {
 	struct cifs_sb_info *cifs_sb;
 	struct vfsmount *mnt;
@@ -239,12 +239,12 @@ static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
 	char *devname = NULL;
 	char *fullpath;
 
-	cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
+	cifs_sb = CIFS_SB(mntpt->d_inode->i_sb);
 	/*
 	 * this function gives us a path with a double backslash prefix. We
 	 * require a single backslash for DFS.
 	 */
-	fullpath = build_path_from_dentry(dentry);
+	fullpath = build_path_from_dentry(mntpt);
 	if (!fullpath)
 		return ERR_PTR(-ENOMEM);
 
@@ -262,35 +262,6 @@ static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
 
 }
 
-static int add_mount_helper(struct vfsmount *newmnt, struct nameidata *nd,
-				struct list_head *mntlist)
-{
-	/* stolen from afs code */
-	int err;
-
-	mntget(newmnt);
-	err = do_add_mount(newmnt, &nd->path, nd->path.mnt->mnt_flags | MNT_SHRINKABLE, mntlist);
-	switch (err) {
-	case 0:
-		path_put(&nd->path);
-		nd->path.mnt = newmnt;
-		nd->path.dentry = dget(newmnt->mnt_root);
-		schedule_delayed_work(&cifs_dfs_automount_task,
-				      cifs_dfs_mountpoint_expiry_timeout);
-		break;
-	case -EBUSY:
-		/* someone else made a mount here whilst we were busy */
-		while (d_mountpoint(nd->path.dentry) &&
-		       follow_down(&nd->path))
-			;
-		err = 0;
-	default:
-		mntput(newmnt);
-		break;
-	}
-	return err;
-}
-
 static void dump_referral(const struct dfs_info3_param *ref)
 {
 	cFYI(1, "DFS: ref path: %s", ref->path_name);
@@ -300,34 +271,30 @@ static void dump_referral(const struct dfs_info3_param *ref)
 				ref->path_consumed);
 }
 
-
-static void*
-cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
+/*
+ * Create a vfsmount that we can automount
+ */
+static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
 {
 	struct dfs_info3_param *referrals = NULL;
 	unsigned int num_referrals = 0;
 	struct cifs_sb_info *cifs_sb;
 	struct cifsSesInfo *ses;
-	char *full_path = NULL;
+	char *full_path;
 	int xid, i;
-	int rc = 0;
-	struct vfsmount *mnt = ERR_PTR(-ENOENT);
+	int rc;
+	struct vfsmount *mnt;
 
 	cFYI(1, "in %s", __func__);
-	BUG_ON(IS_ROOT(dentry));
+	BUG_ON(IS_ROOT(mntpt));
 
 	xid = GetXid();
 
-	dput(nd->path.dentry);
-	nd->path.dentry = dget(dentry);
-
-	cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
+	cifs_sb = CIFS_SB(mntpt->d_inode->i_sb);
+	mnt = ERR_PTR(-EINVAL);
 	ses = cifs_sb->tcon->ses;
-
-	if (!ses) {
-		rc = -EINVAL;
-		goto out_err;
-	}
+	if (!ses)
+		goto free_xid;
 
 	/*
 	 * The MSDFS spec states that paths in DFS referral requests and
@@ -335,56 +302,84 @@ cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
 	 * the double backslashes usually used in the UNC. This function
 	 * gives us the latter, so we must adjust the result.
 	 */
-	full_path = build_path_from_dentry(dentry);
-	if (full_path == NULL) {
-		rc = -ENOMEM;
-		goto out_err;
-	}
+	mnt = ERR_PTR(-ENOMEM);
+	full_path = build_path_from_dentry(mntpt);
+	if (full_path == NULL)
+		goto free_xid;
 
 	rc = get_dfs_path(xid, ses , full_path + 1, cifs_sb->local_nls,
 		&num_referrals, &referrals,
 		cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 
+	mnt = ERR_PTR(-ENOENT);
 	for (i = 0; i < num_referrals; i++) {
 		int len;
-		dump_referral(referrals+i);
+		dump_referral(referrals + i);
 		/* connect to a node */
 		len = strlen(referrals[i].node_name);
 		if (len < 2) {
 			cERROR(1, "%s: Net Address path too short: %s",
 					__func__, referrals[i].node_name);
-			rc = -EINVAL;
-			goto out_err;
+			mnt = ERR_PTR(-EINVAL);
+			break;
 		}
-		mnt = cifs_dfs_do_refmount(nd->path.mnt,
-				nd->path.dentry, referrals + i);
+		mnt = cifs_dfs_do_refmount(mntpt, referrals + i);
 		cFYI(1, "%s: cifs_dfs_do_refmount:%s , mnt:%p", __func__,
 					referrals[i].node_name, mnt);
-
-		/* complete mount procedure if we accured submount */
 		if (!IS_ERR(mnt))
-			break;
+			goto success;
 	}
 
-	/* we need it cause for() above could exit without valid submount */
-	rc = PTR_ERR(mnt);
-	if (IS_ERR(mnt))
-		goto out_err;
-
-	rc = add_mount_helper(mnt, nd, &cifs_dfs_automount_list);
+	/* no valid submounts were found; return error from get_dfs_path() by
+	 * preference */
+	if (rc != 0)
+		mnt = ERR_PTR(rc);
 
-out:
-	FreeXid(xid);
+success:
 	free_dfs_info_array(referrals, num_referrals);
 	kfree(full_path);
+free_xid:
+	FreeXid(xid);
 	cFYI(1, "leaving %s" , __func__);
 	return ERR_PTR(rc);
-out_err:
-	path_put(&nd->path);
-	goto out;
+}
+
+/*
+ * Attempt to automount the referral
+ */
+struct vfsmount *cifs_dfs_d_automount(struct path *path)
+{
+	struct vfsmount *newmnt;
+	int err;
+
+	cFYI(1, "in %s", __func__);
+
+	newmnt = cifs_dfs_do_automount(path->dentry);
+	if (IS_ERR(newmnt)) {
+		cFYI(1, "leaving %s [automount failed]" , __func__);
+		return newmnt;
+	}
+
+	mntget(newmnt);
+	err = do_add_mount(newmnt, path, MNT_SHRINKABLE,
+			   &cifs_dfs_automount_list);
+	switch (err) {
+	case 0:
+		schedule_delayed_work(&cifs_dfs_automount_task,
+				      cifs_dfs_mountpoint_expiry_timeout);
+		cFYI(1, "leaving %s [ok]" , __func__);
+		return newmnt;
+	case -EBUSY:
+		/* someone else made a mount here whilst we were busy */
+		mntput(newmnt);
+		cFYI(1, "leaving %s [EBUSY]" , __func__);
+		return NULL;
+	default:
+		mntput(newmnt);
+		cFYI(1, "leaving %s [error %d]" , __func__, err);
+		return ERR_PTR(err);
+	}
 }
 
 const struct inode_operations cifs_dfs_referral_inode_operations = {
-	.follow_link = cifs_dfs_follow_mountpoint,
 };
-
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index a7eb65c..d74bda0 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -95,6 +95,12 @@ extern int cifs_readdir(struct file *file, void *direntry, filldir_t filldir);
 extern const struct dentry_operations cifs_dentry_ops;
 extern const struct dentry_operations cifs_ci_dentry_ops;
 
+#ifdef CONFIG_CIFS_DFS_UPCALL
+extern struct vfsmount *cifs_dfs_d_automount(struct path *path);
+#else
+#define cifs_dfs_d_automount NULL
+#endif
+
 /* Functions related to symlinks */
 extern void *cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
 extern void cifs_put_link(struct dentry *direntry,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index e7ae78b..b147905 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -804,6 +804,7 @@ cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd)
 
 const struct dentry_operations cifs_dentry_ops = {
 	.d_revalidate = cifs_d_revalidate,
+	.d_automount = cifs_dfs_d_automount,
 /* d_delete:       cifs_d_delete,      */ /* not needed except for debugging */
 };
 
@@ -844,4 +845,5 @@ const struct dentry_operations cifs_ci_dentry_ops = {
 	.d_revalidate = cifs_d_revalidate,
 	.d_hash = cifs_ci_hash,
 	.d_compare = cifs_ci_compare,
+	.d_automount = cifs_dfs_d_automount,
 };
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 6f0683c..011100f 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -31,7 +31,7 @@
 #include "cifs_fs_sb.h"
 
 
-static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
+static void cifs_set_ops(struct inode *inode)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
@@ -59,7 +59,7 @@ static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
 		break;
 	case S_IFDIR:
 #ifdef CONFIG_CIFS_DFS_UPCALL
-		if (is_dfs_referral) {
+		if (IS_AUTOMOUNT(inode)) {
 			inode->i_op = &cifs_dfs_referral_inode_operations;
 		} else {
 #else /* NO DFS support, treat as a directory */
@@ -166,7 +166,9 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
 	}
 	spin_unlock(&inode->i_lock);
 
-	cifs_set_ops(inode, fattr->cf_flags & CIFS_FATTR_DFS_REFERRAL);
+	if (fattr->cf_flags & CIFS_FATTR_DFS_REFERRAL)
+		inode->i_flags |= S_AUTOMOUNT;
+	cifs_set_ops(inode);
 }
 
 void


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 5/6] Remove the automount through follow_link() kludge code from pathwalk
  2010-07-22 17:58 ` David Howells
  (?)
  (?)
@ 2010-07-22 17:59 ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:59 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells

Remove the automount through follow_link() kludge code from pathwalk in favour
of using d_automount().

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c |   17 +++--------------
 1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index fcec3c6..86068a2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -845,17 +845,6 @@ fail:
 }
 
 /*
- * This is a temporary kludge to deal with "automount" symlinks; proper
- * solution is to trigger them on follow_mount(), so that do_lookup()
- * would DTRT.  To be killed before 2.6.34-final.
- */
-static inline int follow_on_final(struct inode *inode, unsigned lookup_flags)
-{
-	return inode && unlikely(inode->i_op->follow_link) &&
-		((lookup_flags & LOOKUP_FOLLOW) || S_ISDIR(inode->i_mode));
-}
-
-/*
  * Name resolution.
  * This is the basic name resolution function, turning a pathname into
  * the final dentry. We expect 'base' to be positive and a directory.
@@ -975,7 +964,8 @@ last_component:
 		if (err)
 			break;
 		inode = next.dentry->d_inode;
-		if (follow_on_final(inode, lookup_flags)) {
+		if (inode && unlikely(inode->i_op->follow_link) &&
+		    (lookup_flags & LOOKUP_FOLLOW)) {
 			err = do_follow_link(&next, nd);
 			if (err)
 				goto return_err;
@@ -1888,8 +1878,7 @@ reval:
 		struct inode *inode = path.dentry->d_inode;
 		void *cookie;
 		error = -ELOOP;
-		/* S_ISDIR part is a temporary automount kludge */
-		if (!(nd.flags & LOOKUP_FOLLOW) && !S_ISDIR(inode->i_mode))
+		if (!(nd.flags & LOOKUP_FOLLOW))
 			goto exit_dput;
 		if (count++ == 32)
 			goto exit_dput;


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
  2010-07-22 17:58 ` David Howells
                   ` (2 preceding siblings ...)
  (?)
@ 2010-07-22 17:59 ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 17:59 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel,
	David Howells

Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of directories
with follow_link semantics.  This can be used by fstatat()/xstat() users to
permit the gathering of attributes on an automount point and also prevent
mass-automounting of a directory of automount points by ls.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c            |   15 ++++++++++-----
 fs/stat.c             |    4 +++-
 include/linux/fcntl.h |    1 +
 include/linux/namei.h |    2 ++
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 86068a2..056427e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -654,7 +654,8 @@ static int follow_automount(struct path *path, int res)
 /* no need for dcache_lock, as serialization is taken care in
  * namespace.c
  */
-static int __follow_mount(struct path *path, unsigned nofollow)
+static int __follow_mount(struct path *path, unsigned nofollow,
+			  struct nameidata *nd)
 {
 	struct vfsmount *mounted;
 	int ret, res = 0;
@@ -674,8 +675,12 @@ static int __follow_mount(struct path *path, unsigned nofollow)
 		}
 		if (!d_automount_point(path->dentry))
 			break;
-		if (nofollow)
-			return -ELOOP;
+		if (!(nd->flags & LOOKUP_CONTINUE)) {
+			if (nofollow)
+				return -ELOOP;
+			if (nd->flags & LOOKUP_NO_AUTOMOUNT)
+				break;
+		}
 		ret = follow_automount(path, res);
 		if (ret < 0)
 			return ret;
@@ -769,7 +774,7 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done:
 	path->mnt = mnt;
 	path->dentry = dentry;
-	ret = __follow_mount(path, 0);
+	ret = __follow_mount(path, 0, nd);
 	if (unlikely(ret < 0))
 		path_put(path);
 	return ret;
@@ -1762,7 +1767,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	error = __follow_mount(path, open_flag & O_NOFOLLOW);
+	error = __follow_mount(path, open_flag & O_NOFOLLOW, nd);
 	if (error < 0)
 		goto exit_dput;
 
diff --git a/fs/stat.c b/fs/stat.c
index c4ecd52..e4662de 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -74,11 +74,13 @@ int vfs_fstatat(int dfd, char __user *filename, struct kstat *stat, int flag)
 	int error = -EINVAL;
 	int lookup_flags = 0;
 
-	if ((flag & ~AT_SYMLINK_NOFOLLOW) != 0)
+	if ((flag & ~AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT) != 0)
 		goto out;
 
 	if (!(flag & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
+	if (flag & AT_NO_AUTOMOUNT)
+		lookup_flags |= LOOKUP_NO_AUTOMOUNT;
 
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (error)
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index afc00af..a562fa5 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -45,6 +45,7 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 05b441d..1e1febf 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -43,12 +43,14 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path components" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - suppress terminal automount
  */
 #define LOOKUP_FOLLOW		 1
 #define LOOKUP_DIRECTORY	 2
 #define LOOKUP_CONTINUE		 4
 #define LOOKUP_PARENT		16
 #define LOOKUP_REVAL		64
+#define LOOKUP_NO_AUTOMOUNT	128
 /*
  * Intent data
  */


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
  2010-07-22 17:58 ` David Howells
@ 2010-07-22 18:01     ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 18:01 UTC (permalink / raw)
  Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
	viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> with follow_link semantics.  This can be used by fstatat()/xstat() users to

xstat() shouldn't be mentioned here.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
@ 2010-07-22 18:01     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-22 18:01 UTC (permalink / raw)
  Cc: dhowells, viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel

David Howells <dhowells@redhat.com> wrote:

> with follow_link semantics.  This can be used by fstatat()/xstat() users to

xstat() shouldn't be mentioned here.

David

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-22 17:58 ` David Howells
@ 2010-07-23 15:09     ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-23 15:09 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA, raven-PKsaG3nR2I+sTnJN9+BGXg,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


Here's an updated patch that:

 (1) Fixes a bug in error handling (needs to use path_put_conditional not
     path_put).

 (2) Absorbs autofs4's decisions about whether to automount or not.  This
     means that colour-ls won't cause automounts unless -L is supplied or it
     doesn't get a DT_DIR flag from getdents().  It also means that autofs4
     can dispense with this logic should it choose to use d_automount().

 (3) Moves all the automounter logic out of __follow_mount() into
     follow_automount().

 (4) Stops pathwalk at the automount point and returns that point in the fs
     tree if it decides not to automount rather than reporting ELOOP (see its
     use of EXDEV for this).

David

---
From: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH] Add a dentry op to handle automounting rather than abusing follow_link()

Add a dentry op (d_automount) to handle automounting directories rather than
abusing the follow_link() inode operation.  The operation is keyed off a new
inode flag (S_AUTOMOUNT).

This makes it easier to add an AT_ flag to suppress terminal segment automount
during pathwalk.  It should also remove the need for the kludge code in the
pathwalk algorithm to handle directories with follow_link() semantics.

I've only changed __follow_mount() to handle automount points, but it might be
necessary to change follow_mount() too.  The latter is only used from
follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
a child of it.

I've also extracted the mount/don't-mount logic from autofs4 and included it
here.  It makes the mount go ahead anyway if someone calls open() or creat(),
tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
or sticks a '/' on the end of the pathname.  If they do a stat(), however,
they'll only trigger the automount if they didn't also say O_NOFOLLOW.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 Documentation/filesystems/Locking |    2 +
 Documentation/filesystems/vfs.txt |   13 +++++
 fs/namei.c                        |   96 ++++++++++++++++++++++++++++++-------
 include/linux/dcache.h            |    5 ++
 include/linux/fs.h                |    2 +
 5 files changed, 100 insertions(+), 18 deletions(-)


diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 96d4293..ccbfa98 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -16,6 +16,7 @@ prototypes:
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
+	struct vfsmount *(*d_automount)(struct path *path);
 
 locking rules:
 	none have BKL
@@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
 d_release:	no		no		no		yes
 d_iput:		no		no		no		yes
 d_dname:	no		no		no		no
+d_automount:	no		no		no		yes
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 94677e7..31a9e8f 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -851,6 +851,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -885,6 +886,18 @@ struct dentry_operations {
 	at the end of the buffer, and returns a pointer to the first char.
 	dynamic_dname() helper function is provided to take care of this.
 
+  d_automount: called when an automount dentry is to be traversed (optional).
+	This should create a new VFS mount record, mount it on the directory
+	and return the record to the caller.  The caller is supplied with a
+	path parameter giving the automount directory to describe the automount
+	target and the parent VFS mount record to provide inheritable mount
+	parameters.  NULL should be returned if someone else managed to make
+	the automount first.  If the automount failed, then an error code
+	should be returned.
+
+	This function is only used if S_AUTOMOUNT is set on the inode to which
+	the dentry refers.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
diff --git a/fs/namei.c b/fs/namei.c
index 868d0cb..6509ca5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -617,24 +617,82 @@ int follow_up(struct path *path)
 	return 1;
 }
 
+/*
+ * Perform an automount
+ * - return -EXDEV to tell __follow_mount() to stop and return the path we were
+ *   called with.
+ */
+static int follow_automount(struct path *path, unsigned flags, int res)
+{
+	struct vfsmount *mnt;
+
+	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
+		return -EREMOTE;
+
+	/* We want to mount if someone is trying to open/create a file of any
+	 * type under the mountpoint, wants to traverse through the mountpoint
+	 * or wants to open the mounted directory.
+	 *
+	 * We don't want to mount if someone's just doing a stat and they've
+	 * set AT_SYMLINK_NOFOLLOW - unless they're stat'ing a directory and
+	 * appended a '/' to the name.
+	 */
+	if (!(flags & LOOKUP_FOLLOW) &&
+	    !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY |
+		       LOOKUP_OPEN | LOOKUP_CREATE)))
+		return -EXDEV;
+
+	current->total_link_count++;
+	if (current->total_link_count >= 40)
+		return -ELOOP;
+
+	mnt = path->dentry->d_op->d_automount(path);
+	if (IS_ERR(mnt))
+		return PTR_ERR(mnt);
+	if (!mnt) /* mount collision */
+		return 0;
+
+	if (mnt->mnt_sb == path->mnt->mnt_sb &&
+	    mnt->mnt_root == path->dentry) {
+		mntput(mnt);
+		return -ELOOP;
+	}
+
+	dput(path->dentry);
+	if (res)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	return 0;
+}
+
 /* no need for dcache_lock, as serialization is taken care in
  * namespace.c
  */
-static int __follow_mount(struct path *path)
+static int __follow_mount(struct path *path, unsigned flags)
 {
-	int res = 0;
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
+	struct vfsmount *mounted;
+	int ret, res = 0;
+	for (;;) {
+		while (d_mountpoint(path->dentry)) {
+			mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			if (res)
+				mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			res = 1;
+		}
+		if (!d_automount_point(path->dentry))
 			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		ret = follow_automount(path, flags, res);
+		if (ret < 0)
+			return ret == -EXDEV ? 0 : ret;
 		res = 1;
 	}
-	return res;
+	return 0;
 }
 
 static void follow_mount(struct path *path)
@@ -702,6 +760,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent;
 	struct inode *dir;
+	int ret;
+
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
@@ -720,8 +780,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done:
 	path->mnt = mnt;
 	path->dentry = dentry;
-	__follow_mount(path);
-	return 0;
+	ret = __follow_mount(path, nd->flags);
+	if (unlikely(ret < 0))
+		path_put_conditional(path, nd);
+	return ret;
 
 need_lookup:
 	parent = nd->path.dentry;
@@ -1721,11 +1783,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	if (__follow_mount(path)) {
-		error = -ELOOP;
-		if (open_flag & O_NOFOLLOW)
-			goto exit_dput;
-	}
+	error = __follow_mount(path, nd->flags);
+	if (error < 0)
+		goto exit_dput;
 
 	error = -ENOENT;
 	if (!path->dentry->d_inode)
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index eebb617..5380bff 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -139,6 +139,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
 /* the dentry parameter passed to d_hash and d_compare is the parent
@@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
 d_delete:	no		yes		no       no
 d_release:	no		no		no       yes
 d_iput:		no		no		no       yes
+d_automount:	no		no		no	 yes
  */
 
 /* d_flags entries */
@@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
 	return dentry->d_mounted;
 }
 
+#define d_automount_point(dentry) \
+	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
+
 extern struct vfsmount *lookup_mnt(struct path *);
 extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 68ca1b0..a83fc81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -235,6 +235,7 @@ struct inodes_stat_t {
 #define S_NOCMTIME	128	/* Do not update file c/mtime */
 #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	512	/* Inode is fs-internal */
+#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -269,6 +270,7 @@ struct inodes_stat_t {
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
 #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
+#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
 
 /* the read-only stuff doesn't really belong here, but any other place is
    probably as bad and I don't want to create yet another include file. */
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
@ 2010-07-23 15:09     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-23 15:09 UTC (permalink / raw)
  To: viro
  Cc: dhowells, raven, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel


Here's an updated patch that:

 (1) Fixes a bug in error handling (needs to use path_put_conditional not
     path_put).

 (2) Absorbs autofs4's decisions about whether to automount or not.  This
     means that colour-ls won't cause automounts unless -L is supplied or it
     doesn't get a DT_DIR flag from getdents().  It also means that autofs4
     can dispense with this logic should it choose to use d_automount().

 (3) Moves all the automounter logic out of __follow_mount() into
     follow_automount().

 (4) Stops pathwalk at the automount point and returns that point in the fs
     tree if it decides not to automount rather than reporting ELOOP (see its
     use of EXDEV for this).

David

---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] Add a dentry op to handle automounting rather than abusing follow_link()

Add a dentry op (d_automount) to handle automounting directories rather than
abusing the follow_link() inode operation.  The operation is keyed off a new
inode flag (S_AUTOMOUNT).

This makes it easier to add an AT_ flag to suppress terminal segment automount
during pathwalk.  It should also remove the need for the kludge code in the
pathwalk algorithm to handle directories with follow_link() semantics.

I've only changed __follow_mount() to handle automount points, but it might be
necessary to change follow_mount() too.  The latter is only used from
follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
a child of it.

I've also extracted the mount/don't-mount logic from autofs4 and included it
here.  It makes the mount go ahead anyway if someone calls open() or creat(),
tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
or sticks a '/' on the end of the pathname.  If they do a stat(), however,
they'll only trigger the automount if they didn't also say O_NOFOLLOW.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/filesystems/Locking |    2 +
 Documentation/filesystems/vfs.txt |   13 +++++
 fs/namei.c                        |   96 ++++++++++++++++++++++++++++++-------
 include/linux/dcache.h            |    5 ++
 include/linux/fs.h                |    2 +
 5 files changed, 100 insertions(+), 18 deletions(-)


diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 96d4293..ccbfa98 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -16,6 +16,7 @@ prototypes:
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
+	struct vfsmount *(*d_automount)(struct path *path);
 
 locking rules:
 	none have BKL
@@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
 d_release:	no		no		no		yes
 d_iput:		no		no		no		yes
 d_dname:	no		no		no		no
+d_automount:	no		no		no		yes
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 94677e7..31a9e8f 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -851,6 +851,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -885,6 +886,18 @@ struct dentry_operations {
 	at the end of the buffer, and returns a pointer to the first char.
 	dynamic_dname() helper function is provided to take care of this.
 
+  d_automount: called when an automount dentry is to be traversed (optional).
+	This should create a new VFS mount record, mount it on the directory
+	and return the record to the caller.  The caller is supplied with a
+	path parameter giving the automount directory to describe the automount
+	target and the parent VFS mount record to provide inheritable mount
+	parameters.  NULL should be returned if someone else managed to make
+	the automount first.  If the automount failed, then an error code
+	should be returned.
+
+	This function is only used if S_AUTOMOUNT is set on the inode to which
+	the dentry refers.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
diff --git a/fs/namei.c b/fs/namei.c
index 868d0cb..6509ca5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -617,24 +617,82 @@ int follow_up(struct path *path)
 	return 1;
 }
 
+/*
+ * Perform an automount
+ * - return -EXDEV to tell __follow_mount() to stop and return the path we were
+ *   called with.
+ */
+static int follow_automount(struct path *path, unsigned flags, int res)
+{
+	struct vfsmount *mnt;
+
+	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
+		return -EREMOTE;
+
+	/* We want to mount if someone is trying to open/create a file of any
+	 * type under the mountpoint, wants to traverse through the mountpoint
+	 * or wants to open the mounted directory.
+	 *
+	 * We don't want to mount if someone's just doing a stat and they've
+	 * set AT_SYMLINK_NOFOLLOW - unless they're stat'ing a directory and
+	 * appended a '/' to the name.
+	 */
+	if (!(flags & LOOKUP_FOLLOW) &&
+	    !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY |
+		       LOOKUP_OPEN | LOOKUP_CREATE)))
+		return -EXDEV;
+
+	current->total_link_count++;
+	if (current->total_link_count >= 40)
+		return -ELOOP;
+
+	mnt = path->dentry->d_op->d_automount(path);
+	if (IS_ERR(mnt))
+		return PTR_ERR(mnt);
+	if (!mnt) /* mount collision */
+		return 0;
+
+	if (mnt->mnt_sb == path->mnt->mnt_sb &&
+	    mnt->mnt_root == path->dentry) {
+		mntput(mnt);
+		return -ELOOP;
+	}
+
+	dput(path->dentry);
+	if (res)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	return 0;
+}
+
 /* no need for dcache_lock, as serialization is taken care in
  * namespace.c
  */
-static int __follow_mount(struct path *path)
+static int __follow_mount(struct path *path, unsigned flags)
 {
-	int res = 0;
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
+	struct vfsmount *mounted;
+	int ret, res = 0;
+	for (;;) {
+		while (d_mountpoint(path->dentry)) {
+			mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			if (res)
+				mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			res = 1;
+		}
+		if (!d_automount_point(path->dentry))
 			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		ret = follow_automount(path, flags, res);
+		if (ret < 0)
+			return ret == -EXDEV ? 0 : ret;
 		res = 1;
 	}
-	return res;
+	return 0;
 }
 
 static void follow_mount(struct path *path)
@@ -702,6 +760,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent;
 	struct inode *dir;
+	int ret;
+
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
@@ -720,8 +780,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done:
 	path->mnt = mnt;
 	path->dentry = dentry;
-	__follow_mount(path);
-	return 0;
+	ret = __follow_mount(path, nd->flags);
+	if (unlikely(ret < 0))
+		path_put_conditional(path, nd);
+	return ret;
 
 need_lookup:
 	parent = nd->path.dentry;
@@ -1721,11 +1783,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	if (__follow_mount(path)) {
-		error = -ELOOP;
-		if (open_flag & O_NOFOLLOW)
-			goto exit_dput;
-	}
+	error = __follow_mount(path, nd->flags);
+	if (error < 0)
+		goto exit_dput;
 
 	error = -ENOENT;
 	if (!path->dentry->d_inode)
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index eebb617..5380bff 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -139,6 +139,7 @@ struct dentry_operations {
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
+	struct vfsmount *(*d_automount)(struct path *);
 };
 
 /* the dentry parameter passed to d_hash and d_compare is the parent
@@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
 d_delete:	no		yes		no       no
 d_release:	no		no		no       yes
 d_iput:		no		no		no       yes
+d_automount:	no		no		no	 yes
  */
 
 /* d_flags entries */
@@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
 	return dentry->d_mounted;
 }
 
+#define d_automount_point(dentry) \
+	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
+
 extern struct vfsmount *lookup_mnt(struct path *);
 extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 68ca1b0..a83fc81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -235,6 +235,7 @@ struct inodes_stat_t {
 #define S_NOCMTIME	128	/* Do not update file c/mtime */
 #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	512	/* Inode is fs-internal */
+#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -269,6 +270,7 @@ struct inodes_stat_t {
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
 #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
+#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
 
 /* the read-only stuff doesn't really belong here, but any other place is
    probably as bad and I don't want to create yet another include file. */

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount [ver #2]
  2010-07-22 17:58 ` David Howells
  (?)
@ 2010-07-23 15:11     ` David Howells
  -1 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-23 15:11 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA, raven-PKsaG3nR2I+sTnJN9+BGXg,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


This is an update required because the first patch in the series was also
altered in the same area.

David
---
From: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount

Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of directories
with follow_link semantics.  This can be used by fstatat() users to permit the
gathering of attributes on an automount point and also prevent
mass-automounting of a directory of automount points by ls.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 fs/namei.c            |    6 ++++++
 fs/stat.c             |    4 +++-
 include/linux/fcntl.h |    1 +
 include/linux/namei.h |    2 ++
 4 files changed, 12 insertions(+), 1 deletions(-)


diff --git a/fs/namei.c b/fs/namei.c
index f2910b7..c154112 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -629,6 +629,12 @@ static int follow_automount(struct path *path, unsigned flags, int res)
 	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
 		return -EREMOTE;
 
+	/* We don't want to mount if someone supplied AT_NO_AUTOMOUNT
+	 * and this is the terminal part of the path.
+	 */
+	if ((flags & LOOKUP_NO_AUTOMOUNT) && !(flags & LOOKUP_CONTINUE))
+		return -EXDEV; /* we actually want to stop here */
+
 	/* We want to mount if someone is trying to open/create a file of any
 	 * type under the mountpoint, wants to traverse through the mountpoint
 	 * or wants to open the mounted directory.
diff --git a/fs/stat.c b/fs/stat.c
index c4ecd52..bae6fa2 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -74,11 +74,13 @@ int vfs_fstatat(int dfd, char __user *filename, struct kstat *stat, int flag)
 	int error = -EINVAL;
 	int lookup_flags = 0;
 
-	if ((flag & ~AT_SYMLINK_NOFOLLOW) != 0)
+	if ((flag & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)) != 0)
 		goto out;
 
 	if (!(flag & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
+	if (flag & AT_NO_AUTOMOUNT)
+		lookup_flags |= LOOKUP_NO_AUTOMOUNT;
 
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (error)
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index afc00af..a562fa5 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -45,6 +45,7 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 05b441d..1e1febf 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -43,12 +43,14 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path components" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - suppress terminal automount
  */
 #define LOOKUP_FOLLOW		 1
 #define LOOKUP_DIRECTORY	 2
 #define LOOKUP_CONTINUE		 4
 #define LOOKUP_PARENT		16
 #define LOOKUP_REVAL		64
+#define LOOKUP_NO_AUTOMOUNT	128
 /*
  * Intent data
  */

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount [ver #2]
@ 2010-07-23 15:11     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-23 15:11 UTC (permalink / raw)
  To: viro
  Cc: dhowells, raven, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel


This is an update required because the first patch in the series was also
altered in the same area.

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount

Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of directories
with follow_link semantics.  This can be used by fstatat() users to permit the
gathering of attributes on an automount point and also prevent
mass-automounting of a directory of automount points by ls.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c            |    6 ++++++
 fs/stat.c             |    4 +++-
 include/linux/fcntl.h |    1 +
 include/linux/namei.h |    2 ++
 4 files changed, 12 insertions(+), 1 deletions(-)


diff --git a/fs/namei.c b/fs/namei.c
index f2910b7..c154112 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -629,6 +629,12 @@ static int follow_automount(struct path *path, unsigned flags, int res)
 	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
 		return -EREMOTE;
 
+	/* We don't want to mount if someone supplied AT_NO_AUTOMOUNT
+	 * and this is the terminal part of the path.
+	 */
+	if ((flags & LOOKUP_NO_AUTOMOUNT) && !(flags & LOOKUP_CONTINUE))
+		return -EXDEV; /* we actually want to stop here */
+
 	/* We want to mount if someone is trying to open/create a file of any
 	 * type under the mountpoint, wants to traverse through the mountpoint
 	 * or wants to open the mounted directory.
diff --git a/fs/stat.c b/fs/stat.c
index c4ecd52..bae6fa2 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -74,11 +74,13 @@ int vfs_fstatat(int dfd, char __user *filename, struct kstat *stat, int flag)
 	int error = -EINVAL;
 	int lookup_flags = 0;
 
-	if ((flag & ~AT_SYMLINK_NOFOLLOW) != 0)
+	if ((flag & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)) != 0)
 		goto out;
 
 	if (!(flag & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
+	if (flag & AT_NO_AUTOMOUNT)
+		lookup_flags |= LOOKUP_NO_AUTOMOUNT;
 
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (error)
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index afc00af..a562fa5 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -45,6 +45,7 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 05b441d..1e1febf 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -43,12 +43,14 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path components" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - suppress terminal automount
  */
 #define LOOKUP_FOLLOW		 1
 #define LOOKUP_DIRECTORY	 2
 #define LOOKUP_CONTINUE		 4
 #define LOOKUP_PARENT		16
 #define LOOKUP_REVAL		64
+#define LOOKUP_NO_AUTOMOUNT	128
 /*
  * Intent data
  */

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount [ver #2]
@ 2010-07-23 15:11     ` David Howells
  0 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2010-07-23 15:11 UTC (permalink / raw)
  To: viro
  Cc: dhowells, raven, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel


This is an update required because the first patch in the series was also
altered in the same area.

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount

Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of directories
with follow_link semantics.  This can be used by fstatat() users to permit the
gathering of attributes on an automount point and also prevent
mass-automounting of a directory of automount points by ls.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c            |    6 ++++++
 fs/stat.c             |    4 +++-
 include/linux/fcntl.h |    1 +
 include/linux/namei.h |    2 ++
 4 files changed, 12 insertions(+), 1 deletions(-)


diff --git a/fs/namei.c b/fs/namei.c
index f2910b7..c154112 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -629,6 +629,12 @@ static int follow_automount(struct path *path, unsigned flags, int res)
 	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
 		return -EREMOTE;
 
+	/* We don't want to mount if someone supplied AT_NO_AUTOMOUNT
+	 * and this is the terminal part of the path.
+	 */
+	if ((flags & LOOKUP_NO_AUTOMOUNT) && !(flags & LOOKUP_CONTINUE))
+		return -EXDEV; /* we actually want to stop here */
+
 	/* We want to mount if someone is trying to open/create a file of any
 	 * type under the mountpoint, wants to traverse through the mountpoint
 	 * or wants to open the mounted directory.
diff --git a/fs/stat.c b/fs/stat.c
index c4ecd52..bae6fa2 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -74,11 +74,13 @@ int vfs_fstatat(int dfd, char __user *filename, struct kstat *stat, int flag)
 	int error = -EINVAL;
 	int lookup_flags = 0;
 
-	if ((flag & ~AT_SYMLINK_NOFOLLOW) != 0)
+	if ((flag & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)) != 0)
 		goto out;
 
 	if (!(flag & AT_SYMLINK_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
+	if (flag & AT_NO_AUTOMOUNT)
+		lookup_flags |= LOOKUP_NO_AUTOMOUNT;
 
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (error)
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index afc00af..a562fa5 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -45,6 +45,7 @@
 #define AT_REMOVEDIR		0x200   /* Remove directory instead of
                                            unlinking file.  */
 #define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 05b441d..1e1febf 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -43,12 +43,14 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - internal "there are more path components" flag
  *  - locked when lookup done with dcache_lock held
  *  - dentry cache is untrusted; force a real lookup
+ *  - suppress terminal automount
  */
 #define LOOKUP_FOLLOW		 1
 #define LOOKUP_DIRECTORY	 2
 #define LOOKUP_CONTINUE		 4
 #define LOOKUP_PARENT		16
 #define LOOKUP_REVAL		64
+#define LOOKUP_NO_AUTOMOUNT	128
 /*
  * Intent data
  */

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-23 15:09     ` David Howells
@ 2010-07-24  4:11         ` Ian Kent
  -1 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-24  4:11 UTC (permalink / raw)
  To: David Howells
  Cc: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Fri, 2010-07-23 at 16:09 +0100, David Howells wrote:
> Here's an updated patch that:
> 
>  (1) Fixes a bug in error handling (needs to use path_put_conditional not
>      path_put).
> 
>  (2) Absorbs autofs4's decisions about whether to automount or not.  This
>      means that colour-ls won't cause automounts unless -L is supplied or it
>      doesn't get a DT_DIR flag from getdents().  It also means that autofs4
>      can dispense with this logic should it choose to use d_automount().

I think my statements about this were a little incorrect.

In the current kernel the check isn't imposed for ->lookup() but only
->d_revalidate() (the deosn't already exist vs the already exists
cases). Since ->d_lookup() (currently) leaves the dentry negative until
->mkdir() that could be used in the check. But then this may be starting
to get a little too autofs specific, maybe we should re-consider passing
the flags, I don't know.

In any case I'll have a go at using this as is, and see what happens.

> 
>  (3) Moves all the automounter logic out of __follow_mount() into
>      follow_automount().
> 
>  (4) Stops pathwalk at the automount point and returns that point in the fs
>      tree if it decides not to automount rather than reporting ELOOP (see its
>      use of EXDEV for this).
> 
> David
> 
> ---
> From: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Subject: [PATCH] Add a dentry op to handle automounting rather than abusing follow_link()
> 
> Add a dentry op (d_automount) to handle automounting directories rather than
> abusing the follow_link() inode operation.  The operation is keyed off a new
> inode flag (S_AUTOMOUNT).
> 
> This makes it easier to add an AT_ flag to suppress terminal segment automount
> during pathwalk.  It should also remove the need for the kludge code in the
> pathwalk algorithm to handle directories with follow_link() semantics.
> 
> I've only changed __follow_mount() to handle automount points, but it might be
> necessary to change follow_mount() too.  The latter is only used from
> follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
> a child of it.
> 
> I've also extracted the mount/don't-mount logic from autofs4 and included it
> here.  It makes the mount go ahead anyway if someone calls open() or creat(),
> tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
> or sticks a '/' on the end of the pathname.  If they do a stat(), however,
> they'll only trigger the automount if they didn't also say O_NOFOLLOW.
> 
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
>  Documentation/filesystems/Locking |    2 +
>  Documentation/filesystems/vfs.txt |   13 +++++
>  fs/namei.c                        |   96 ++++++++++++++++++++++++++++++-------
>  include/linux/dcache.h            |    5 ++
>  include/linux/fs.h                |    2 +
>  5 files changed, 100 insertions(+), 18 deletions(-)
> 
> 
> diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
> index 96d4293..ccbfa98 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -16,6 +16,7 @@ prototypes:
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
> +	struct vfsmount *(*d_automount)(struct path *path);
>  
>  locking rules:
>  	none have BKL
> @@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
>  d_release:	no		no		no		yes
>  d_iput:		no		no		no		yes
>  d_dname:	no		no		no		no
> +d_automount:	no		no		no		yes
>  
>  --------------------------- inode_operations --------------------------- 
>  prototypes:
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index 94677e7..31a9e8f 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -851,6 +851,7 @@ struct dentry_operations {
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)(struct dentry *, char *, int);
> +	struct vfsmount *(*d_automount)(struct path *);
>  };
>  
>    d_revalidate: called when the VFS needs to revalidate a dentry. This
> @@ -885,6 +886,18 @@ struct dentry_operations {
>  	at the end of the buffer, and returns a pointer to the first char.
>  	dynamic_dname() helper function is provided to take care of this.
>  
> +  d_automount: called when an automount dentry is to be traversed (optional).
> +	This should create a new VFS mount record, mount it on the directory
> +	and return the record to the caller.  The caller is supplied with a
> +	path parameter giving the automount directory to describe the automount
> +	target and the parent VFS mount record to provide inheritable mount
> +	parameters.  NULL should be returned if someone else managed to make
> +	the automount first.  If the automount failed, then an error code
> +	should be returned.
> +
> +	This function is only used if S_AUTOMOUNT is set on the inode to which
> +	the dentry refers.
> +
>  Example :
>  
>  static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
> diff --git a/fs/namei.c b/fs/namei.c
> index 868d0cb..6509ca5 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -617,24 +617,82 @@ int follow_up(struct path *path)
>  	return 1;
>  }
>  
> +/*
> + * Perform an automount
> + * - return -EXDEV to tell __follow_mount() to stop and return the path we were
> + *   called with.
> + */
> +static int follow_automount(struct path *path, unsigned flags, int res)
> +{
> +	struct vfsmount *mnt;
> +
> +	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
> +		return -EREMOTE;
> +
> +	/* We want to mount if someone is trying to open/create a file of any
> +	 * type under the mountpoint, wants to traverse through the mountpoint
> +	 * or wants to open the mounted directory.
> +	 *
> +	 * We don't want to mount if someone's just doing a stat and they've
> +	 * set AT_SYMLINK_NOFOLLOW - unless they're stat'ing a directory and
> +	 * appended a '/' to the name.
> +	 */
> +	if (!(flags & LOOKUP_FOLLOW) &&
> +	    !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY |
> +		       LOOKUP_OPEN | LOOKUP_CREATE)))
> +		return -EXDEV;
> +
> +	current->total_link_count++;
> +	if (current->total_link_count >= 40)
> +		return -ELOOP;
> +
> +	mnt = path->dentry->d_op->d_automount(path);
> +	if (IS_ERR(mnt))
> +		return PTR_ERR(mnt);
> +	if (!mnt) /* mount collision */
> +		return 0;
> +
> +	if (mnt->mnt_sb == path->mnt->mnt_sb &&
> +	    mnt->mnt_root == path->dentry) {
> +		mntput(mnt);
> +		return -ELOOP;
> +	}
> +
> +	dput(path->dentry);
> +	if (res)
> +		mntput(path->mnt);
> +	path->mnt = mnt;
> +	path->dentry = dget(mnt->mnt_root);
> +	return 0;
> +}
> +
>  /* no need for dcache_lock, as serialization is taken care in
>   * namespace.c
>   */
> -static int __follow_mount(struct path *path)
> +static int __follow_mount(struct path *path, unsigned flags)
>  {
> -	int res = 0;
> -	while (d_mountpoint(path->dentry)) {
> -		struct vfsmount *mounted = lookup_mnt(path);
> -		if (!mounted)
> +	struct vfsmount *mounted;
> +	int ret, res = 0;
> +	for (;;) {
> +		while (d_mountpoint(path->dentry)) {
> +			mounted = lookup_mnt(path);
> +			if (!mounted)
> +				break;
> +			dput(path->dentry);
> +			if (res)
> +				mntput(path->mnt);
> +			path->mnt = mounted;
> +			path->dentry = dget(mounted->mnt_root);
> +			res = 1;
> +		}
> +		if (!d_automount_point(path->dentry))
>  			break;
> -		dput(path->dentry);
> -		if (res)
> -			mntput(path->mnt);
> -		path->mnt = mounted;
> -		path->dentry = dget(mounted->mnt_root);
> +		ret = follow_automount(path, flags, res);
> +		if (ret < 0)
> +			return ret == -EXDEV ? 0 : ret;
>  		res = 1;
>  	}
> -	return res;
> +	return 0;
>  }
>  
>  static void follow_mount(struct path *path)
> @@ -702,6 +760,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
>  	struct vfsmount *mnt = nd->path.mnt;
>  	struct dentry *dentry, *parent;
>  	struct inode *dir;
> +	int ret;
> +
>  	/*
>  	 * See if the low-level filesystem might want
>  	 * to use its own hash..
> @@ -720,8 +780,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
>  done:
>  	path->mnt = mnt;
>  	path->dentry = dentry;
> -	__follow_mount(path);
> -	return 0;
> +	ret = __follow_mount(path, nd->flags);
> +	if (unlikely(ret < 0))
> +		path_put_conditional(path, nd);
> +	return ret;
>  
>  need_lookup:
>  	parent = nd->path.dentry;
> @@ -1721,11 +1783,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
>  	if (open_flag & O_EXCL)
>  		goto exit_dput;
>  
> -	if (__follow_mount(path)) {
> -		error = -ELOOP;
> -		if (open_flag & O_NOFOLLOW)
> -			goto exit_dput;
> -	}
> +	error = __follow_mount(path, nd->flags);
> +	if (error < 0)
> +		goto exit_dput;
>  
>  	error = -ENOENT;
>  	if (!path->dentry->d_inode)
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index eebb617..5380bff 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -139,6 +139,7 @@ struct dentry_operations {
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)(struct dentry *, char *, int);
> +	struct vfsmount *(*d_automount)(struct path *);
>  };
>  
>  /* the dentry parameter passed to d_hash and d_compare is the parent
> @@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
>  d_delete:	no		yes		no       no
>  d_release:	no		no		no       yes
>  d_iput:		no		no		no       yes
> +d_automount:	no		no		no	 yes
>   */
>  
>  /* d_flags entries */
> @@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
>  	return dentry->d_mounted;
>  }
>  
> +#define d_automount_point(dentry) \
> +	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
> +
>  extern struct vfsmount *lookup_mnt(struct path *);
>  extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 68ca1b0..a83fc81 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -235,6 +235,7 @@ struct inodes_stat_t {
>  #define S_NOCMTIME	128	/* Do not update file c/mtime */
>  #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
>  #define S_PRIVATE	512	/* Inode is fs-internal */
> +#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
>  
>  /*
>   * Note that nosuid etc flags are inode-specific: setting some file-system
> @@ -269,6 +270,7 @@ struct inodes_stat_t {
>  #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
>  #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
>  #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
> +#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
>  
>  /* the read-only stuff doesn't really belong here, but any other place is
>     probably as bad and I don't want to create yet another include file. */

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
@ 2010-07-24  4:11         ` Ian Kent
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-24  4:11 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel

On Fri, 2010-07-23 at 16:09 +0100, David Howells wrote:
> Here's an updated patch that:
> 
>  (1) Fixes a bug in error handling (needs to use path_put_conditional not
>      path_put).
> 
>  (2) Absorbs autofs4's decisions about whether to automount or not.  This
>      means that colour-ls won't cause automounts unless -L is supplied or it
>      doesn't get a DT_DIR flag from getdents().  It also means that autofs4
>      can dispense with this logic should it choose to use d_automount().

I think my statements about this were a little incorrect.

In the current kernel the check isn't imposed for ->lookup() but only
->d_revalidate() (the deosn't already exist vs the already exists
cases). Since ->d_lookup() (currently) leaves the dentry negative until
->mkdir() that could be used in the check. But then this may be starting
to get a little too autofs specific, maybe we should re-consider passing
the flags, I don't know.

In any case I'll have a go at using this as is, and see what happens.

> 
>  (3) Moves all the automounter logic out of __follow_mount() into
>      follow_automount().
> 
>  (4) Stops pathwalk at the automount point and returns that point in the fs
>      tree if it decides not to automount rather than reporting ELOOP (see its
>      use of EXDEV for this).
> 
> David
> 
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] Add a dentry op to handle automounting rather than abusing follow_link()
> 
> Add a dentry op (d_automount) to handle automounting directories rather than
> abusing the follow_link() inode operation.  The operation is keyed off a new
> inode flag (S_AUTOMOUNT).
> 
> This makes it easier to add an AT_ flag to suppress terminal segment automount
> during pathwalk.  It should also remove the need for the kludge code in the
> pathwalk algorithm to handle directories with follow_link() semantics.
> 
> I've only changed __follow_mount() to handle automount points, but it might be
> necessary to change follow_mount() too.  The latter is only used from
> follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
> a child of it.
> 
> I've also extracted the mount/don't-mount logic from autofs4 and included it
> here.  It makes the mount go ahead anyway if someone calls open() or creat(),
> tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
> or sticks a '/' on the end of the pathname.  If they do a stat(), however,
> they'll only trigger the automount if they didn't also say O_NOFOLLOW.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  Documentation/filesystems/Locking |    2 +
>  Documentation/filesystems/vfs.txt |   13 +++++
>  fs/namei.c                        |   96 ++++++++++++++++++++++++++++++-------
>  include/linux/dcache.h            |    5 ++
>  include/linux/fs.h                |    2 +
>  5 files changed, 100 insertions(+), 18 deletions(-)
> 
> 
> diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
> index 96d4293..ccbfa98 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -16,6 +16,7 @@ prototypes:
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
> +	struct vfsmount *(*d_automount)(struct path *path);
>  
>  locking rules:
>  	none have BKL
> @@ -27,6 +28,7 @@ d_delete:	yes		no		yes		no
>  d_release:	no		no		no		yes
>  d_iput:		no		no		no		yes
>  d_dname:	no		no		no		no
> +d_automount:	no		no		no		yes
>  
>  --------------------------- inode_operations --------------------------- 
>  prototypes:
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index 94677e7..31a9e8f 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -851,6 +851,7 @@ struct dentry_operations {
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)(struct dentry *, char *, int);
> +	struct vfsmount *(*d_automount)(struct path *);
>  };
>  
>    d_revalidate: called when the VFS needs to revalidate a dentry. This
> @@ -885,6 +886,18 @@ struct dentry_operations {
>  	at the end of the buffer, and returns a pointer to the first char.
>  	dynamic_dname() helper function is provided to take care of this.
>  
> +  d_automount: called when an automount dentry is to be traversed (optional).
> +	This should create a new VFS mount record, mount it on the directory
> +	and return the record to the caller.  The caller is supplied with a
> +	path parameter giving the automount directory to describe the automount
> +	target and the parent VFS mount record to provide inheritable mount
> +	parameters.  NULL should be returned if someone else managed to make
> +	the automount first.  If the automount failed, then an error code
> +	should be returned.
> +
> +	This function is only used if S_AUTOMOUNT is set on the inode to which
> +	the dentry refers.
> +
>  Example :
>  
>  static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
> diff --git a/fs/namei.c b/fs/namei.c
> index 868d0cb..6509ca5 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -617,24 +617,82 @@ int follow_up(struct path *path)
>  	return 1;
>  }
>  
> +/*
> + * Perform an automount
> + * - return -EXDEV to tell __follow_mount() to stop and return the path we were
> + *   called with.
> + */
> +static int follow_automount(struct path *path, unsigned flags, int res)
> +{
> +	struct vfsmount *mnt;
> +
> +	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
> +		return -EREMOTE;
> +
> +	/* We want to mount if someone is trying to open/create a file of any
> +	 * type under the mountpoint, wants to traverse through the mountpoint
> +	 * or wants to open the mounted directory.
> +	 *
> +	 * We don't want to mount if someone's just doing a stat and they've
> +	 * set AT_SYMLINK_NOFOLLOW - unless they're stat'ing a directory and
> +	 * appended a '/' to the name.
> +	 */
> +	if (!(flags & LOOKUP_FOLLOW) &&
> +	    !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY |
> +		       LOOKUP_OPEN | LOOKUP_CREATE)))
> +		return -EXDEV;
> +
> +	current->total_link_count++;
> +	if (current->total_link_count >= 40)
> +		return -ELOOP;
> +
> +	mnt = path->dentry->d_op->d_automount(path);
> +	if (IS_ERR(mnt))
> +		return PTR_ERR(mnt);
> +	if (!mnt) /* mount collision */
> +		return 0;
> +
> +	if (mnt->mnt_sb == path->mnt->mnt_sb &&
> +	    mnt->mnt_root == path->dentry) {
> +		mntput(mnt);
> +		return -ELOOP;
> +	}
> +
> +	dput(path->dentry);
> +	if (res)
> +		mntput(path->mnt);
> +	path->mnt = mnt;
> +	path->dentry = dget(mnt->mnt_root);
> +	return 0;
> +}
> +
>  /* no need for dcache_lock, as serialization is taken care in
>   * namespace.c
>   */
> -static int __follow_mount(struct path *path)
> +static int __follow_mount(struct path *path, unsigned flags)
>  {
> -	int res = 0;
> -	while (d_mountpoint(path->dentry)) {
> -		struct vfsmount *mounted = lookup_mnt(path);
> -		if (!mounted)
> +	struct vfsmount *mounted;
> +	int ret, res = 0;
> +	for (;;) {
> +		while (d_mountpoint(path->dentry)) {
> +			mounted = lookup_mnt(path);
> +			if (!mounted)
> +				break;
> +			dput(path->dentry);
> +			if (res)
> +				mntput(path->mnt);
> +			path->mnt = mounted;
> +			path->dentry = dget(mounted->mnt_root);
> +			res = 1;
> +		}
> +		if (!d_automount_point(path->dentry))
>  			break;
> -		dput(path->dentry);
> -		if (res)
> -			mntput(path->mnt);
> -		path->mnt = mounted;
> -		path->dentry = dget(mounted->mnt_root);
> +		ret = follow_automount(path, flags, res);
> +		if (ret < 0)
> +			return ret == -EXDEV ? 0 : ret;
>  		res = 1;
>  	}
> -	return res;
> +	return 0;
>  }
>  
>  static void follow_mount(struct path *path)
> @@ -702,6 +760,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
>  	struct vfsmount *mnt = nd->path.mnt;
>  	struct dentry *dentry, *parent;
>  	struct inode *dir;
> +	int ret;
> +
>  	/*
>  	 * See if the low-level filesystem might want
>  	 * to use its own hash..
> @@ -720,8 +780,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
>  done:
>  	path->mnt = mnt;
>  	path->dentry = dentry;
> -	__follow_mount(path);
> -	return 0;
> +	ret = __follow_mount(path, nd->flags);
> +	if (unlikely(ret < 0))
> +		path_put_conditional(path, nd);
> +	return ret;
>  
>  need_lookup:
>  	parent = nd->path.dentry;
> @@ -1721,11 +1783,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
>  	if (open_flag & O_EXCL)
>  		goto exit_dput;
>  
> -	if (__follow_mount(path)) {
> -		error = -ELOOP;
> -		if (open_flag & O_NOFOLLOW)
> -			goto exit_dput;
> -	}
> +	error = __follow_mount(path, nd->flags);
> +	if (error < 0)
> +		goto exit_dput;
>  
>  	error = -ENOENT;
>  	if (!path->dentry->d_inode)
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index eebb617..5380bff 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -139,6 +139,7 @@ struct dentry_operations {
>  	void (*d_release)(struct dentry *);
>  	void (*d_iput)(struct dentry *, struct inode *);
>  	char *(*d_dname)(struct dentry *, char *, int);
> +	struct vfsmount *(*d_automount)(struct path *);
>  };
>  
>  /* the dentry parameter passed to d_hash and d_compare is the parent
> @@ -157,6 +158,7 @@ d_compare:	no		yes		yes      no
>  d_delete:	no		yes		no       no
>  d_release:	no		no		no       yes
>  d_iput:		no		no		no       yes
> +d_automount:	no		no		no	 yes
>   */
>  
>  /* d_flags entries */
> @@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
>  	return dentry->d_mounted;
>  }
>  
> +#define d_automount_point(dentry) \
> +	(dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
> +
>  extern struct vfsmount *lookup_mnt(struct path *);
>  extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 68ca1b0..a83fc81 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -235,6 +235,7 @@ struct inodes_stat_t {
>  #define S_NOCMTIME	128	/* Do not update file c/mtime */
>  #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
>  #define S_PRIVATE	512	/* Inode is fs-internal */
> +#define S_AUTOMOUNT	1024	/* Automount/referral quasi-directory */
>  
>  /*
>   * Note that nosuid etc flags are inode-specific: setting some file-system
> @@ -269,6 +270,7 @@ struct inodes_stat_t {
>  #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
>  #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
>  #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
> +#define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
>  
>  /* the read-only stuff doesn't really belong here, but any other place is
>     probably as bad and I don't want to create yet another include file. */



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-23 15:09     ` David Howells
  (?)
  (?)
@ 2010-07-26 14:19     ` David Howells
       [not found]       ` <9168.1280153997-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  2010-07-26 15:54       ` David Howells
  -1 siblings, 2 replies; 27+ messages in thread
From: David Howells @ 2010-07-26 14:19 UTC (permalink / raw)
  To: Ian Kent
  Cc: dhowells, viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel


Ian Kent <raven@themaw.net> wrote:

> >  (4) Stops pathwalk at the automount point and returns that point in the fs
> >      tree if it decides not to automount rather than reporting ELOOP (see its
> >      use of EXDEV for this).

Does it make autofs easier if d_op->d_automount() is allowed to return -EXDEV
to request this?  Then you can return it in Oz mode to allow the daemon to
see/use the underlying mountpoint without recursing back into d_automount().

Ideally, the daemon would use AT_NO_AUTOMOUNT, but there's no way to pass that
to sys_mount() or sys_umount().

David

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-26 14:19     ` David Howells
@ 2010-07-26 15:22           ` Ian Kent
  2010-07-26 15:54       ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-26 15:22 UTC (permalink / raw)
  To: David Howells
  Cc: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Mon, 2010-07-26 at 15:19 +0100, David Howells wrote:
> Ian Kent <raven-PKsaG3nR2I+sTnJN9+BGXg@public.gmane.org> wrote:
> 
> > >  (4) Stops pathwalk at the automount point and returns that point in the fs
> > >      tree if it decides not to automount rather than reporting ELOOP (see its
> > >      use of EXDEV for this).
> 
> Does it make autofs easier if d_op->d_automount() is allowed to return -EXDEV
> to request this?  Then you can return it in Oz mode to allow the daemon to
> see/use the underlying mountpoint without recursing back into d_automount().

Yes, it's really useful.

> 
> Ideally, the daemon would use AT_NO_AUTOMOUNT, but there's no way to pass that
> to sys_mount() or sys_umount().

The use of EXDEV, and if we had a way of saying we want a negative
dentry to trigger a mount, allows this mechanism to work for autofs
without any user space changes for direct and indirect mounts, at least
to match the current function.

Having said that though I've only just begun to test the various cases.

I know this was originally meant to deal with just the follow_link abuse
but the approach as it is appears to be able to resolve a long standing
deadlock bug autofs has with indirect mounts and affords a huge amount
of simplification to the autofs module code. I really hope we can work
out a suitable way to change the implementation to allow for negative
dentrys to trigger mounts. Of course it's quite possible I'll hit a snag
during testing but I hope not as I've spent a lot of time on alternate
approaches in the last few years.

Ian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
@ 2010-07-26 15:22           ` Ian Kent
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-26 15:22 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel

On Mon, 2010-07-26 at 15:19 +0100, David Howells wrote:
> Ian Kent <raven@themaw.net> wrote:
> 
> > >  (4) Stops pathwalk at the automount point and returns that point in the fs
> > >      tree if it decides not to automount rather than reporting ELOOP (see its
> > >      use of EXDEV for this).
> 
> Does it make autofs easier if d_op->d_automount() is allowed to return -EXDEV
> to request this?  Then you can return it in Oz mode to allow the daemon to
> see/use the underlying mountpoint without recursing back into d_automount().

Yes, it's really useful.

> 
> Ideally, the daemon would use AT_NO_AUTOMOUNT, but there's no way to pass that
> to sys_mount() or sys_umount().

The use of EXDEV, and if we had a way of saying we want a negative
dentry to trigger a mount, allows this mechanism to work for autofs
without any user space changes for direct and indirect mounts, at least
to match the current function.

Having said that though I've only just begun to test the various cases.

I know this was originally meant to deal with just the follow_link abuse
but the approach as it is appears to be able to resolve a long standing
deadlock bug autofs has with indirect mounts and affords a huge amount
of simplification to the autofs module code. I really hope we can work
out a suitable way to change the implementation to allow for negative
dentrys to trigger mounts. Of course it's quite possible I'll hit a snag
during testing but I hope not as I've spent a lot of time on alternate
approaches in the last few years.

Ian


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-26 14:19     ` David Howells
       [not found]       ` <9168.1280153997-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-07-26 15:54       ` David Howells
       [not found]         ` <30118.1280159695-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  2010-07-27  8:48         ` David Howells
  1 sibling, 2 replies; 27+ messages in thread
From: David Howells @ 2010-07-26 15:54 UTC (permalink / raw)
  To: Ian Kent
  Cc: dhowells, viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel

Ian Kent <raven@themaw.net> wrote:

> > Does it make autofs easier if d_op->d_automount() is allowed to return
> > -EXDEV to request this?  Then you can return it in Oz mode to allow the
> > daemon to see/use the underlying mountpoint without recursing back into
> > d_automount().
> 
> Yes, it's really useful.

I think what's required, then, is if d_automount() returns -EXDEV then:

 (1) If the dentry is terminal in the lookup path, then we just return -EXDEV
     to indicate to __follow_mount() that we really do want to stop there.

 (2) If the dentry is not terminal, then we convert the error to -EREMOTE to
     indicate that we can't complete the pathwalk as one of the earlier
     components is inaccessible.

See the attached patch.

David
---
diff --git a/fs/namei.c b/fs/namei.c
index c154112..6c385d4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -653,8 +653,20 @@ static int follow_automount(struct path *path, unsigned flags, int res)
 		return -ELOOP;
 
 	mnt = path->dentry->d_op->d_automount(path);
-	if (IS_ERR(mnt))
+	if (IS_ERR(mnt)) {
+		/*
+		 * The filesystem is allowed to return -EXDEV here to indicate
+		 * they don't want to automount.  For instance, autofs would do
+		 * this so that its userspace daemon can mount on this dentry.
+		 *
+		 * However, we can only permit this if it's a terminal point in
+		 * the path being looked up; if it wasn't then the remainder of
+		 * the path is inaccessible and we should say so.
+		 */
+		if (PTR_ERR(mnt) == -EXDEV && (flags & LOOKUP_CONTINUE))
+			return -EREMOTE;
 		return PTR_ERR(mnt);
+	}
 	if (!mnt) /* mount collision */
 		return 0;
 

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-26 15:54       ` David Howells
@ 2010-07-27  3:43             ` Ian Kent
  2010-07-27  8:48         ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-27  3:43 UTC (permalink / raw)
  To: David Howells
  Cc: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Mon, 2010-07-26 at 16:54 +0100, David Howells wrote:
> Ian Kent <raven-PKsaG3nR2I+sTnJN9+BGXg@public.gmane.org> wrote:
> 
> > > Does it make autofs easier if d_op->d_automount() is allowed to return
> > > -EXDEV to request this?  Then you can return it in Oz mode to allow the
> > > daemon to see/use the underlying mountpoint without recursing back into
> > > d_automount().
> > 
> > Yes, it's really useful.
> 
> I think what's required, then, is if d_automount() returns -EXDEV then:
> 
>  (1) If the dentry is terminal in the lookup path, then we just return -EXDEV
>      to indicate to __follow_mount() that we really do want to stop there.
> 
>  (2) If the dentry is not terminal, then we convert the error to -EREMOTE to
>      indicate that we can't complete the pathwalk as one of the earlier
>      components is inaccessible.

Is this something others need?

Again, the exists vs not yet exists case for paths within indirect
autofs mounts. At the moment I can just set the flag on all dentrys in
the autofs fs and return EXDEV for non-empty directories in order to
return the dentry as a path component. OTOH if the dentry is a mount
embeded in the path and the mount fails we get a error return.

I could clear the flag on non-root parent dentrys during mkdir if this
is needed by others.
  
> 
> See the attached patch.
> 
> David
> ---
> diff --git a/fs/namei.c b/fs/namei.c
> index c154112..6c385d4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -653,8 +653,20 @@ static int follow_automount(struct path *path, unsigned flags, int res)
>  		return -ELOOP;
>  
>  	mnt = path->dentry->d_op->d_automount(path);
> -	if (IS_ERR(mnt))
> +	if (IS_ERR(mnt)) {
> +		/*
> +		 * The filesystem is allowed to return -EXDEV here to indicate
> +		 * they don't want to automount.  For instance, autofs would do
> +		 * this so that its userspace daemon can mount on this dentry.
> +		 *
> +		 * However, we can only permit this if it's a terminal point in
> +		 * the path being looked up; if it wasn't then the remainder of
> +		 * the path is inaccessible and we should say so.
> +		 */
> +		if (PTR_ERR(mnt) == -EXDEV && (flags & LOOKUP_CONTINUE))
> +			return -EREMOTE;
>  		return PTR_ERR(mnt);
> +	}
>  	if (!mnt) /* mount collision */
>  		return 0;
>  
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
@ 2010-07-27  3:43             ` Ian Kent
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-27  3:43 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel

On Mon, 2010-07-26 at 16:54 +0100, David Howells wrote:
> Ian Kent <raven@themaw.net> wrote:
> 
> > > Does it make autofs easier if d_op->d_automount() is allowed to return
> > > -EXDEV to request this?  Then you can return it in Oz mode to allow the
> > > daemon to see/use the underlying mountpoint without recursing back into
> > > d_automount().
> > 
> > Yes, it's really useful.
> 
> I think what's required, then, is if d_automount() returns -EXDEV then:
> 
>  (1) If the dentry is terminal in the lookup path, then we just return -EXDEV
>      to indicate to __follow_mount() that we really do want to stop there.
> 
>  (2) If the dentry is not terminal, then we convert the error to -EREMOTE to
>      indicate that we can't complete the pathwalk as one of the earlier
>      components is inaccessible.

Is this something others need?

Again, the exists vs not yet exists case for paths within indirect
autofs mounts. At the moment I can just set the flag on all dentrys in
the autofs fs and return EXDEV for non-empty directories in order to
return the dentry as a path component. OTOH if the dentry is a mount
embeded in the path and the mount fails we get a error return.

I could clear the flag on non-root parent dentrys during mkdir if this
is needed by others.
  
> 
> See the attached patch.
> 
> David
> ---
> diff --git a/fs/namei.c b/fs/namei.c
> index c154112..6c385d4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -653,8 +653,20 @@ static int follow_automount(struct path *path, unsigned flags, int res)
>  		return -ELOOP;
>  
>  	mnt = path->dentry->d_op->d_automount(path);
> -	if (IS_ERR(mnt))
> +	if (IS_ERR(mnt)) {
> +		/*
> +		 * The filesystem is allowed to return -EXDEV here to indicate
> +		 * they don't want to automount.  For instance, autofs would do
> +		 * this so that its userspace daemon can mount on this dentry.
> +		 *
> +		 * However, we can only permit this if it's a terminal point in
> +		 * the path being looked up; if it wasn't then the remainder of
> +		 * the path is inaccessible and we should say so.
> +		 */
> +		if (PTR_ERR(mnt) == -EXDEV && (flags & LOOKUP_CONTINUE))
> +			return -EREMOTE;
>  		return PTR_ERR(mnt);
> +	}
>  	if (!mnt) /* mount collision */
>  		return 0;
>  
> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-26 15:54       ` David Howells
       [not found]         ` <30118.1280159695-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-07-27  8:48         ` David Howells
       [not found]           ` <12883.1280220521-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  1 sibling, 1 reply; 27+ messages in thread
From: David Howells @ 2010-07-27  8:48 UTC (permalink / raw)
  To: Ian Kent
  Cc: dhowells, viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs,
	linux-kernel

Ian Kent <raven@themaw.net> wrote:

> Is this something others need?

Not as far as I know...  I think autofs is the only one doing out-of-kernel
automounting.

That doesn't mean it shouldn't be provided, though...

> Again, the exists vs not yet exists case for paths within indirect
> autofs mounts. At the moment I can just set the flag on all dentrys in
> the autofs fs and return EXDEV for non-empty directories in order to
> return the dentry as a path component. OTOH if the dentry is a mount
> embeded in the path and the mount fails we get a error return.

Seems redundant, but I'd say go with it for now.  Maybe we can offload
S_AUTOMOUNT to the dentry.

> I could clear the flag on non-root parent dentrys during mkdir if this
> is needed by others.

I'm not sure that would actually matter, since it would come to
follow_automount() at the same place.

Note that someone who tries to stat() with AT_NO_AUTOMOUNT will cause the call
to d_automount() to be suppressed and will see the negative or non-mounted
directory.  That might be okay for you.

David

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
  2010-07-27  8:48         ` David Howells
@ 2010-07-27 12:51               ` Ian Kent
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-27 12:51 UTC (permalink / raw)
  To: David Howells
  Cc: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, 2010-07-27 at 09:48 +0100, David Howells wrote:
> Ian Kent <raven-PKsaG3nR2I+sTnJN9+BGXg@public.gmane.org> wrote:
> 
> > Is this something others need?
> 
> Not as far as I know...  I think autofs is the only one doing out-of-kernel
> automounting.
> 
> That doesn't mean it shouldn't be provided, though...
> 
> > Again, the exists vs not yet exists case for paths within indirect
> > autofs mounts. At the moment I can just set the flag on all dentrys in
> > the autofs fs and return EXDEV for non-empty directories in order to
> > return the dentry as a path component. OTOH if the dentry is a mount
> > embeded in the path and the mount fails we get a error return.
> 
> Seems redundant, but I'd say go with it for now.  Maybe we can offload
> S_AUTOMOUNT to the dentry.

Yes, it is redundant but it is simple and works fine as long as I don't
apply the patch to return -EREMOTE for non-terminal dentrys. I'd need to
clear the flag, as below, on non-terminal dentrys if the -EREMOTE patch
is needed.

> 
> > I could clear the flag on non-root parent dentrys during mkdir if this
> > is needed by others.
> 
> I'm not sure that would actually matter, since it would come to
> follow_automount() at the same place.
> 
> Note that someone who tries to stat() with AT_NO_AUTOMOUNT will cause the call
> to d_automount() to be suppressed and will see the negative or non-mounted
> directory.  That might be okay for you.

With one small correction to the autofs patch I've been able to
successfully run the autofs Connectathon parser test without any user
space changes for both the nobrowse (the usual case) and the browse
(pre-created mount point directories) cases. The test consists of 230
mounts, with success and fail cases, for both direct and indirect
mounts. These mounts include various offset mount constructs as well. To
consider the test a success I require the mounts to also expire
correctly.

There are more tests to run, such as the program mount test, and the
often problematic submount test. Also, I have a submount based stress
test.

But, all in all, this is shaping up to be just what I need. Having one
single place to call back to the daemon, where I don't need to play
around with the inode mutex, makes me think this implementation will
finally resolve my deadlock issue with indirect mounts.

Ian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
@ 2010-07-27 12:51               ` Ian Kent
  0 siblings, 0 replies; 27+ messages in thread
From: Ian Kent @ 2010-07-27 12:51 UTC (permalink / raw)
  To: David Howells
  Cc: viro, linux-fsdevel, linux-afs, linux-nfs, linux-cifs, linux-kernel

On Tue, 2010-07-27 at 09:48 +0100, David Howells wrote:
> Ian Kent <raven@themaw.net> wrote:
> 
> > Is this something others need?
> 
> Not as far as I know...  I think autofs is the only one doing out-of-kernel
> automounting.
> 
> That doesn't mean it shouldn't be provided, though...
> 
> > Again, the exists vs not yet exists case for paths within indirect
> > autofs mounts. At the moment I can just set the flag on all dentrys in
> > the autofs fs and return EXDEV for non-empty directories in order to
> > return the dentry as a path component. OTOH if the dentry is a mount
> > embeded in the path and the mount fails we get a error return.
> 
> Seems redundant, but I'd say go with it for now.  Maybe we can offload
> S_AUTOMOUNT to the dentry.

Yes, it is redundant but it is simple and works fine as long as I don't
apply the patch to return -EREMOTE for non-terminal dentrys. I'd need to
clear the flag, as below, on non-terminal dentrys if the -EREMOTE patch
is needed.

> 
> > I could clear the flag on non-root parent dentrys during mkdir if this
> > is needed by others.
> 
> I'm not sure that would actually matter, since it would come to
> follow_automount() at the same place.
> 
> Note that someone who tries to stat() with AT_NO_AUTOMOUNT will cause the call
> to d_automount() to be suppressed and will see the negative or non-mounted
> directory.  That might be okay for you.

With one small correction to the autofs patch I've been able to
successfully run the autofs Connectathon parser test without any user
space changes for both the nobrowse (the usual case) and the browse
(pre-created mount point directories) cases. The test consists of 230
mounts, with success and fail cases, for both direct and indirect
mounts. These mounts include various offset mount constructs as well. To
consider the test a success I require the mounts to also expire
correctly.

There are more tests to run, such as the program mount test, and the
often problematic submount test. Also, I have a submount based stress
test.

But, all in all, this is shaping up to be just what I need. Having one
single place to call back to the daemon, where I don't need to play
around with the inode mutex, makes me think this implementation will
finally resolve my deadlock issue with indirect mounts.

Ian



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2010-07-27 12:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-22 17:58 [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
2010-07-22 17:58 ` David Howells
2010-07-22 17:58 ` [PATCH 2/6] AFS: Use d_automount() " David Howells
2010-07-22 17:59 ` [PATCH 5/6] Remove the automount through follow_link() kludge code from pathwalk David Howells
2010-07-22 17:59 ` [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount David Howells
     [not found] ` <20100722175847.5552.11520.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
2010-07-22 17:58   ` [PATCH 3/6] NFS: Use d_automount() rather than abusing follow_link() David Howells
2010-07-22 17:58     ` David Howells
2010-07-22 17:59   ` [PATCH 4/6] CIFS: " David Howells
2010-07-22 17:59     ` David Howells
2010-07-23 15:09   ` [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2] David Howells
2010-07-23 15:09     ` David Howells
     [not found]     ` <17723.1279897759-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-07-24  4:11       ` Ian Kent
2010-07-24  4:11         ` Ian Kent
2010-07-26 14:19     ` David Howells
     [not found]       ` <9168.1280153997-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-07-26 15:22         ` Ian Kent
2010-07-26 15:22           ` Ian Kent
2010-07-26 15:54       ` David Howells
     [not found]         ` <30118.1280159695-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-07-27  3:43           ` Ian Kent
2010-07-27  3:43             ` Ian Kent
2010-07-27  8:48         ` David Howells
     [not found]           ` <12883.1280220521-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-07-27 12:51             ` Ian Kent
2010-07-27 12:51               ` Ian Kent
     [not found] ` <20100722175913.5552.3905.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
2010-07-22 18:01   ` [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount David Howells
2010-07-22 18:01     ` David Howells
2010-07-23 15:11   ` [PATCH 6/6] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount [ver #2] David Howells
2010-07-23 15:11     ` David Howells
2010-07-23 15:11     ` David Howells

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.