All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/18] Introduce automount support in the VFS
@ 2011-01-11 17:48 David Howells
  2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
                   ` (19 more replies)
  0 siblings, 20 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel


Hi Nick,

Could you look over these patches, please?  They were surviving Ian's autofs
tests quite nicely, but I had to rework them a bit to take account of your
RCU-mode pathwalk changes.  I'm not sure the locking is fully correct.  In some
of the autofs patches you'll find locks with marks on them (search for '//')
where dcache_lock was being used and I'm not sure of the correct transformation
now that it no longer exists.

Hi Al,

Could you look over these patches and make sure you're happy with the way we've
done things?

Hi Ian,

Could you have a poke at the autofs4 changes, make sure things still work?

---
The attached patches implement VFS support for automounting - which several
filesystems, including autofs, can use to resolve problems with their current
implementations.  This means that these filesystems no longer have to abuse
lookup(), follow_link() and d_revalidate() to achieve the desired effects.

Probably the most significant advantage for autofs is a long standing potential
deadlock can be removed which otherwise cannot be resolved.


These patches introduce a couple of new dentry operations for use by
automounters and make AFS, NFS, CIFS and autofs4 use them.

There are two dentry operations provided:

 (1) struct vfsmount *(*d_automount)(struct path *path);

     This is used by follow_automount() in fs/namei.c to ask the filesystem
     that owns the dentry at the current path point to mount something on
     @path.

     It is called if either the inode belonging to the given dentry is flagged
     S_AUTOMOUNT or the dentry is flagged DCACHE_NEED_AUTOMOUNT, and if the
     dentry has nothing mounted on it in the current namespace when someone
     attempts to use that dentry.

     No locks will be held when this is called.

     d_op->d_automount() may return one of:

     (a) The vfsmount mounted upon that dentry, in which case pathwalk will
     	 move to the root dentry of that vfsmount.

     (b) NULL if something was already mounted there, in which case pathwalk
     	 will loop around and recheck the mountings.

     (c) -EISDIR, in which case pathwalk will stop at this point and attempt to
     	 use that dentry as the object of interest.  If the current dentry is
     	 not terminal within the path, -EREMOTE will be returned.

     (d) An error value, to be returned immediately.

     Automount transits are counted as symlinks to prevent circular references
     from being a problem.  If one is detected, -ELOOP will be returned.

     If stat() is given AT_NO_AUTOMOUNT then d_op->d_automount() will not be
     invoked on a terminal dentry; instead that dentry will be returned by
     pathwalk.

     follow_automount() also does not invoke d_op->d_automount() if the caller
     gave AT_SYMLINK_NOFOLLOW to stat(), but rather returns the base dentry.


 (2) int (*d_manage)(struct path *path, bool mounting_here);

     This is called by managed_dentry() or follow_down() in fs/namei.c to
     indicate to a filesystem that pathwalk is about to step off of the current
     path point and walk to another point in the path.

     This is called if DCACHE_MANAGE_TRANSIT is set on a dentry.

     This can then be used by autofs to stop non-daemon processes from walking
     until it has finished constructing or expiring the tree behind the dentry.
     It could also be used to prevent undesirables from mounting on this
     dentry.

     @mounting_here is true if called from follow_down() from mount, in which
     case namespace_sem is held exclusively by the caller of follow_down().
     Otherwise, no locks are held.

     d_op->d_manage() may return one of:

     (a) 0 to continue the pathwalk as normal.

     (b) -EISDIR to prevent managed_dentry() from crossing to a mounted
     	 filesystem or calling d_op->d_automount(), in which case the dentry
     	 will be treated as an ordinary directory.

     (c) An error to abort the pathwalk completely.


To make this work for autofs a couple of additional dentry d_flags have been
defined: DCACHE_MANAGE_TRANSIT and DCACHE_NEED_AUTOMOUNT.  This allows
managed_dentry() to test all three conditions with minimumal overhead if none
of them are true.

For other filesystems, setting S_AUTOMOUNT is sufficient.  This is noted by
d_set_d_op() which will set DCACHE_NEED_AUTOMOUNT automatically if it is seen.
Checking S_AUTOMOUNT doesn't work for autofs, however, since the dentry might
not have an inode, hence why a dentry flag also.

S_AUTOMOUNT and d_automount() are introduced in patch 1; d_manage(),
DCACHE_MANAGE_TRANSIT and DCACHE_NEED_AUTOMOUNT are introduced in patch 7.


=======
UPDATES
=======

 [ver #3]
   - Update to take account of Nick Piggin's RCU-based pathwalk changes.

 [ver #2]
   - Fixed a EXDEV in patch 6 to be EISDIR.  We were previously using EXDEV to
     indicate we wanted to handle a directory as a directory and not to process
     it as a mountpoint.
   - Move some autofs v4 pseudo mount bits into the v4 pseudo direct mount
     patch [patch 16].
   - Move a comment fix to the autofs d_automount() patch [patch 10 -> 9].
   - Adjust the patch titles of the last three autofs patches.

David
---

David Howells (9):
      Remove a further kludge from __do_follow_link()
      Make follow_down() handle d_manage()
      Add more dentry flags for special function directories
      Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
      Remove the automount through follow_link() kludge code from pathwalk
      CIFS: Use d_automount() rather than abusing follow_link()
      NFS: Use d_automount() rather than abusing follow_link()
      AFS: Use d_automount() rather than abusing follow_link()
      Add a dentry op to handle automounting rather than abusing follow_link()

Ian Kent (9):
      autofs4: Bump version
      autofs4: Add v4 pseudo direct mount support
      autofs4: Fix wait validation
      autofs4: Clean up autofs4_free_ino()
      autofs4: Clean up dentry operations
      autofs4: Clean up inode operations
      autofs4: Remove unused code
      autofs4: Add d_manage() dentry operation
      autofs4: Add d_automount() dentry operation


 Documentation/filesystems/Locking |    2 
 Documentation/filesystems/vfs.txt |   22 +
 drivers/staging/autofs/dirhash.c  |    5 
 fs/afs/dir.c                      |    1 
 fs/afs/inode.c                    |    3 
 fs/afs/internal.h                 |    1 
 fs/afs/mntpt.c                    |   47 +--
 fs/autofs4/autofs_i.h             |  100 ++++-
 fs/autofs4/dev-ioctl.c            |    2 
 fs/autofs4/expire.c               |   51 ++-
 fs/autofs4/inode.c                |   28 --
 fs/autofs4/root.c                 |  681 ++++++++++++++++---------------------
 fs/autofs4/waitq.c                |   17 +
 fs/cifs/cifs_dfs_ref.c            |  134 ++++---
 fs/cifs/cifsfs.h                  |    6 
 fs/cifs/dir.c                     |    2 
 fs/cifs/inode.c                   |    8 
 fs/dcache.c                       |    2 
 fs/namei.c                        |  303 +++++++++++++---
 fs/namespace.c                    |   14 -
 fs/nfs/dir.c                      |    4 
 fs/nfs/inode.c                    |    4 
 fs/nfs/internal.h                 |    1 
 fs/nfs/namespace.c                |   87 ++---
 fs/nfsd/vfs.c                     |    5 
 fs/stat.c                         |    4 
 include/linux/auto_fs4.h          |    2 
 include/linux/dcache.h            |   16 +
 include/linux/fcntl.h             |    1 
 include/linux/fs.h                |    2 
 include/linux/namei.h             |    5 
 include/linux/nfs_fs.h            |    1 
 32 files changed, 867 insertions(+), 694 deletions(-)


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

* [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-13  3:53   ` Nick Piggin
  2011-01-13 12:20   ` David Howells
  2011-01-11 17:48 ` [PATCH 02/18] AFS: Use d_automount() " David Howells
                   ` (18 subsequent siblings)
  19 siblings, 2 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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.

A new pathwalk subroutine, follow_automount() is added to handle mountpoints.
It will return -EREMOTE if the S_AUTOMOUNT was set, but no d_automount() op was
supplied, -ELOOP if we've encountered too many symlinks or mountpoints, -EISDIR
if the walk point should be used without mounting and 0 if successful.  path
will be updated if an automount took place to point to the mounted filesystem.

I've only changed __follow_mount() to handle call follow_automount(), but it
might be necessary to change follow_mount() too.  The latter is only called
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>
Acked-by: Ian Kent <raven@themaw.net>
---

 Documentation/filesystems/Locking |    2 +
 Documentation/filesystems/vfs.txt |   13 ++++
 fs/namei.c                        |  120 ++++++++++++++++++++++++++++---------
 include/linux/dcache.h            |    4 +
 include/linux/fs.h                |    2 +
 5 files changed, 111 insertions(+), 30 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 977d891..7ebe42d 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -19,6 +19,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:
 		rename_lock	->d_lock	may block	rcu-walk
@@ -29,6 +30,7 @@ d_delete:	no		yes		no		no
 d_release:	no		no		yes		no
 d_iput:		no		no		yes		no
 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 fbb324e..bb8d277 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -864,6 +864,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
@@ -930,6 +931,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 24ece10..159da29 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -877,38 +877,84 @@ int follow_up(struct path *path)
 }
 
 /*
- * serialization is taken care of in namespace.c
+ * Perform an automount
+ * - return -EISDIR to tell __follow_mount() to stop and return the path we
+ *   were called with.
  */
-static void __follow_mount_rcu(struct nameidata *nd, struct path *path,
-				struct inode **inode)
+static int follow_automount(struct path *path, unsigned flags,
+			    bool *need_mntput)
 {
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted;
-		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
-		if (!mounted)
-			return;
-		path->mnt = mounted;
-		path->dentry = mounted->mnt_root;
-		nd->seq = read_seqcount_begin(&path->dentry->d_seq);
-		*inode = path->dentry->d_inode;
+	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 -EISDIR;
+
+	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 (*need_mntput)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	*need_mntput = true;
+	return 0;
 }
 
-static int __follow_mount(struct path *path)
+/*
+ * serialization is taken care of in namespace.c
+ */
+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;
+	bool need_mntput = false;
+	int ret;
+
+	for (;;) {
+		while (d_mountpoint(path->dentry)) {
+			mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			if (need_mntput)
+				mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			need_mntput = true;
+		}
+		if (!d_automount_point(path->dentry))
 			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
-		res = 1;
+		ret = follow_automount(path, flags, &need_mntput);
+		if (ret < 0)
+			return ret == -EISDIR ? 0 : ret;
 	}
-	return res;
+	return 0;
 }
 
 static void follow_mount(struct path *path)
@@ -939,6 +985,21 @@ int follow_down(struct path *path)
 	return 0;
 }
 
+static void __follow_mount_rcu(struct nameidata *nd, struct path *path,
+				struct inode **inode)
+{
+	while (d_mountpoint(path->dentry)) {
+		struct vfsmount *mounted;
+		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
+		if (!mounted)
+			return;
+		path->mnt = mounted;
+		path->dentry = mounted->mnt_root;
+		nd->seq = read_seqcount_begin(&path->dentry->d_seq);
+		*inode = path->dentry->d_inode;
+	}
+}
+
 static int follow_dotdot_rcu(struct nameidata *nd)
 {
 	struct inode *inode = nd->inode;
@@ -1038,6 +1099,7 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent = nd->path.dentry;
 	struct inode *dir;
+
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
@@ -1083,7 +1145,7 @@ found:
 done:
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount(path);
+		__follow_mount(path, nd->flags);
 		*inode = path->dentry->d_inode;
 	}
 	return 0;
@@ -2178,11 +2240,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 59fcd24..444614b 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -167,6 +167,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 *);
 } ____cacheline_aligned;
 
 /*
@@ -404,6 +405,9 @@ static inline int d_mountpoint(struct dentry *dentry)
 	return dentry->d_flags & DCACHE_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 f84d992..5416e1a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -242,6 +242,7 @@ struct inodes_stat_t {
 #define S_SWAPFILE	256	/* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	512	/* Inode is fs-internal */
 #define S_IMA		1024	/* Inode has an associated IMA struct */
+#define S_AUTOMOUNT	2048	/* Automount/referral quasi-directory */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -277,6 +278,7 @@ struct inodes_stat_t {
 #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
 #define IS_IMA(inode)		((inode)->i_flags & S_IMA)
+#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] 30+ messages in thread

* [PATCH 02/18] AFS: Use d_automount() rather than abusing follow_link()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
  2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 03/18] NFS: " David Howells
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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/inode.c    |    3 ++-
 fs/afs/internal.h |    1 +
 fs/afs/mntpt.c    |   47 +++++++++++++++--------------------------------
 4 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 34a3263..70f07e3 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -66,6 +66,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/inode.c b/fs/afs/inode.c
index 0747339..db66c52 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -184,7 +184,8 @@ struct inode *afs_iget_autocell(struct inode *dir, const char *dev_name,
 	inode->i_generation	= 0;
 
 	set_bit(AFS_VNODE_PSEUDODIR, &vnode->flags);
-	inode->i_flags |= S_NOATIME;
+	set_bit(AFS_VNODE_MOUNTPOINT, &vnode->flags);
+	inode->i_flags |= S_AUTOMOUNT | S_NOATIME;
 	unlock_new_inode(inode);
 	_leave(" = %p", inode);
 	return inode;
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 6d4bc1c..c59bb7c 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -590,6 +590,7 @@ extern const struct inode_operations afs_mntpt_inode_operations;
 extern const struct inode_operations afs_autocell_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 6153417..0f7dd7a 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 = {
@@ -34,13 +33,11 @@ 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,
 };
 
 const struct inode_operations afs_autocell_inode_operations = {
-	.follow_link	= afs_mntpt_follow_link,
 	.getattr	= afs_getattr,
 };
 
@@ -88,6 +85,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);
 	}
 
@@ -238,52 +236,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] 30+ messages in thread

* [PATCH 03/18] NFS: Use d_automount() rather than abusing follow_link()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
  2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
  2011-01-11 17:48 ` [PATCH 02/18] AFS: Use d_automount() " David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 04/18] CIFS: " David Howells
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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>
Acked-by: Ian Kent <raven@themaw.net>
---

 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 d33da53..959927c 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -970,7 +970,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 */
@@ -1173,6 +1173,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)
@@ -1248,6 +1249,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 017daa3..6ace134 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -300,7 +300,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;
@@ -1208,7 +1208,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 e6356b7..66c3dc4 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -245,6 +245,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 74aaf39..f3fbb1b 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -97,9 +97,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
@@ -109,87 +108,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 0779bb8..6023efa 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -215,7 +215,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] 30+ messages in thread

* [PATCH 04/18] CIFS: Use d_automount() rather than abusing follow_link()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (2 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 03/18] NFS: " David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 05/18] Remove the automount through follow_link() kludge code from pathwalk David Howells
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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

[NOTE: THIS IS UNTESTED!]

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

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

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index c68a056..ddd0b3e 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -255,35 +255,6 @@ static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb,
 
 }
 
-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);
@@ -293,45 +264,43 @@ 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;
 	struct tcon_link *tlink;
 
 	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);
-
 	/*
 	 * The MSDFS spec states that paths in DFS referral requests and
 	 * responses must be prefixed by a single '\' character instead of
 	 * 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;
 
-	cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
+	cifs_sb = CIFS_SB(mntpt->d_inode->i_sb);
 	tlink = cifs_sb_tlink(cifs_sb);
+	mnt = ERR_PTR(-EINVAL);
 	if (IS_ERR(tlink)) {
-		rc = PTR_ERR(tlink);
-		goto out_err;
+		mnt = ERR_CAST(tlink);
+		goto free_full_path;
 	}
 	ses = tlink_tcon(tlink)->ses;
 
@@ -341,46 +310,77 @@ cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
 
 	cifs_put_tlink(tlink);
 
+	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(cifs_sb,
 				full_path, 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);
+free_full_path:
 	kfree(full_path);
+free_xid:
+	FreeXid(xid);
 	cFYI(1, "leaving %s" , __func__);
-	return ERR_PTR(rc);
-out_err:
-	path_put(&nd->path);
-	goto out;
+	return mnt;
+}
+
+/*
+ * 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 897b2b2..851030f 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -93,6 +93,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 2e77382..e50ee06 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -698,6 +698,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 */
 };
 
@@ -734,4 +735,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 0c7e369..e0b1686 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -32,7 +32,7 @@
 #include "fscache.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);
 
@@ -60,7 +60,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 */
@@ -167,7 +167,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] 30+ messages in thread

* [PATCH 05/18] Remove the automount through follow_link() kludge code from pathwalk
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (3 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 04/18] CIFS: " David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 06/18] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount David Howells
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Ian Kent <raven@themaw.net>
---

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

diff --git a/fs/namei.c b/fs/namei.c
index 159da29..dfafb97 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1193,17 +1193,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.
@@ -1341,7 +1330,8 @@ last_component:
 		err = do_lookup(nd, &this, &next, &inode);
 		if (err)
 			break;
-		if (follow_on_final(inode, lookup_flags)) {
+		if (inode && unlikely(inode->i_op->follow_link) &&
+		    (lookup_flags & LOOKUP_FOLLOW)) {
 			if (nameidata_dentry_drop_rcu_maybe(nd, next.dentry))
 				return -ECHILD;
 			BUG_ON(inode != next.dentry->d_inode);
@@ -2390,8 +2380,7 @@ reval:
 		struct path holder;
 		void *cookie;
 		error = -ELOOP;
-		/* S_ISDIR part is a temporary automount kludge */
-		if (!(nd.flags & LOOKUP_FOLLOW) && !S_ISDIR(nd.inode->i_mode))
+		if (!(nd.flags & LOOKUP_FOLLOW))
 			goto exit_dput;
 		if (count++ == 32)
 			goto exit_dput;


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

* [PATCH 06/18] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (4 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 05/18] Remove the automount through follow_link() kludge code from pathwalk David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 07/18] Add more dentry flags for special function directories David Howells
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

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>
Acked-by: Ian Kent <raven@themaw.net>
---

 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 dfafb97..7622825 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -889,6 +889,12 @@ static int follow_automount(struct path *path, unsigned flags,
 	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 12e90e2..d5c61cf 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -75,11 +75,13 @@ int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 	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 18d06ad..a12d0d7 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -45,6 +45,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
  *  - ending slashes ok even for nonexistent files
  *  - internal "there are more path components" flag
  *  - dentry cache is untrusted; force a real lookup
+ *  - suppress terminal automount
  */
 #define LOOKUP_FOLLOW		0x0001
 #define LOOKUP_DIRECTORY	0x0002
@@ -53,6 +54,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_PARENT		0x0010
 #define LOOKUP_REVAL		0x0020
 #define LOOKUP_RCU		0x0040
+#define LOOKUP_NO_AUTOMOUNT	0x0080
 /*
  * Intent data
  */


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

* [PATCH 07/18] Add more dentry flags for special function directories
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (5 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 06/18] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 08/18] Make follow_down() handle d_manage() David Howells
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

Add more flags to the d_flags in struct dentry for special function directories
such as mountpoints and autofs substructures.  The relevant flags are:

 (*) DCACHE_MOUNTED.

     (Already exists).  Indicates that this dentry has things mounted upon it.

 (*) DCACHE_NEED_AUTOMOUNT.

     This is a reflection of the S_AUTOMOUNT inode flag.  This is reflected by
     d_set_d_op().  follow_automount() is now keyed off of this rather than
     being keyed off S_AUTOMOUNT directly.  Possibly S_AUTOMOUNT should shift
     to the dentry entirely.

     This may also be tweaked live (such as by the autofs4 filesystem) to alter
     the effect.

 (*) DCACHE_MANAGE_TRANSIT.

     This is an indicator that the filesystem that owns the dentry wants to
     manage processes transiting away from that dentry.  If this is set on a
     dentry, then a new dentry op:

		int (*d_manage)(struct path *);

     is invoked.  This is allowed to sleep and is allowed to return an error.

     This allows autofs to hold non-Oz-mode processes here without any
     filesystem locks being held.

__follow_mount() is replaced by managed_dentry() which now handles transit to a
mountpoint's root dentry, automount points and points that the filesystem wants
to manage.


==========================
WHAT THIS MEANS FOR AUTOFS
==========================

autofs currently uses the lookup() inode op and the d_revalidate() dentry op to
trigger the automounting of indirect mounts, and both of these can be called
with i_mutex held.

autofs knows that the i_mutex will be held by the caller in lookup(), and so
can drop it before invoking the daemon - but this isn't so for d_revalidate(),
since the lock is only held on _some_ of the code paths that call it.  This
means that autofs can't risk dropping i_mutex from its d_revalidate() function
before it calls the daemon.

The bug could manifest itself as, for example, a process that's trying to
validate an automount dentry that gets made to wait because that dentry is
expired and needs cleaning up:

	mkdir         S ffffffff8014e05a     0 32580  24956
	Call Trace:
	 [<ffffffff885371fd>] :autofs4:autofs4_wait+0x674/0x897
	 [<ffffffff80127f7d>] avc_has_perm+0x46/0x58
	 [<ffffffff8009fdcf>] autoremove_wake_function+0x0/0x2e
	 [<ffffffff88537be6>] :autofs4:autofs4_expire_wait+0x41/0x6b
	 [<ffffffff88535cfc>] :autofs4:autofs4_revalidate+0x91/0x149
	 [<ffffffff80036d96>] __lookup_hash+0xa0/0x12f
	 [<ffffffff80057a2f>] lookup_create+0x46/0x80
	 [<ffffffff800e6e31>] sys_mkdirat+0x56/0xe4

versus the automount daemon which wants to remove that dentry, but can't
because the normal process is holding the i_mutex lock:

	automount     D ffffffff8014e05a     0 32581      1              32561
	Call Trace:
	 [<ffffffff80063c3f>] __mutex_lock_slowpath+0x60/0x9b
	 [<ffffffff8000ccf1>] do_path_lookup+0x2ca/0x2f1
	 [<ffffffff80063c89>] .text.lock.mutex+0xf/0x14
	 [<ffffffff800e6d55>] do_rmdir+0x77/0xde
	 [<ffffffff8005d229>] tracesys+0x71/0xe0
	 [<ffffffff8005d28d>] tracesys+0xd5/0xe0

which means that the system is deadlocked.

This patch allows autofs to hold up normal processes whilst the daemon goes
ahead and does things to the dentry tree behind the automouter point without
risking a deadlock as no locks are held in d_manage() or d_automount().

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Ian Kent <raven@themaw.net>
---

 Documentation/filesystems/vfs.txt |   13 +++
 fs/dcache.c                       |    2 
 fs/namei.c                        |  153 ++++++++++++++++++++++++++-----------
 include/linux/dcache.h            |   13 ++-
 4 files changed, 130 insertions(+), 51 deletions(-)

diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index bb8d277..99f0127 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -865,6 +865,7 @@ struct dentry_operations {
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
 	struct vfsmount *(*d_automount)(struct path *);
+	int (*d_manage)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -940,8 +941,16 @@ struct dentry_operations {
 	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.
+	This function is only used if DMANAGED_AUTOMOUNT is set on the dentry.
+	This is set by d_add() if S_AUTOMOUNT is set on the inode being added.
+
+  d_manage: called to allow the filesystem to manage the transition from a
+	dentry (optional).  This allows autofs, for example, to hold up clients
+	waiting to explore behind a 'mountpoint', whilst letting the daemon go
+	past and construct the subtree there.
+
+	This function is only used if DMANAGED_TRANSIT is set on the dentry
+	being transited from.
 
 Example :
 
diff --git a/fs/dcache.c b/fs/dcache.c
index 5699d4c..0e0978a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1371,6 +1371,8 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
 		dentry->d_flags |= DCACHE_OP_REVALIDATE;
 	if (op->d_delete)
 		dentry->d_flags |= DCACHE_OP_DELETE;
+	if (op->d_automount && dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
+		dentry->d_flags |= DCACHE_NEED_AUTOMOUNT;
 
 }
 EXPORT_SYMBOL(d_set_d_op);
diff --git a/fs/namei.c b/fs/namei.c
index 7622825..1e31f96 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -878,7 +878,7 @@ int follow_up(struct path *path)
 
 /*
  * Perform an automount
- * - return -EISDIR to tell __follow_mount() to stop and return the path we
+ * - return -EISDIR to tell managed_dentry() to stop and return the path we
  *   were called with.
  */
 static int follow_automount(struct path *path, unsigned flags,
@@ -893,7 +893,7 @@ static int follow_automount(struct path *path, unsigned flags,
 	 * 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 */
+		return -EISDIR; /* 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
@@ -913,8 +913,20 @@ static int follow_automount(struct path *path, unsigned flags,
 		return -ELOOP;
 
 	mnt = path->dentry->d_op->d_automount(path);
-	if (IS_ERR(mnt))
+	if (IS_ERR(mnt)) {
+		/*
+		 * The filesystem is allowed to return -EISDIR here to indicate
+		 * it doesn'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) == -EISDIR && (flags & LOOKUP_CONTINUE))
+			return -EREMOTE;
 		return PTR_ERR(mnt);
+	}
 	if (!mnt) /* mount collision */
 		return 0;
 
@@ -934,46 +946,66 @@ static int follow_automount(struct path *path, unsigned flags,
 }
 
 /*
- * serialization is taken care of in namespace.c
+ * Handle a dentry that is managed in some way.
+ * - Flagged for transit management (autofs)
+ * - Flagged as mountpoint
+ * - Flagged as automount point
+ *
+ * This may only be called in refwalk mode.
  */
-static int __follow_mount(struct path *path, unsigned flags)
+static int managed_dentry(struct path *path, unsigned flags)
 {
-	struct vfsmount *mounted;
+	unsigned managed;
 	bool need_mntput = false;
 	int ret;
 
-	for (;;) {
-		while (d_mountpoint(path->dentry)) {
-			mounted = lookup_mnt(path);
-			if (!mounted)
-				break;
-			dput(path->dentry);
-			if (need_mntput)
-				mntput(path->mnt);
-			path->mnt = mounted;
-			path->dentry = dget(mounted->mnt_root);
-			need_mntput = true;
+	/* Given that we're not holding a lock here, we retain the value in a
+	 * local variable for each dentry as we look at it so that we don't see
+	 * the components of that value change under us */
+	while (managed = ACCESS_ONCE(path->dentry->d_flags),
+	       managed &= DCACHE_MANAGED_DENTRY,
+	       unlikely(managed != 0)) {
+		/* Allow the filesystem to manage the transit without i_mutex
+		 * being held. */
+		if (managed & DCACHE_MANAGE_TRANSIT) {
+			BUG_ON(!path->dentry->d_op);
+			BUG_ON(!path->dentry->d_op->d_manage);
+			ret = path->dentry->d_op->d_manage(path);
+			if (ret < 0)
+				return ret;
 		}
-		if (!d_automount_point(path->dentry))
-			break;
-		ret = follow_automount(path, flags, &need_mntput);
-		if (ret < 0)
-			return ret == -EISDIR ? 0 : ret;
-	}
-	return 0;
-}
 
-static void follow_mount(struct path *path)
-{
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
-			break;
-		dput(path->dentry);
-		mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		/* Transit to a mounted filesystem. */
+		if (managed & DCACHE_MOUNTED) {
+			struct vfsmount *mounted = lookup_mnt(path);
+			if (mounted) {
+				dput(path->dentry);
+				if (need_mntput)
+					mntput(path->mnt);
+				path->mnt = mounted;
+				path->dentry = dget(mounted->mnt_root);
+				need_mntput = true;
+				continue;
+			}
+
+			/* Something is mounted on this dentry in another
+			 * namespace and/or whatever was mounted there in this
+			 * namespace got unmounted before we managed to get the
+			 * vfsmount_lock */
+		}
+
+		/* Handle an automount point */
+		if (managed & DCACHE_NEED_AUTOMOUNT) {
+			ret = follow_automount(path, flags, &need_mntput);
+			if (ret < 0)
+				return ret == -EISDIR ? 0 : ret;
+			continue;
+		}
+
+		/* We didn't change the current path point */
+		break;
 	}
+	return 0;
 }
 
 int follow_down(struct path *path)
@@ -991,19 +1023,31 @@ int follow_down(struct path *path)
 	return 0;
 }
 
-static void __follow_mount_rcu(struct nameidata *nd, struct path *path,
-				struct inode **inode)
+/*
+ * Skip to top of mountpoint pile in rcuwalk mode.  If requested we abort the
+ * walk when we hit a dentry that has DCACHE_MANAGE_TRANSIT flagged (we don't
+ * want to take note of it when walking "..").
+ */
+static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
+			       struct inode **inode,
+			       bool abort_on_managed_dentry)
 {
+	unsigned abort_mask =
+		abort_on_managed_dentry ? DCACHE_MANAGE_TRANSIT : 0;
+
 	while (d_mountpoint(path->dentry)) {
 		struct vfsmount *mounted;
+		if (path->dentry->d_flags & abort_mask)
+			return true;
 		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
 		if (!mounted)
-			return;
+			break;
 		path->mnt = mounted;
 		path->dentry = mounted->mnt_root;
 		nd->seq = read_seqcount_begin(&path->dentry->d_seq);
 		*inode = path->dentry->d_inode;
 	}
+	return false;
 }
 
 static int follow_dotdot_rcu(struct nameidata *nd)
@@ -1012,7 +1056,7 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 
 	set_root_rcu(nd);
 
-	while(1) {
+	while (1) {
 		if (nd->path.dentry == nd->root.dentry &&
 		    nd->path.mnt == nd->root.mnt) {
 			break;
@@ -1035,12 +1079,28 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 		nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
 		inode = nd->path.dentry->d_inode;
 	}
-	__follow_mount_rcu(nd, &nd->path, &inode);
+	__follow_mount_rcu(nd, &nd->path, &inode, false);
 	nd->inode = inode;
 
 	return 0;
 }
 
+/*
+ * Skip to top of mountpoint pile in refwalk mode for follow_dotdot()
+ */
+static void follow_mount(struct path *path)
+{
+	while (d_mountpoint(path->dentry)) {
+		struct vfsmount *mounted = lookup_mnt(path);
+		if (!mounted)
+			break;
+		dput(path->dentry);
+		mntput(path->mnt);
+		path->mnt = mounted;
+		path->dentry = dget(mounted->mnt_root);
+	}
+}
+
 static void follow_dotdot(struct nameidata *nd)
 {
 	set_root(nd);
@@ -1105,13 +1165,14 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent = nd->path.dentry;
 	struct inode *dir;
+	int err;
 
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
 	 */
 	if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
-		int err = parent->d_op->d_hash(parent, nd->inode, name);
+		err = parent->d_op->d_hash(parent, nd->inode, name);
 		if (err < 0)
 			return err;
 	}
@@ -1140,7 +1201,9 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 			goto need_revalidate;
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount_rcu(nd, path, inode);
+		if (unlikely(__follow_mount_rcu(nd, path, inode, true)) &&
+		    nameidata_drop_rcu(nd))
+			return -ECHILD;
 	} else {
 		dentry = __d_lookup(parent, name);
 		if (!dentry)
@@ -1151,7 +1214,9 @@ found:
 done:
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount(path, nd->flags);
+		err = managed_dentry(path, nd->flags);
+		if (unlikely(err < 0))
+			return err;
 		*inode = path->dentry->d_inode;
 	}
 	return 0;
@@ -2236,7 +2301,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	error = __follow_mount(path, nd->flags);
+	error = managed_dentry(path, nd->flags);
 	if (error < 0)
 		goto exit_dput;
 
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 444614b..c6a4821 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -168,6 +168,7 @@ struct dentry_operations {
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
 	struct vfsmount *(*d_automount)(struct path *);
+	int (*d_manage)(struct path *);
 } ____cacheline_aligned;
 
 /*
@@ -206,13 +207,18 @@ struct dentry_operations {
 
 #define DCACHE_CANT_MOUNT	0x0100
 #define DCACHE_GENOCIDE		0x0200
-#define DCACHE_MOUNTED		0x0400	/* is a mountpoint */
 
 #define DCACHE_OP_HASH		0x1000
 #define DCACHE_OP_COMPARE	0x2000
 #define DCACHE_OP_REVALIDATE	0x4000
 #define DCACHE_OP_DELETE	0x8000
 
+#define DCACHE_MOUNTED		0x10000	/* is a mountpoint */
+#define DCACHE_NEED_AUTOMOUNT	0x20000	/* handle automount on this dir */
+#define DCACHE_MANAGE_TRANSIT	0x40000	/* manage transit from this dirent */
+#define DCACHE_MANAGED_DENTRY \
+	(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)
+
 extern seqlock_t rename_lock;
 
 static inline int dname_external(struct dentry *dentry)
@@ -400,14 +406,11 @@ static inline void dont_mount(struct dentry *dentry)
 
 extern void dput(struct dentry *);
 
-static inline int d_mountpoint(struct dentry *dentry)
+static inline bool d_mountpoint(struct dentry *dentry)
 {
 	return dentry->d_flags & DCACHE_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);
 


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

* [PATCH 08/18] Make follow_down() handle d_manage()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (6 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 07/18] Add more dentry flags for special function directories David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:48 ` [PATCH 09/18] autofs4: Add d_automount() dentry operation David Howells
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

The previous patch (that adds d_manage()) offers autofs the opportunity to
block processes whilst it is rearranging its dentry tree, but only covers cases
where managed_dentry() is called.  Some places call follow_down(), which would
allow processes to bypass autofs's attempts to block them.

Make follow_down() handle managed dentries.  Do this by renaming follow_down()
to follow_down_one() and instituting a new follow_down().  follow_down_one() is
then only used where a call to d_manage() is not needed.

follow_down() then incorporates the loop from its remaining callers to follow
down through all mounted filesystems at that point.  Before each mountpoint is
transited and if requested by the filesystem, d_manage() is called to hold or
reject that transit.  The callers of follow_down() must then handle a possible
error condition.

follow_down() is given a parameter to say whether someone is trying to mount on
that point (and holding namespace_sem).  This is now passed on to d_manage().
The filesystem may reject this request by returning an error from d_manage().

Furthermore, d_manage() may end follow_down() processing early by returning
-EISDIR to indicate it wants the dentry to be dealt with as an ordinary
directory, not a mountpoint.  This permits autofs to let its daemon see the
underlying dentry.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Ian Kent <raven@themaw.net>
---

 drivers/staging/autofs/dirhash.c |    5 +--
 fs/autofs4/autofs_i.h            |   13 ---------
 fs/autofs4/dev-ioctl.c           |    2 +
 fs/autofs4/expire.c              |    2 +
 fs/autofs4/root.c                |   11 +++----
 fs/namei.c                       |   58 ++++++++++++++++++++++++++++++++++++--
 fs/namespace.c                   |   14 +++++----
 fs/nfsd/vfs.c                    |    5 ++-
 include/linux/dcache.h           |    7 ++++-
 include/linux/namei.h            |    3 +-
 10 files changed, 83 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/autofs/dirhash.c b/drivers/staging/autofs/dirhash.c
index d3f42c8..a08bd73 100644
--- a/drivers/staging/autofs/dirhash.c
+++ b/drivers/staging/autofs/dirhash.c
@@ -88,14 +88,13 @@ struct autofs_dir_ent *autofs_expire(struct super_block *sb,
 		}
 		path.mnt = mnt;
 		path_get(&path);
-		if (!follow_down(&path)) {
+		if (!follow_down_one(&path)) {
 			path_put(&path);
 			DPRINTK(("autofs: not expirable\
 			(not a mounted directory): %s\n", ent->name));
 			continue;
 		}
-		while (d_mountpoint(path.dentry) && follow_down(&path))
-			;
+		follow_down(&path, false);  // TODO: need to check error
 		umount_ok = may_umount(path.mnt);
 		path_put(&path);
 
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 0fffe1c..eb67953 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -229,19 +229,6 @@ int autofs4_wait(struct autofs_sb_info *,struct dentry *, enum autofs_notify);
 int autofs4_wait_release(struct autofs_sb_info *,autofs_wqt_t,int);
 void autofs4_catatonic_mode(struct autofs_sb_info *);
 
-static inline int autofs4_follow_mount(struct path *path)
-{
-	int res = 0;
-
-	while (d_mountpoint(path->dentry)) {
-		int followed = follow_down(path);
-		if (!followed)
-			break;
-		res = 1;
-	}
-	return res;
-}
-
 static inline u32 autofs4_get_dev(struct autofs_sb_info *sbi)
 {
 	return new_encode_dev(sbi->sb->s_dev);
diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index eff9a41..1442da4 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -551,7 +551,7 @@ static int autofs_dev_ioctl_ismountpoint(struct file *fp,
 
 		err = have_submounts(path.dentry);
 
-		if (follow_down(&path))
+		if (follow_down_one(&path))
 			magic = path.mnt->mnt_sb->s_magic;
 	}
 
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index cc1d013..6a930b9 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -56,7 +56,7 @@ static int autofs4_mount_busy(struct vfsmount *mnt, struct dentry *dentry)
 
 	path_get(&path);
 
-	if (!follow_down(&path))
+	if (!follow_down_one(&path))
 		goto done;
 
 	if (is_autofs4_dentry(path.dentry)) {
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 651e4ef..2022563 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -234,7 +234,7 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
 		nd->flags);
 	/*
 	 * For an expire of a covered direct or offset mount we need
-	 * to break out of follow_down() at the autofs mount trigger
+	 * to break out of follow_down_one() at the autofs mount trigger
 	 * (d_mounted--), so we can see the expiring flag, and manage
 	 * the blocking and following here until the expire is completed.
 	 */
@@ -243,7 +243,7 @@ static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
 		if (ino->flags & AUTOFS_INF_EXPIRING) {
 			spin_unlock(&sbi->fs_lock);
 			/* Follow down to our covering mount. */
-			if (!follow_down(&nd->path))
+			if (!follow_down_one(&nd->path))
 				goto done;
 			goto follow;
 		}
@@ -292,11 +292,10 @@ follow:
 	 * multi-mount with no root offset so we don't need
 	 * to follow it.
 	 */
-	if (d_mountpoint(dentry)) {
-		if (!autofs4_follow_mount(&nd->path)) {
-			status = -ENOENT;
+	if (d_managed(dentry)) {
+		status = follow_down(&nd->path, false);
+		if (status < 0)
 			goto out_error;
-		}
 	}
 
 done:
diff --git a/fs/namei.c b/fs/namei.c
index 1e31f96..e2c5db2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -970,9 +970,9 @@ static int managed_dentry(struct path *path, unsigned flags)
 		if (managed & DCACHE_MANAGE_TRANSIT) {
 			BUG_ON(!path->dentry->d_op);
 			BUG_ON(!path->dentry->d_op->d_manage);
-			ret = path->dentry->d_op->d_manage(path);
+			ret = path->dentry->d_op->d_manage(path, false);
 			if (ret < 0)
-				return ret;
+				return ret == -EISDIR ? 0 : ret;
 		}
 
 		/* Transit to a mounted filesystem. */
@@ -1008,7 +1008,7 @@ static int managed_dentry(struct path *path, unsigned flags)
 	return 0;
 }
 
-int follow_down(struct path *path)
+int follow_down_one(struct path *path)
 {
 	struct vfsmount *mounted;
 
@@ -1086,6 +1086,57 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 }
 
 /*
+ * Follow down to the covering mount currently visible to userspace.  At each
+ * point, the filesystem owning that dentry may be queried as to whether the
+ * caller is permitted to proceed or not.
+ *
+ * Care must be taken as namespace_sem may be held (indicated by mounting_here
+ * being true).
+ */
+int follow_down(struct path *path, bool mounting_here)
+{
+	unsigned managed;
+	int ret;
+
+	while (managed = ACCESS_ONCE(path->dentry->d_flags),
+	       unlikely(managed & DCACHE_MANAGED_DENTRY)) {
+		/* Allow the filesystem to manage the transit without i_mutex
+		 * being held.
+		 *
+		 * We indicate to the filesystem if someone is trying to mount
+		 * something here.  This gives autofs the chance to deny anyone
+		 * other than its daemon the right to mount on its
+		 * superstructure.
+		 *
+		 * The filesystem may sleep at this point.
+		 */
+		if (managed & DCACHE_MANAGE_TRANSIT) {
+			BUG_ON(!path->dentry->d_op);
+			BUG_ON(!path->dentry->d_op->d_manage);
+			ret = path->dentry->d_op->d_manage(path, mounting_here);
+			if (ret < 0)
+				return ret == -EISDIR ? 0 : ret;
+		}
+
+		/* Transit to a mounted filesystem. */
+		if (managed & DCACHE_MOUNTED) {
+			struct vfsmount *mounted = lookup_mnt(path);
+			if (!mounted)
+				break;
+			dput(path->dentry);
+			mntput(path->mnt);
+			path->mnt = mounted;
+			path->dentry = dget(mounted->mnt_root);
+			continue;
+		}
+
+		/* Don't handle automount points here */
+		break;
+	}
+	return 0;
+}
+
+/*
  * Skip to top of mountpoint pile in refwalk mode for follow_dotdot()
  */
 static void follow_mount(struct path *path)
@@ -3511,6 +3562,7 @@ const struct inode_operations page_symlink_inode_operations = {
 };
 
 EXPORT_SYMBOL(user_path_at);
+EXPORT_SYMBOL(follow_down_one);
 EXPORT_SYMBOL(follow_down);
 EXPORT_SYMBOL(follow_up);
 EXPORT_SYMBOL(get_write_access); /* binfmt_aout */
diff --git a/fs/namespace.c b/fs/namespace.c
index 3ddfd90..d94ccd6 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1844,9 +1844,10 @@ static int do_move_mount(struct path *path, char *old_name)
 		return err;
 
 	down_write(&namespace_sem);
-	while (d_mountpoint(path->dentry) &&
-	       follow_down(path))
-		;
+	err = follow_down(path, true);
+	if (err < 0)
+		goto out;
+
 	err = -EINVAL;
 	if (!check_mnt(path->mnt) || !check_mnt(old_path.mnt))
 		goto out;
@@ -1940,9 +1941,10 @@ int do_add_mount(struct vfsmount *newmnt, struct path *path,
 
 	down_write(&namespace_sem);
 	/* Something was mounted here while we slept */
-	while (d_mountpoint(path->dentry) &&
-	       follow_down(path))
-		;
+	err = follow_down(path, true);
+	if (err < 0)
+		goto unlock;
+
 	err = -EINVAL;
 	if (!(mnt_flags & MNT_SHRINKABLE) && !check_mnt(path->mnt))
 		goto unlock;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 3a35902..4f3f9d9 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -88,8 +88,9 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 			    .dentry = dget(dentry)};
 	int err = 0;
 
-	while (d_mountpoint(path.dentry) && follow_down(&path))
-		;
+	err = follow_down(&path, false);
+	if (err < 0)
+		goto out;
 
 	exp2 = rqst_exp_get_by_name(rqstp, &path);
 	if (IS_ERR(exp2)) {
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index c6a4821..946521d 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -168,7 +168,7 @@ struct dentry_operations {
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
 	struct vfsmount *(*d_automount)(struct path *);
-	int (*d_manage)(struct path *);
+	int (*d_manage)(struct path *, bool);
 } ____cacheline_aligned;
 
 /*
@@ -406,6 +406,11 @@ static inline void dont_mount(struct dentry *dentry)
 
 extern void dput(struct dentry *);
 
+static inline bool d_managed(struct dentry *dentry)
+{
+	return dentry->d_flags & DCACHE_MANAGED_DENTRY;
+}
+
 static inline bool d_mountpoint(struct dentry *dentry)
 {
 	return dentry->d_flags & DCACHE_MOUNTED;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index a12d0d7..f276d4f 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -81,7 +81,8 @@ extern struct file *lookup_instantiate_filp(struct nameidata *nd, struct dentry
 
 extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
 
-extern int follow_down(struct path *);
+extern int follow_down_one(struct path *);
+extern int follow_down(struct path *, bool);
 extern int follow_up(struct path *);
 
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);


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

* [PATCH 09/18] autofs4: Add d_automount() dentry operation
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (7 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 08/18] Make follow_down() handle d_manage() David Howells
@ 2011-01-11 17:48 ` David Howells
  2011-01-11 17:49 ` [PATCH 10/18] autofs4: Add d_manage() " David Howells
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:48 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

Add a function to use the newly defined ->d_automount() dentry operation
for triggering mounts instead of doing the user space callback in ->lookup()
and ->d_revalidate().

Note, to be useful the subsequent patch to add the ->d_manage() dentry
operation is also needed so the discussion of functionality is deferred to
that patch.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/autofs_i.h |   30 ++++++
 fs/autofs4/expire.c   |    6 +
 fs/autofs4/inode.c    |    4 +
 fs/autofs4/root.c     |  261 ++++++++++++++++++++++++++++---------------------
 4 files changed, 189 insertions(+), 112 deletions(-)

diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index eb67953..1ebfe53 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -218,6 +218,36 @@ extern const struct inode_operations autofs4_direct_root_inode_operations;
 extern const struct file_operations autofs4_dir_operations;
 extern const struct file_operations autofs4_root_operations;
 
+/* Operations methods */
+
+struct vfsmount *autofs4_d_automount(struct path *);
+
+/* VFS automount flags management functions */
+
+static inline void __managed_dentry_set_automount(struct dentry *dentry)
+{
+	dentry->d_flags |= DCACHE_NEED_AUTOMOUNT;
+}
+
+static inline void managed_dentry_set_automount(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_set_automount(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
+static inline void __managed_dentry_clear_automount(struct dentry *dentry)
+{
+	dentry->d_flags &= ~DCACHE_NEED_AUTOMOUNT;
+}
+
+static inline void managed_dentry_clear_automount(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_clear_automount(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
 /* Initializing function */
 
 int autofs4_fill_super(struct super_block *, void *, int);
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index 6a930b9..0571ec8 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -300,6 +300,7 @@ struct dentry *autofs4_expire_direct(struct super_block *sb,
 			spin_unlock(&root->d_lock);
 		}
 		ino->flags |= AUTOFS_INF_EXPIRING;
+		managed_dentry_set_automount(root);
 		init_completion(&ino->expire_complete);
 		spin_unlock(&sbi->fs_lock);
 		return root;
@@ -408,6 +409,7 @@ found:
 		expired, (int)expired->d_name.len, expired->d_name.name);
 	ino = autofs4_dentry_ino(expired);
 	ino->flags |= AUTOFS_INF_EXPIRING;
+	managed_dentry_set_automount(expired);
 	init_completion(&ino->expire_complete);
 	spin_unlock(&sbi->fs_lock);
 	spin_lock(&autofs4_lock);
@@ -479,6 +481,8 @@ int autofs4_expire_run(struct super_block *sb,
 	spin_lock(&sbi->fs_lock);
 	ino = autofs4_dentry_ino(dentry);
 	ino->flags &= ~AUTOFS_INF_EXPIRING;
+	if (!d_unhashed(dentry))
+		managed_dentry_clear_automount(dentry);
 	complete_all(&ino->expire_complete);
 	spin_unlock(&sbi->fs_lock);
 
@@ -516,6 +520,8 @@ int autofs4_do_expire_multi(struct super_block *sb, struct vfsmount *mnt,
 			ino->flags &= ~AUTOFS_INF_MOUNTPOINT;
 		}
 		ino->flags &= ~AUTOFS_INF_EXPIRING;
+		if (ret)
+			managed_dentry_clear_automount(dentry);
 		complete_all(&ino->expire_complete);
 		spin_unlock(&sbi->fs_lock);
 		dput(dentry);
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index a7bdb9d..d0aa38c 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -252,6 +252,7 @@ static struct autofs_info *autofs4_mkroot(struct autofs_sb_info *sbi)
 }
 
 static const struct dentry_operations autofs4_sb_dentry_operations = {
+	.d_automount	= autofs4_d_automount,
 	.d_release      = autofs4_dentry_release,
 };
 
@@ -320,6 +321,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
 		goto fail_dput;
 	}
 
+	if (autofs_type_trigger(sbi->type))
+		__managed_dentry_set_automount(root);
+
 	root_inode->i_fop = &autofs4_root_operations;
 	root_inode->i_op = autofs_type_trigger(sbi->type) ?
 			&autofs4_direct_root_inode_operations :
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 2022563..bf8a69d 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -35,7 +35,6 @@ static long autofs4_root_compat_ioctl(struct file *,unsigned int,unsigned long);
 #endif
 static int autofs4_dir_open(struct inode *inode, struct file *file);
 static struct dentry *autofs4_lookup(struct inode *,struct dentry *, struct nameidata *);
-static void *autofs4_follow_link(struct dentry *, struct nameidata *);
 
 #define TRIGGER_FLAGS   (LOOKUP_CONTINUE | LOOKUP_DIRECTORY)
 #define TRIGGER_INTENTS (LOOKUP_OPEN | LOOKUP_CREATE)
@@ -73,7 +72,6 @@ const struct inode_operations autofs4_direct_root_inode_operations = {
 	.unlink		= autofs4_dir_unlink,
 	.mkdir		= autofs4_dir_mkdir,
 	.rmdir		= autofs4_dir_rmdir,
-	.follow_link	= autofs4_follow_link,
 };
 
 const struct inode_operations autofs4_dir_inode_operations = {
@@ -420,13 +418,12 @@ void autofs4_dentry_release(struct dentry *de)
 
 /* For dentries of directories in the root dir */
 static const struct dentry_operations autofs4_root_dentry_operations = {
-	.d_revalidate	= autofs4_revalidate,
 	.d_release	= autofs4_dentry_release,
 };
 
 /* For other dentries */
 static const struct dentry_operations autofs4_dentry_operations = {
-	.d_revalidate	= autofs4_revalidate,
+	.d_automount	= autofs4_d_automount,
 	.d_release	= autofs4_dentry_release,
 };
 
@@ -540,50 +537,176 @@ next:
 	return NULL;
 }
 
+static int autofs4_mount_wait(struct dentry *dentry)
+{
+	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+	struct autofs_info *ino = autofs4_dentry_ino(dentry);
+	int status;
+
+	if (ino->flags & AUTOFS_INF_PENDING) {
+		DPRINTK("waiting for mount name=%.*s",
+			dentry->d_name.len, dentry->d_name.name);
+		status = autofs4_wait(sbi, dentry, NFY_MOUNT);
+		DPRINTK("mount wait done status=%d", status);
+		ino->last_used = jiffies;
+		return status;
+	}
+	return 0;
+}
+
+static int do_expire_wait(struct dentry *dentry)
+{
+	struct dentry *expiring;
+
+	expiring = autofs4_lookup_expiring(dentry);
+	if (!expiring)
+		return autofs4_expire_wait(dentry);
+	else {
+		/*
+		 * If we are racing with expire the request might not
+		 * be quite complete, but the directory has been removed
+		 * so it must have been successful, just wait for it.
+		 */
+		autofs4_expire_wait(expiring);
+		autofs4_del_expiring(expiring);
+		dput(expiring);
+	}
+	return 0;
+}
+
+static struct dentry *autofs4_mountpoint_changed(struct path *path)
+{
+	struct dentry *dentry = path->dentry;
+	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+
+	/*
+	 * If this is an indirect mount the dentry could have gone away
+	 * as a result of an expire and a new one created.
+	 */
+	if (autofs_type_indirect(sbi->type) && d_unhashed(dentry)) {
+		struct dentry *parent = dentry->d_parent;
+		struct dentry *new = d_lookup(parent, &dentry->d_name);
+		if (!new)
+			return NULL;
+		dput(path->dentry);
+		path->dentry = new;
+	}
+	return path->dentry;
+}
+
+struct vfsmount *autofs4_d_automount(struct path *path)
+{
+	struct dentry *dentry = path->dentry;
+	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+	struct autofs_info *ino = autofs4_dentry_ino(dentry);
+	int status;
+
+	DPRINTK("dentry=%p %.*s",
+		dentry, dentry->d_name.len, dentry->d_name.name);
+
+	/* The daemon never triggers a mount. */
+	if (autofs4_oz_mode(sbi))
+		return NULL;
+
+	/*
+	 * If an expire request is pending everyone must wait.
+	 * If the expire fails we're still mounted so continue
+	 * the follow and return. A return of -EAGAIN (which only
+	 * happens with indirect mounts) means the expire completed
+	 * and the directory was removed, so just go ahead and try
+	 * the mount.
+	 */
+	status = do_expire_wait(dentry);
+	if (status && status != -EAGAIN)
+		return NULL;
+
+	/* Callback to the daemon to perform the mount or wait */
+	spin_lock(&sbi->fs_lock);
+	if (ino->flags & AUTOFS_INF_PENDING) {
+		spin_unlock(&sbi->fs_lock);
+		status = autofs4_mount_wait(dentry);
+		if (status)
+			return ERR_PTR(status);
+		spin_lock(&sbi->fs_lock);
+		goto done;
+	}
+
+	/*
+	 * If the dentry is a symlink it's equivalent to a directory
+	 * having d_mounted() true, so there's no need to call back
+	 * to the daemon.
+	 */
+	if (dentry->d_inode && S_ISLNK(dentry->d_inode->i_mode))
+		goto done;
+	spin_lock(&dentry->d_lock); /////////////// IS THIS THE RIGHT LOCK?  USED TO BE DCACHE_LOCK
+	if (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
+		ino->flags |= AUTOFS_INF_PENDING;
+		spin_unlock(&dentry->d_lock);
+		spin_unlock(&sbi->fs_lock);
+		status = autofs4_mount_wait(dentry);
+		if (status)
+			return ERR_PTR(status);
+		spin_lock(&sbi->fs_lock);
+		ino->flags &= ~AUTOFS_INF_PENDING;
+		goto done;
+	}
+	spin_unlock(&dentry->d_lock);
+done:
+	/*
+	 * Any needed mounting has been completed and the path updated
+	 * so turn this into a normal dentry so we don't continually
+	 * call ->d_automount().
+	 */
+	managed_dentry_clear_automount(dentry);
+	spin_unlock(&sbi->fs_lock);
+
+	/* Mount succeeded, check if we ended up with a new dentry */
+	dentry = autofs4_mountpoint_changed(path);
+	if (!dentry)
+		return ERR_PTR(-ENOENT);
+
+	return NULL;
+}
+
 /* Lookups in the root directory */
 static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 {
 	struct autofs_sb_info *sbi;
 	struct autofs_info *ino;
-	struct dentry *expiring, *active;
-	int oz_mode;
+	struct dentry *active;
 
-	DPRINTK("name = %.*s",
-		dentry->d_name.len, dentry->d_name.name);
+	DPRINTK("name = %.*s", dentry->d_name.len, dentry->d_name.name);
 
 	/* File name too long to exist */
 	if (dentry->d_name.len > NAME_MAX)
 		return ERR_PTR(-ENAMETOOLONG);
 
 	sbi = autofs4_sbi(dir->i_sb);
-	oz_mode = autofs4_oz_mode(sbi);
 
 	DPRINTK("pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d",
-		 current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
+		current->pid, task_pgrp_nr(current), sbi->catatonic, oz_mode);
 
 	active = autofs4_lookup_active(dentry);
 	if (active) {
-		dentry = active;
-		ino = autofs4_dentry_ino(dentry);
+		return active;
 	} else {
-		/*
-		 * Mark the dentry incomplete but don't hash it. We do this
-		 * to serialize our inode creation operations (symlink and
-		 * mkdir) which prevents deadlock during the callback to
-		 * the daemon. Subsequent user space lookups for the same
-		 * dentry are placed on the wait queue while the daemon
-		 * itself is allowed passage unresticted so the create
-		 * operation itself can then hash the dentry. Finally,
-		 * we check for the hashed dentry and return the newly
-		 * hashed dentry.
-		 */
 		d_set_d_op(dentry, &autofs4_root_dentry_operations);
 
 		/*
-		 * And we need to ensure that the same dentry is used for
-		 * all following lookup calls until it is hashed so that
-		 * the dentry flags are persistent throughout the request.
+		 * A dentry that is not within the root can never trigger a
+		 * mount operation, unless the directory already exists, so we
+		 * can return fail immediately.  The daemon however does need
+		 * to create directories within the file system.
 		 */
+		if (!autofs4_oz_mode(sbi) && !IS_ROOT(dentry->d_parent))
+			return ERR_PTR(-ENOENT);
+
+		/* Mark entries in the root as mount triggers */
+		if (autofs_type_indirect(sbi->type) && IS_ROOT(dentry->d_parent)) {
+			d_set_d_op(dentry, &autofs4_dentry_operations);
+			managed_dentry_set_automount(dentry);
+		}
+
 		ino = autofs4_init_ino(NULL, sbi, 0555);
 		if (!ino)
 			return ERR_PTR(-ENOMEM);
@@ -595,82 +718,6 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
 
 		d_instantiate(dentry, NULL);
 	}
-
-	if (!oz_mode) {
-		mutex_unlock(&dir->i_mutex);
-		expiring = autofs4_lookup_expiring(dentry);
-		if (expiring) {
-			/*
-			 * If we are racing with expire the request might not
-			 * be quite complete but the directory has been removed
-			 * so it must have been successful, so just wait for it.
-			 */
-			autofs4_expire_wait(expiring);
-			autofs4_del_expiring(expiring);
-			dput(expiring);
-		}
-
-		spin_lock(&sbi->fs_lock);
-		ino->flags |= AUTOFS_INF_PENDING;
-		spin_unlock(&sbi->fs_lock);
-		if (dentry->d_op && dentry->d_op->d_revalidate)
-			(dentry->d_op->d_revalidate)(dentry, nd);
-		mutex_lock(&dir->i_mutex);
-	}
-
-	/*
-	 * If we are still pending, check if we had to handle
-	 * a signal. If so we can force a restart..
-	 */
-	if (ino->flags & AUTOFS_INF_PENDING) {
-		/* See if we were interrupted */
-		if (signal_pending(current)) {
-			sigset_t *sigset = &current->pending.signal;
-			if (sigismember (sigset, SIGKILL) ||
-			    sigismember (sigset, SIGQUIT) ||
-			    sigismember (sigset, SIGINT)) {
-			    if (active)
-				dput(active);
-			    return ERR_PTR(-ERESTARTNOINTR);
-			}
-		}
-		if (!oz_mode) {
-			spin_lock(&sbi->fs_lock);
-			ino->flags &= ~AUTOFS_INF_PENDING;
-			spin_unlock(&sbi->fs_lock);
-		}
-	}
-
-	/*
-	 * If this dentry is unhashed, then we shouldn't honour this
-	 * lookup.  Returning ENOENT here doesn't do the right thing
-	 * for all system calls, but it should be OK for the operations
-	 * we permit from an autofs.
-	 */
-	if (!oz_mode && d_unhashed(dentry)) {
-		/*
-		 * A user space application can (and has done in the past)
-		 * remove and re-create this directory during the callback.
-		 * This can leave us with an unhashed dentry, but a
-		 * successful mount!  So we need to perform another
-		 * cached lookup in case the dentry now exists.
-		 */
-		struct dentry *parent = dentry->d_parent;
-		struct dentry *new = d_lookup(parent, &dentry->d_name);
-		if (new != NULL)
-			dentry = new;
-		else
-			dentry = ERR_PTR(-ENOENT);
-
-		if (active)
-			dput(active);
-
-		return dentry;
-	}
-
-	if (active)
-		return active;
-
 	return NULL;
 }
 
@@ -715,11 +762,6 @@ static int autofs4_dir_symlink(struct inode *dir,
 	}
 	d_add(dentry, inode);
 
-	if (dir == dir->i_sb->s_root->d_inode)
-		d_set_d_op(dentry, &autofs4_root_dentry_operations);
-	else
-		d_set_d_op(dentry, &autofs4_dentry_operations);
-
 	dentry->d_fsdata = ino;
 	ino->dentry = dget(dentry);
 	atomic_inc(&ino->count);
@@ -850,11 +892,6 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	}
 	d_add(dentry, inode);
 
-	if (dir == dir->i_sb->s_root->d_inode)
-		d_set_d_op(dentry, &autofs4_root_dentry_operations);
-	else
-		d_set_d_op(dentry, &autofs4_dentry_operations);
-
 	dentry->d_fsdata = ino;
 	ino->dentry = dget(dentry);
 	atomic_inc(&ino->count);


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

* [PATCH 10/18] autofs4: Add d_manage() dentry operation
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (8 preceding siblings ...)
  2011-01-11 17:48 ` [PATCH 09/18] autofs4: Add d_automount() dentry operation David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 11/18] autofs4: Remove unused code David Howells
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

This patch required a previous patch to add the ->d_automount()
dentry operation.

Add a function to use the newly defined ->d_manage() dentry operation
for blocking during mount and expire.

Whether the VFS calls the dentry operations d_automount() and d_manage()
is controled by the DMANAGED_AUTOMOUNT and DMANAGED_TRANSIT flags. autofs
uses the d_automount() operation to callback to user space to request
mount operations and the d_manage() operation to block walks into mounts
that are under construction or destruction.

In order to prevent these functions from being called unnecessarily the
DMANAGED_* flags are cleared for cases which would cause this. In the
common case the DMANAGED_AUTOMOUNT and DMANAGED_TRANSIT flags are both
set for dentrys waiting to be mounted. The DMANAGED_TRANSIT flag is
cleared upon successful mount request completion and set during expire
runs, both during the dentry expire check, and if selected for expire,
is left set until a subsequent successful mount request completes.

The exception to this is the so-called rootless multi-mount which has
no actual mount at its base. In this case the DMANAGED_AUTOMOUNT flag
is cleared upon successful mount request completion as well and set
again after a successful expire.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/autofs_i.h |   50 ++++++++++++++++++++++++-
 fs/autofs4/expire.c   |   51 +++++++++++++------------
 fs/autofs4/inode.c    |    3 +
 fs/autofs4/root.c     |  100 +++++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 164 insertions(+), 40 deletions(-)

diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 1ebfe53..7eff538 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -99,7 +99,6 @@ struct autofs_info {
 };
 
 #define AUTOFS_INF_EXPIRING	(1<<0) /* dentry is in the process of expiring */
-#define AUTOFS_INF_MOUNTPOINT	(1<<1) /* mountpoint status for direct expire */
 #define AUTOFS_INF_PENDING	(1<<2) /* dentry pending mount */
 
 struct autofs_wait_queue {
@@ -221,6 +220,7 @@ extern const struct file_operations autofs4_root_operations;
 /* Operations methods */
 
 struct vfsmount *autofs4_d_automount(struct path *);
+int autofs4_d_manage(struct path *, bool);
 
 /* VFS automount flags management functions */
 
@@ -248,6 +248,54 @@ static inline void managed_dentry_clear_automount(struct dentry *dentry)
 	spin_unlock(&dentry->d_lock);
 }
 
+static inline void __managed_dentry_set_transit(struct dentry *dentry)
+{
+	dentry->d_flags |= DCACHE_MANAGE_TRANSIT;
+}
+
+static inline void managed_dentry_set_transit(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_set_transit(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
+static inline void __managed_dentry_clear_transit(struct dentry *dentry)
+{
+	dentry->d_flags &= ~DCACHE_MANAGE_TRANSIT;
+}
+
+static inline void managed_dentry_clear_transit(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_clear_transit(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
+static inline void __managed_dentry_set_managed(struct dentry *dentry)
+{
+	dentry->d_flags |= (DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT);
+}
+
+static inline void managed_dentry_set_managed(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_set_managed(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
+static inline void __managed_dentry_clear_managed(struct dentry *dentry)
+{
+	dentry->d_flags &= ~(DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT);
+}
+
+static inline void managed_dentry_clear_managed(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	__managed_dentry_clear_managed(dentry);
+	spin_unlock(&dentry->d_lock);
+}
+
 /* Initializing function */
 
 int autofs4_fill_super(struct super_block *, void *, int);
diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
index 0571ec8..3ed79d7 100644
--- a/fs/autofs4/expire.c
+++ b/fs/autofs4/expire.c
@@ -26,10 +26,6 @@ static inline int autofs4_can_expire(struct dentry *dentry,
 	if (ino == NULL)
 		return 0;
 
-	/* No point expiring a pending mount */
-	if (ino->flags & AUTOFS_INF_PENDING)
-		return 0;
-
 	if (!do_now) {
 		/* Too young to die */
 		if (!timeout || time_after(ino->last_used + timeout, now))
@@ -283,6 +279,7 @@ struct dentry *autofs4_expire_direct(struct super_block *sb,
 	unsigned long timeout;
 	struct dentry *root = dget(sb->s_root);
 	int do_now = how & AUTOFS_EXP_IMMEDIATE;
+	struct autofs_info *ino;
 
 	if (!root)
 		return NULL;
@@ -291,20 +288,21 @@ struct dentry *autofs4_expire_direct(struct super_block *sb,
 	timeout = sbi->exp_timeout;
 
 	spin_lock(&sbi->fs_lock);
+	ino = autofs4_dentry_ino(root);
+	/* No point expiring a pending mount */
+	if (ino->flags & AUTOFS_INF_PENDING) {
+		spin_unlock(&sbi->fs_lock);
+		return NULL;
+	}
+	managed_dentry_set_transit(root);
 	if (!autofs4_direct_busy(mnt, root, timeout, do_now)) {
 		struct autofs_info *ino = autofs4_dentry_ino(root);
-		if (d_mountpoint(root)) {
-			ino->flags |= AUTOFS_INF_MOUNTPOINT;
-			spin_lock(&root->d_lock);
-			root->d_flags &= ~DCACHE_MOUNTED;
-			spin_unlock(&root->d_lock);
-		}
 		ino->flags |= AUTOFS_INF_EXPIRING;
-		managed_dentry_set_automount(root);
 		init_completion(&ino->expire_complete);
 		spin_unlock(&sbi->fs_lock);
 		return root;
 	}
+	managed_dentry_clear_transit(root);
 	spin_unlock(&sbi->fs_lock);
 	dput(root);
 
@@ -341,6 +339,10 @@ struct dentry *autofs4_expire_indirect(struct super_block *sb,
 	while ((dentry = get_next_positive_dentry(dentry, root))) {
 		spin_lock(&sbi->fs_lock);
 		ino = autofs4_dentry_ino(dentry);
+		/* No point expiring a pending mount */
+		if (ino->flags & AUTOFS_INF_PENDING)
+			goto cont;
+		managed_dentry_set_transit(dentry);
 
 		/*
 		 * Case 1: (i) indirect mount or top level pseudo direct mount
@@ -400,6 +402,8 @@ struct dentry *autofs4_expire_indirect(struct super_block *sb,
 			}
 		}
 next:
+		managed_dentry_clear_transit(dentry);
+cont:
 		spin_unlock(&sbi->fs_lock);
 	}
 	return NULL;
@@ -409,7 +413,6 @@ found:
 		expired, (int)expired->d_name.len, expired->d_name.name);
 	ino = autofs4_dentry_ino(expired);
 	ino->flags |= AUTOFS_INF_EXPIRING;
-	managed_dentry_set_automount(expired);
 	init_completion(&ino->expire_complete);
 	spin_unlock(&sbi->fs_lock);
 	spin_lock(&autofs4_lock);
@@ -482,7 +485,7 @@ int autofs4_expire_run(struct super_block *sb,
 	ino = autofs4_dentry_ino(dentry);
 	ino->flags &= ~AUTOFS_INF_EXPIRING;
 	if (!d_unhashed(dentry))
-		managed_dentry_clear_automount(dentry);
+		managed_dentry_clear_transit(dentry);
 	complete_all(&ino->expire_complete);
 	spin_unlock(&sbi->fs_lock);
 
@@ -508,20 +511,18 @@ int autofs4_do_expire_multi(struct super_block *sb, struct vfsmount *mnt,
 		ret = autofs4_wait(sbi, dentry, NFY_EXPIRE);
 
 		spin_lock(&sbi->fs_lock);
-		if (ino->flags & AUTOFS_INF_MOUNTPOINT) {
-			spin_lock(&sb->s_root->d_lock);
-			/*
-			 * If we haven't been expired away, then reset
-			 * mounted status.
-			 */
-			if (mnt->mnt_parent != mnt)
-				sb->s_root->d_flags |= DCACHE_MOUNTED;
-			spin_unlock(&sb->s_root->d_lock);
-			ino->flags &= ~AUTOFS_INF_MOUNTPOINT;
-		}
 		ino->flags &= ~AUTOFS_INF_EXPIRING;
+		spin_lock(&dentry->d_lock);
 		if (ret)
-			managed_dentry_clear_automount(dentry);
+			__managed_dentry_clear_transit(dentry);
+		else {
+			if ((IS_ROOT(dentry) ||
+			    (autofs_type_indirect(sbi->type) &&
+			     IS_ROOT(dentry->d_parent))) &&
+			    !(dentry->d_flags & DCACHE_NEED_AUTOMOUNT))
+				__managed_dentry_set_automount(dentry);
+		}
+		spin_unlock(&dentry->d_lock);
 		complete_all(&ino->expire_complete);
 		spin_unlock(&sbi->fs_lock);
 		dput(dentry);
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index d0aa38c..75c1ed8 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -253,6 +253,7 @@ static struct autofs_info *autofs4_mkroot(struct autofs_sb_info *sbi)
 
 static const struct dentry_operations autofs4_sb_dentry_operations = {
 	.d_automount	= autofs4_d_automount,
+	.d_manage	= autofs4_d_manage,
 	.d_release      = autofs4_dentry_release,
 };
 
@@ -322,7 +323,7 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
 	}
 
 	if (autofs_type_trigger(sbi->type))
-		__managed_dentry_set_automount(root);
+		__managed_dentry_set_managed(root);
 
 	root_inode->i_fop = &autofs4_root_operations;
 	root_inode->i_op = autofs_type_trigger(sbi->type) ?
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index bf8a69d..b2cc422 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -424,6 +424,7 @@ static const struct dentry_operations autofs4_root_dentry_operations = {
 /* For other dentries */
 static const struct dentry_operations autofs4_dentry_operations = {
 	.d_automount	= autofs4_d_automount,
+	.d_manage	= autofs4_d_manage,
 	.d_release	= autofs4_dentry_release,
 };
 
@@ -604,6 +605,20 @@ struct vfsmount *autofs4_d_automount(struct path *path)
 	DPRINTK("dentry=%p %.*s",
 		dentry, dentry->d_name.len, dentry->d_name.name);
 
+	/*
+	 * Someone may have manually umounted this or it was a submount
+	 * that has gone away.
+	 */
+	//spin_lock(&dcache_lock);  /////////////// JUST DELETE THIS LOCK?
+	if (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
+		spin_lock(&dentry->d_lock);
+		if (!(dentry->d_flags & DCACHE_MANAGE_TRANSIT) &&
+		     (dentry->d_flags & DCACHE_NEED_AUTOMOUNT))
+			__managed_dentry_set_transit(path->dentry);
+		spin_unlock(&dentry->d_lock);
+	}
+	//spin_unlock(&dcache_lock);
+
 	/* The daemon never triggers a mount. */
 	if (autofs4_oz_mode(sbi))
 		return NULL;
@@ -633,31 +648,65 @@ struct vfsmount *autofs4_d_automount(struct path *path)
 
 	/*
 	 * If the dentry is a symlink it's equivalent to a directory
-	 * having d_mounted() true, so there's no need to call back
+	 * having d_mountpoint() true, so there's no need to call back
 	 * to the daemon.
 	 */
 	if (dentry->d_inode && S_ISLNK(dentry->d_inode->i_mode))
 		goto done;
-	spin_lock(&dentry->d_lock); /////////////// IS THIS THE RIGHT LOCK?  USED TO BE DCACHE_LOCK
-	if (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
+	if (!d_mountpoint(dentry)) {
+		/*
+		 * It's possible that user space hasn't removed directories
+		 * after umounting a rootless multi-mount, although it
+		 * should. For v5 have_submounts() is sufficient to handle
+		 * this because the leaves of the directory tree under the
+		 * mount never trigger mounts themselves (they have an autofs
+		 * trigger mount mounted on them). But v4 pseudo direct mounts
+		 * do need the leaves to to trigger mounts. In this case we
+		 * have no choice but to use the list_empty() check and
+		 * require user space behave.
+		 */
+		if (sbi->version > 4) {
+			if (have_submounts(dentry))
+				goto done;
+		} else {
+			spin_lock(&dentry->d_lock); /////////////// IS THIS THE RIGHT LOCK?  USED TO BE DCACHE_LOCK
+			if (!list_empty(&dentry->d_subdirs)) {
+				spin_unlock(&dentry->d_lock);
+				goto done;
+			}
+			spin_unlock(&dentry->d_lock);
+		}
 		ino->flags |= AUTOFS_INF_PENDING;
-		spin_unlock(&dentry->d_lock);
 		spin_unlock(&sbi->fs_lock);
 		status = autofs4_mount_wait(dentry);
 		if (status)
 			return ERR_PTR(status);
 		spin_lock(&sbi->fs_lock);
 		ino->flags &= ~AUTOFS_INF_PENDING;
-		goto done;
 	}
-	spin_unlock(&dentry->d_lock);
 done:
-	/*
-	 * Any needed mounting has been completed and the path updated
-	 * so turn this into a normal dentry so we don't continually
-	 * call ->d_automount().
-	 */
-	managed_dentry_clear_automount(dentry);
+	if (!(ino->flags & AUTOFS_INF_EXPIRING)) {
+		/*
+		 * Any needed mounting has been completed and the path updated
+		 * so turn this into a normal dentry so we don't continually
+		 * call ->d_automount() and ->d_manage().
+		 */
+		//spin_lock(&dcache_lock); /////////// JUST DELETE THIS LOCK?
+		spin_lock(&dentry->d_lock);
+		__managed_dentry_clear_transit(dentry);
+		/*
+		 * Only clear DMANAGED_AUTOMOUNT for rootless multi-mounts and
+		 * symlinks as in all other cases the dentry will be covered by
+		 * an actual mount so ->d_automount() won't be called during
+		 * the follow.
+		 */
+		if ((!d_mountpoint(dentry) &&
+		    !list_empty(&dentry->d_subdirs)) ||
+		    (dentry->d_inode && S_ISLNK(dentry->d_inode->i_mode)))
+			__managed_dentry_clear_automount(dentry);
+		spin_unlock(&dentry->d_lock);
+		//spin_unlock(&dcache_lock);
+	}
 	spin_unlock(&sbi->fs_lock);
 
 	/* Mount succeeded, check if we ended up with a new dentry */
@@ -668,6 +717,31 @@ done:
 	return NULL;
 }
 
+int autofs4_d_manage(struct path *path, bool mounting_here)
+{
+	struct dentry *dentry = path->dentry;
+	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
+
+	DPRINTK("dentry=%p %.*s",
+		dentry, dentry->d_name.len, dentry->d_name.name);
+
+	/* The daemon never waits. */
+	if (autofs4_oz_mode(sbi) || mounting_here) {
+		if (!d_mountpoint(dentry))
+			return -EISDIR;
+		return 0;
+	}
+
+	/* Wait for pending expires */
+	do_expire_wait(dentry);
+
+	/*
+	 * This dentry may be under construction so wait on mount
+	 * completion.
+	 */
+	return autofs4_mount_wait(dentry);
+}
+
 /* Lookups in the root directory */
 static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 {
@@ -704,7 +778,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
 		/* Mark entries in the root as mount triggers */
 		if (autofs_type_indirect(sbi->type) && IS_ROOT(dentry->d_parent)) {
 			d_set_d_op(dentry, &autofs4_dentry_operations);
-			managed_dentry_set_automount(dentry);
+			__managed_dentry_set_managed(dentry);
 		}
 
 		ino = autofs4_init_ino(NULL, sbi, 0555);


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

* [PATCH 11/18] autofs4: Remove unused code
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (9 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 10/18] autofs4: Add d_manage() " David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 12/18] autofs4: Clean up inode operations David Howells
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

Remove code that is not used due to the use of ->d_automount()
and ->d_manage().

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/autofs_i.h |    7 -
 fs/autofs4/root.c     |  243 -------------------------------------------------
 2 files changed, 0 insertions(+), 250 deletions(-)

diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 7eff538..8b746f6 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -175,13 +175,6 @@ static inline int autofs4_ispending(struct dentry *dentry)
 	return 0;
 }
 
-static inline void autofs4_copy_atime(struct file *src, struct file *dst)
-{
-	dst->f_path.dentry->d_inode->i_atime =
-		src->f_path.dentry->d_inode->i_atime;
-	return;
-}
-
 struct inode *autofs4_get_inode(struct super_block *, struct autofs_info *);
 void autofs4_free_ino(struct autofs_info *);
 
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index b2cc422..91db5dd 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -36,9 +36,6 @@ static long autofs4_root_compat_ioctl(struct file *,unsigned int,unsigned long);
 static int autofs4_dir_open(struct inode *inode, struct file *file);
 static struct dentry *autofs4_lookup(struct inode *,struct dentry *, struct nameidata *);
 
-#define TRIGGER_FLAGS   (LOOKUP_CONTINUE | LOOKUP_DIRECTORY)
-#define TRIGGER_INTENTS (LOOKUP_OPEN | LOOKUP_CREATE)
-
 const struct file_operations autofs4_root_operations = {
 	.open		= dcache_dir_open,
 	.release	= dcache_dir_close,
@@ -114,14 +111,6 @@ static void autofs4_del_active(struct dentry *dentry)
 	return;
 }
 
-static unsigned int autofs4_need_mount(unsigned int flags)
-{
-	unsigned int res = 0;
-	if (flags & (TRIGGER_FLAGS | TRIGGER_INTENTS))
-		res = 1;
-	return res;
-}
-
 static int autofs4_dir_open(struct inode *inode, struct file *file)
 {
 	struct dentry *dentry = file->f_path.dentry;
@@ -156,238 +145,6 @@ out:
 	return dcache_dir_open(inode, file);
 }
 
-static int try_to_fill_dentry(struct dentry *dentry, int flags)
-{
-	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
-	struct autofs_info *ino = autofs4_dentry_ino(dentry);
-	int status;
-
-	DPRINTK("dentry=%p %.*s ino=%p",
-		 dentry, dentry->d_name.len, dentry->d_name.name, dentry->d_inode);
-
-	/*
-	 * Wait for a pending mount, triggering one if there
-	 * isn't one already
-	 */
-	if (dentry->d_inode == NULL) {
-		DPRINTK("waiting for mount name=%.*s",
-			 dentry->d_name.len, dentry->d_name.name);
-
-		status = autofs4_wait(sbi, dentry, NFY_MOUNT);
-
-		DPRINTK("mount done status=%d", status);
-
-		/* Turn this into a real negative dentry? */
-		if (status == -ENOENT) {
-			spin_lock(&sbi->fs_lock);
-			ino->flags &= ~AUTOFS_INF_PENDING;
-			spin_unlock(&sbi->fs_lock);
-			return status;
-		} else if (status) {
-			/* Return a negative dentry, but leave it "pending" */
-			return status;
-		}
-	/* Trigger mount for path component or follow link */
-	} else if (ino->flags & AUTOFS_INF_PENDING ||
-			autofs4_need_mount(flags)) {
-		DPRINTK("waiting for mount name=%.*s",
-			dentry->d_name.len, dentry->d_name.name);
-
-		spin_lock(&sbi->fs_lock);
-		ino->flags |= AUTOFS_INF_PENDING;
-		spin_unlock(&sbi->fs_lock);
-		status = autofs4_wait(sbi, dentry, NFY_MOUNT);
-
-		DPRINTK("mount done status=%d", status);
-
-		if (status) {
-			spin_lock(&sbi->fs_lock);
-			ino->flags &= ~AUTOFS_INF_PENDING;
-			spin_unlock(&sbi->fs_lock);
-			return status;
-		}
-	}
-
-	/* Initialize expiry counter after successful mount */
-	ino->last_used = jiffies;
-
-	spin_lock(&sbi->fs_lock);
-	ino->flags &= ~AUTOFS_INF_PENDING;
-	spin_unlock(&sbi->fs_lock);
-
-	return 0;
-}
-
-/* For autofs direct mounts the follow link triggers the mount */
-static void *autofs4_follow_link(struct dentry *dentry, struct nameidata *nd)
-{
-	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
-	struct autofs_info *ino = autofs4_dentry_ino(dentry);
-	int oz_mode = autofs4_oz_mode(sbi);
-	unsigned int lookup_type;
-	int status;
-
-	DPRINTK("dentry=%p %.*s oz_mode=%d nd->flags=%d",
-		dentry, dentry->d_name.len, dentry->d_name.name, oz_mode,
-		nd->flags);
-	/*
-	 * For an expire of a covered direct or offset mount we need
-	 * to break out of follow_down_one() at the autofs mount trigger
-	 * (d_mounted--), so we can see the expiring flag, and manage
-	 * the blocking and following here until the expire is completed.
-	 */
-	if (oz_mode) {
-		spin_lock(&sbi->fs_lock);
-		if (ino->flags & AUTOFS_INF_EXPIRING) {
-			spin_unlock(&sbi->fs_lock);
-			/* Follow down to our covering mount. */
-			if (!follow_down_one(&nd->path))
-				goto done;
-			goto follow;
-		}
-		spin_unlock(&sbi->fs_lock);
-		goto done;
-	}
-
-	/* If an expire request is pending everyone must wait. */
-	autofs4_expire_wait(dentry);
-
-	/* We trigger a mount for almost all flags */
-	lookup_type = autofs4_need_mount(nd->flags);
-	spin_lock(&sbi->fs_lock);
-	spin_lock(&autofs4_lock);
-	spin_lock(&dentry->d_lock);
-	if (!(lookup_type || ino->flags & AUTOFS_INF_PENDING)) {
-		spin_unlock(&dentry->d_lock);
-		spin_unlock(&autofs4_lock);
-		spin_unlock(&sbi->fs_lock);
-		goto follow;
-	}
-
-	/*
-	 * If the dentry contains directories then it is an autofs
-	 * multi-mount with no root mount offset. So don't try to
-	 * mount it again.
-	 */
-	if (ino->flags & AUTOFS_INF_PENDING ||
-	    (!d_mountpoint(dentry) && list_empty(&dentry->d_subdirs))) {
-		spin_unlock(&dentry->d_lock);
-		spin_unlock(&autofs4_lock);
-		spin_unlock(&sbi->fs_lock);
-
-		status = try_to_fill_dentry(dentry, nd->flags);
-		if (status)
-			goto out_error;
-
-		goto follow;
-	}
-	spin_unlock(&dentry->d_lock);
-	spin_unlock(&autofs4_lock);
-	spin_unlock(&sbi->fs_lock);
-follow:
-	/*
-	 * If there is no root mount it must be an autofs
-	 * multi-mount with no root offset so we don't need
-	 * to follow it.
-	 */
-	if (d_managed(dentry)) {
-		status = follow_down(&nd->path, false);
-		if (status < 0)
-			goto out_error;
-	}
-
-done:
-	return NULL;
-
-out_error:
-	path_put(&nd->path);
-	return ERR_PTR(status);
-}
-
-/*
- * Revalidate is called on every cache lookup.  Some of those
- * cache lookups may actually happen while the dentry is not
- * yet completely filled in, and revalidate has to delay such
- * lookups..
- */
-static int autofs4_revalidate(struct dentry *dentry, struct nameidata *nd)
-{
-	struct inode *dir;
-	struct autofs_sb_info *sbi;
-	int oz_mode;
-	int flags = nd ? nd->flags : 0;
-	int status = 1;
-
-	if (flags & LOOKUP_RCU)
-		return -ECHILD;
-
-	dir = dentry->d_parent->d_inode;
-	sbi = autofs4_sbi(dir->i_sb);
-	oz_mode = autofs4_oz_mode(sbi);
-
-	/* Pending dentry */
-	spin_lock(&sbi->fs_lock);
-	if (autofs4_ispending(dentry)) {
-		/* The daemon never causes a mount to trigger */
-		spin_unlock(&sbi->fs_lock);
-
-		if (oz_mode)
-			return 1;
-
-		/*
-		 * If the directory has gone away due to an expire
-		 * we have been called as ->d_revalidate() and so
-		 * we need to return false and proceed to ->lookup().
-		 */
-		if (autofs4_expire_wait(dentry) == -EAGAIN)
-			return 0;
-
-		/*
-		 * A zero status is success otherwise we have a
-		 * negative error code.
-		 */
-		status = try_to_fill_dentry(dentry, flags);
-		if (status == 0)
-			return 1;
-
-		return status;
-	}
-	spin_unlock(&sbi->fs_lock);
-
-	/* Negative dentry.. invalidate if "old" */
-	if (dentry->d_inode == NULL)
-		return 0;
-
-	/* Check for a non-mountpoint directory with no contents */
-	spin_lock(&autofs4_lock);
-	spin_lock(&dentry->d_lock);
-	if (S_ISDIR(dentry->d_inode->i_mode) &&
-	    !d_mountpoint(dentry) && list_empty(&dentry->d_subdirs)) {
-		DPRINTK("dentry=%p %.*s, emptydir",
-			 dentry, dentry->d_name.len, dentry->d_name.name);
-		spin_unlock(&dentry->d_lock);
-		spin_unlock(&autofs4_lock);
-
-		/* The daemon never causes a mount to trigger */
-		if (oz_mode)
-			return 1;
-
-		/*
-		 * A zero status is success otherwise we have a
-		 * negative error code.
-		 */
-		status = try_to_fill_dentry(dentry, flags);
-		if (status == 0)
-			return 1;
-
-		return status;
-	}
-	spin_unlock(&dentry->d_lock);
-	spin_unlock(&autofs4_lock);
-
-	return 1;
-}
-
 void autofs4_dentry_release(struct dentry *de)
 {
 	struct autofs_info *inf;


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

* [PATCH 12/18] autofs4: Clean up inode operations
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (10 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 11/18] autofs4: Remove unused code David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 13/18] autofs4: Clean up dentry operations David Howells
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

Since the use of ->follow_link() has been eliminated there is no
need to separate the indirect and direct inode operations.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/autofs_i.h |    3 ---
 fs/autofs4/inode.c    |    4 +---
 fs/autofs4/root.c     |   15 ---------------
 3 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 8b746f6..c3b0afe 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -204,9 +204,6 @@ void autofs_dev_ioctl_exit(void);
 
 extern const struct inode_operations autofs4_symlink_inode_operations;
 extern const struct inode_operations autofs4_dir_inode_operations;
-extern const struct inode_operations autofs4_root_inode_operations;
-extern const struct inode_operations autofs4_indirect_root_inode_operations;
-extern const struct inode_operations autofs4_direct_root_inode_operations;
 extern const struct file_operations autofs4_dir_operations;
 extern const struct file_operations autofs4_root_operations;
 
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 75c1ed8..dac3dc7 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -326,9 +326,7 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
 		__managed_dentry_set_managed(root);
 
 	root_inode->i_fop = &autofs4_root_operations;
-	root_inode->i_op = autofs_type_trigger(sbi->type) ?
-			&autofs4_direct_root_inode_operations :
-			&autofs4_indirect_root_inode_operations;
+	root_inode->i_op = &autofs4_dir_inode_operations;
 
 	/* Couldn't this be tested earlier? */
 	if (sbi->max_proto < AUTOFS_MIN_PROTO_VERSION ||
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 91db5dd..ab391d4 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -56,21 +56,6 @@ const struct file_operations autofs4_dir_operations = {
 	.llseek		= dcache_dir_lseek,
 };
 
-const struct inode_operations autofs4_indirect_root_inode_operations = {
-	.lookup		= autofs4_lookup,
-	.unlink		= autofs4_dir_unlink,
-	.symlink	= autofs4_dir_symlink,
-	.mkdir		= autofs4_dir_mkdir,
-	.rmdir		= autofs4_dir_rmdir,
-};
-
-const struct inode_operations autofs4_direct_root_inode_operations = {
-	.lookup		= autofs4_lookup,
-	.unlink		= autofs4_dir_unlink,
-	.mkdir		= autofs4_dir_mkdir,
-	.rmdir		= autofs4_dir_rmdir,
-};
-
 const struct inode_operations autofs4_dir_inode_operations = {
 	.lookup		= autofs4_lookup,
 	.unlink		= autofs4_dir_unlink,


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

* [PATCH 13/18] autofs4: Clean up dentry operations
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (11 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 12/18] autofs4: Clean up inode operations David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 14/18] autofs4: Clean up autofs4_free_ino() David Howells
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

There are now two distinct dentry operations uses. One for dentrys
that trigger mounts and one for dentrys that do not.

Rationalize the use of these dentry operations and rename them to
reflect their function.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/autofs_i.h |    7 ++-----
 fs/autofs4/inode.c    |   12 ++++--------
 fs/autofs4/root.c     |   36 ++++++++++++++++++++----------------
 3 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index c3b0afe..c28085c 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -206,11 +206,8 @@ extern const struct inode_operations autofs4_symlink_inode_operations;
 extern const struct inode_operations autofs4_dir_inode_operations;
 extern const struct file_operations autofs4_dir_operations;
 extern const struct file_operations autofs4_root_operations;
-
-/* Operations methods */
-
-struct vfsmount *autofs4_d_automount(struct path *);
-int autofs4_d_manage(struct path *, bool);
+extern const struct dentry_operations autofs4_dentry_operations;
+extern const struct dentry_operations autofs4_mount_dentry_operations;
 
 /* VFS automount flags management functions */
 
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index dac3dc7..427c357 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -251,12 +251,6 @@ static struct autofs_info *autofs4_mkroot(struct autofs_sb_info *sbi)
 	return ino;
 }
 
-static const struct dentry_operations autofs4_sb_dentry_operations = {
-	.d_automount	= autofs4_d_automount,
-	.d_manage	= autofs4_d_manage,
-	.d_release      = autofs4_dentry_release,
-};
-
 int autofs4_fill_super(struct super_block *s, void *data, int silent)
 {
 	struct inode * root_inode;
@@ -311,7 +305,7 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
 		goto fail_iput;
 	pipe = NULL;
 
-	d_set_d_op(root, &autofs4_sb_dentry_operations);
+	d_set_d_op(root, &autofs4_dentry_operations);
 	root->d_fsdata = ino;
 
 	/* Can this call block? */
@@ -322,8 +316,10 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
 		goto fail_dput;
 	}
 
-	if (autofs_type_trigger(sbi->type))
+	if (autofs_type_trigger(sbi->type)) {
+		d_set_d_op(root, &autofs4_mount_dentry_operations);
 		__managed_dentry_set_managed(root);
+	}
 
 	root_inode->i_fop = &autofs4_root_operations;
 	root_inode->i_op = &autofs4_dir_inode_operations;
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index ab391d4..b3ab9e9 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -35,6 +35,8 @@ static long autofs4_root_compat_ioctl(struct file *,unsigned int,unsigned long);
 #endif
 static int autofs4_dir_open(struct inode *inode, struct file *file);
 static struct dentry *autofs4_lookup(struct inode *,struct dentry *, struct nameidata *);
+static struct vfsmount *autofs4_d_automount(struct path *);
+static int autofs4_d_manage(struct path *, bool);
 
 const struct file_operations autofs4_root_operations = {
 	.open		= dcache_dir_open,
@@ -64,6 +66,18 @@ const struct inode_operations autofs4_dir_inode_operations = {
 	.rmdir		= autofs4_dir_rmdir,
 };
 
+/* For dentries that don't initiate mounting */
+const struct dentry_operations autofs4_dentry_operations = {
+	.d_release	= autofs4_dentry_release,
+};
+
+/* For dentries that do initiate mounting */
+const struct dentry_operations autofs4_mount_dentry_operations = {
+	.d_automount	= autofs4_d_automount,
+	.d_manage	= autofs4_d_manage,
+	.d_release	= autofs4_dentry_release,
+};
+
 static void autofs4_add_active(struct dentry *dentry)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
@@ -158,18 +172,6 @@ void autofs4_dentry_release(struct dentry *de)
 	}
 }
 
-/* For dentries of directories in the root dir */
-static const struct dentry_operations autofs4_root_dentry_operations = {
-	.d_release	= autofs4_dentry_release,
-};
-
-/* For other dentries */
-static const struct dentry_operations autofs4_dentry_operations = {
-	.d_automount	= autofs4_d_automount,
-	.d_manage	= autofs4_d_manage,
-	.d_release	= autofs4_dentry_release,
-};
-
 static struct dentry *autofs4_lookup_active(struct dentry *dentry)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
@@ -337,7 +339,7 @@ static struct dentry *autofs4_mountpoint_changed(struct path *path)
 	return path->dentry;
 }
 
-struct vfsmount *autofs4_d_automount(struct path *path)
+static struct vfsmount *autofs4_d_automount(struct path *path)
 {
 	struct dentry *dentry = path->dentry;
 	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
@@ -506,7 +508,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
 	if (active) {
 		return active;
 	} else {
-		d_set_d_op(dentry, &autofs4_root_dentry_operations);
+		d_set_d_op(dentry, &autofs4_dentry_operations);
 
 		/*
 		 * A dentry that is not within the root can never trigger a
@@ -519,7 +521,7 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
 
 		/* Mark entries in the root as mount triggers */
 		if (autofs_type_indirect(sbi->type) && IS_ROOT(dentry->d_parent)) {
-			d_set_d_op(dentry, &autofs4_dentry_operations);
+			d_set_d_op(dentry, &autofs4_mount_dentry_operations);
 			__managed_dentry_set_managed(dentry);
 		}
 
@@ -578,6 +580,8 @@ static int autofs4_dir_symlink(struct inode *dir,
 	}
 	d_add(dentry, inode);
 
+	d_set_d_op(dentry, &autofs4_dentry_operations);
+
 	dentry->d_fsdata = ino;
 	ino->dentry = dget(dentry);
 	atomic_inc(&ino->count);
@@ -796,7 +800,7 @@ static inline int autofs4_ask_umount(struct vfsmount *mnt, int __user *p)
 int is_autofs4_dentry(struct dentry *dentry)
 {
 	return dentry && dentry->d_inode &&
-		(dentry->d_op == &autofs4_root_dentry_operations ||
+		(dentry->d_op == &autofs4_mount_dentry_operations ||
 		 dentry->d_op == &autofs4_dentry_operations) &&
 		dentry->d_fsdata != NULL;
 }


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

* [PATCH 14/18] autofs4: Clean up autofs4_free_ino()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (12 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 13/18] autofs4: Clean up dentry operations David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 15/18] autofs4: Fix wait validation David Howells
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

When this function is called the local reference count does't need to
be updated since the dentry is going away and dput definitely must
not be called here.

Also the autofs info struct field inode isn't used so remove it.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/inode.c |   13 -------------
 fs/autofs4/root.c  |    9 ---------
 2 files changed, 0 insertions(+), 22 deletions(-)

diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 427c357..3ecd2e2 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -45,7 +45,6 @@ struct autofs_info *autofs4_init_ino(struct autofs_info *ino,
 
 	if (!reinit) {
 		ino->flags = 0;
-		ino->inode = NULL;
 		ino->dentry = NULL;
 		ino->size = 0;
 		INIT_LIST_HEAD(&ino->active);
@@ -76,19 +75,8 @@ struct autofs_info *autofs4_init_ino(struct autofs_info *ino,
 
 void autofs4_free_ino(struct autofs_info *ino)
 {
-	struct autofs_info *p_ino;
-
 	if (ino->dentry) {
 		ino->dentry->d_fsdata = NULL;
-		if (ino->dentry->d_inode) {
-			struct dentry *parent = ino->dentry->d_parent;
-			if (atomic_dec_and_test(&ino->count)) {
-				p_ino = autofs4_dentry_ino(parent);
-				if (p_ino && parent != ino->dentry)
-					atomic_dec(&p_ino->count);
-			}
-			dput(ino->dentry);
-		}
 		ino->dentry = NULL;
 	}
 	if (ino->free)
@@ -390,7 +378,6 @@ struct inode *autofs4_get_inode(struct super_block *sb,
 	if (inode == NULL)
 		return NULL;
 
-	inf->inode = inode;
 	inode->i_mode = inf->mode;
 	if (sb->s_root) {
 		inode->i_uid = sb->s_root->d_inode->i_uid;
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index b3ab9e9..51a4c2a 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -151,11 +151,8 @@ void autofs4_dentry_release(struct dentry *de)
 	DPRINTK("releasing %p", de);
 
 	inf = autofs4_dentry_ino(de);
-	de->d_fsdata = NULL;
-
 	if (inf) {
 		struct autofs_sb_info *sbi = autofs4_sbi(de->d_sb);
-
 		if (sbi) {
 			spin_lock(&sbi->lookup_lock);
 			if (!list_empty(&inf->active))
@@ -164,10 +161,6 @@ void autofs4_dentry_release(struct dentry *de)
 				list_del(&inf->expiring);
 			spin_unlock(&sbi->lookup_lock);
 		}
-
-		inf->dentry = NULL;
-		inf->inode = NULL;
-
 		autofs4_free_ino(inf);
 	}
 }
@@ -588,7 +581,6 @@ static int autofs4_dir_symlink(struct inode *dir,
 	p_ino = autofs4_dentry_ino(dentry->d_parent);
 	if (p_ino && dentry->d_parent != dentry)
 		atomic_inc(&p_ino->count);
-	ino->inode = inode;
 
 	ino->u.symlink = cp;
 	dir->i_mtime = CURRENT_TIME;
@@ -718,7 +710,6 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	p_ino = autofs4_dentry_ino(dentry->d_parent);
 	if (p_ino && dentry->d_parent != dentry)
 		atomic_inc(&p_ino->count);
-	ino->inode = inode;
 	inc_nlink(dir);
 	dir->i_mtime = CURRENT_TIME;
 


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

* [PATCH 15/18] autofs4: Fix wait validation
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (13 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 14/18] autofs4: Clean up autofs4_free_ino() David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 16/18] autofs4: Add v4 pseudo direct mount support David Howells
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

It is possible for the check in wait.c:validate_request() to return
an incorrect result if the dentry that was mounted upon has changed
during the callback.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/waitq.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index c5f8459..5601005 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -309,6 +309,9 @@ static int validate_request(struct autofs_wait_queue **wait,
 	 * completed while we waited on the mutex ...
 	 */
 	if (notify == NFY_MOUNT) {
+		struct dentry *new = NULL;
+		int valid = 1;
+
 		/*
 		 * If the dentry was successfully mounted while we slept
 		 * on the wait queue mutex we can return success. If it
@@ -316,8 +319,20 @@ static int validate_request(struct autofs_wait_queue **wait,
 		 * a multi-mount with no mount at it's base) we can
 		 * continue on and create a new request.
 		 */
+		if (!IS_ROOT(dentry)) {
+			if (dentry->d_inode && d_unhashed(dentry)) {
+				struct dentry *parent = dentry->d_parent;
+				new = d_lookup(parent, &dentry->d_name);
+				if (new)
+					dentry = new;
+			}
+		}
 		if (have_submounts(dentry))
-			return 0;
+			valid = 0;
+
+		if (new)
+			dput(new);
+		return valid;
 	}
 
 	return 1;


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

* [PATCH 16/18] autofs4: Add v4 pseudo direct mount support
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (14 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 15/18] autofs4: Fix wait validation David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 17/18] autofs4: Bump version David Howells
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

Version 4 of autofs provides a pseudo direct mount implementation
that relies on directories at the leaves of a directory tree under
an indirect mount to trigger mounts.

This patch adds support for that functionality.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/autofs4/root.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 51a4c2a..752693f 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -635,6 +635,60 @@ static int autofs4_dir_unlink(struct inode *dir, struct dentry *dentry)
 	return 0;
 }
 
+/*
+ * Version 4 of autofs provides a pseudo direct mount implementation
+ * that relies on directories at the leaves of a directory tree under
+ * an indirect mount to trigger mounts. To allow for this we need to
+ * set the DMANAGED_AUTOMOUNT and DMANAGED_TRANSIT flags on the leaves
+ * of the directory tree. There is no need to clear the automount flag
+ * following a mount or restore it after an expire because these mounts
+ * are always covered. However, it is neccessary to ensure that these
+ * flags are clear on non-empty directories to avoid unnecessary calls
+ * during path walks.
+ */
+static void autofs_set_leaf_automount_flags(struct dentry *dentry)
+{
+	struct dentry *parent;
+
+	/* root and dentrys in the root are already handled */
+	if (IS_ROOT(dentry->d_parent))
+		return;
+
+	managed_dentry_set_managed(dentry);
+
+	parent = dentry->d_parent;
+	/* only consider parents below dentrys in the root */
+	if (IS_ROOT(parent->d_parent))
+		return;
+	managed_dentry_clear_managed(parent);
+	return;
+}
+
+static void autofs_clear_leaf_automount_flags(struct dentry *dentry)
+{
+	struct list_head *d_child;
+	struct dentry *parent;
+
+	/* flags for dentrys in the root are handled elsewhere */
+	if (IS_ROOT(dentry->d_parent))
+		return;
+
+	managed_dentry_clear_managed(dentry);
+
+	parent = dentry->d_parent;
+	/* only consider parents below dentrys in the root */
+	if (IS_ROOT(parent->d_parent))
+		return;
+	d_child = &dentry->d_u.d_child;
+	//spin_lock(&dcache_lock);  ////////////// JUST DELETE THIS LOCK?
+	/* Set parent managed if it's becoming empty */
+	if (d_child->next == &parent->d_subdirs &&
+	    d_child->prev == &parent->d_subdirs)
+		managed_dentry_set_managed(parent);
+	//spin_unlock(&dcache_lock);
+	return;
+}
+
 static int autofs4_dir_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct autofs_sb_info *sbi = autofs4_sbi(dir->i_sb);
@@ -662,6 +716,9 @@ static int autofs4_dir_rmdir(struct inode *dir, struct dentry *dentry)
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&autofs4_lock);
 
+	if (sbi->version < 5)
+		autofs_clear_leaf_automount_flags(dentry);
+
 	if (atomic_dec_and_test(&ino->count)) {
 		p_ino = autofs4_dentry_ino(dentry->d_parent);
 		if (p_ino && dentry->d_parent != dentry)
@@ -704,6 +761,9 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	}
 	d_add(dentry, inode);
 
+	if (sbi->version < 5)
+		autofs_set_leaf_automount_flags(dentry);
+
 	dentry->d_fsdata = ino;
 	ino->dentry = dget(dentry);
 	atomic_inc(&ino->count);


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

* [PATCH 17/18] autofs4: Bump version
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (15 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 16/18] autofs4: Add v4 pseudo direct mount support David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-11 17:49 ` [PATCH 18/18] Remove a further kludge from __do_follow_link() David Howells
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

From: Ian Kent <raven@themaw.net>

Increase the autofs module sub-version so we can tell what kernel
implementation is being used from user space debug logging.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/auto_fs4.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/auto_fs4.h b/include/linux/auto_fs4.h
index 8b49ac4..e02982f 100644
--- a/include/linux/auto_fs4.h
+++ b/include/linux/auto_fs4.h
@@ -24,7 +24,7 @@
 #define AUTOFS_MIN_PROTO_VERSION	3
 #define AUTOFS_MAX_PROTO_VERSION	5
 
-#define AUTOFS_PROTO_SUBVERSION		1
+#define AUTOFS_PROTO_SUBVERSION		2
 
 /* Mask for expire behaviour */
 #define AUTOFS_EXP_IMMEDIATE		1


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

* [PATCH 18/18] Remove a further kludge from __do_follow_link()
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (16 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 17/18] autofs4: Bump version David Howells
@ 2011-01-11 17:49 ` David Howells
  2011-01-12 13:52 ` [PATCH 07/18] Add more dentry flags for special function directories [UPDATE] David Howells
  2011-01-12 19:16 ` [PATCH 00/18] Introduce automount support in the VFS David Howells
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-11 17:49 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel

Remove a further kludge from __do_follow_link() as it's no longer retired with
the automount code.

This reverts the non-helper-function parts of
051d381259eb57d6074d02a6ba6e90e744f1a29f, which breaks union mounts.

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

 fs/namei.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index e2c5db2..d81c91e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -781,13 +781,8 @@ __do_follow_link(struct path *path, struct nameidata *nd, void **p)
 	touch_atime(path->mnt, dentry);
 	nd_set_link(nd, NULL);
 
-	if (path->mnt != nd->path.mnt) {
-		path_to_nameidata(path, nd);
-		nd->inode = nd->path.dentry->d_inode;
-		dget(dentry);
-	}
-	mntget(path->mnt);
-
+	if (path->mnt == nd->path.mnt)
+		mntget(path->mnt);
 	nd->last_type = LAST_BIND;
 	*p = dentry->d_inode->i_op->follow_link(dentry, nd);
 	error = PTR_ERR(*p);


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

* Re: [PATCH 07/18] Add more dentry flags for special function directories [UPDATE]
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (17 preceding siblings ...)
  2011-01-11 17:49 ` [PATCH 18/18] Remove a further kludge from __do_follow_link() David Howells
@ 2011-01-12 13:52 ` David Howells
  2011-01-12 19:16 ` [PATCH 00/18] Introduce automount support in the VFS David Howells
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-12 13:52 UTC (permalink / raw)
  To: npiggin, viro, raven; +Cc: dhowells, autofs, linux-fsdevel


Here's an updated patch 7.  d_set_d_op() can't be used to set
DCACHE_NEED_AUTOMOUNT as d_inode is not set at that point.  It has to be done
in __d_instantiate() at the latest.

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] Add more dentry flags for special function directories

Add more flags to the d_flags in struct dentry for special function directories
such as mountpoints and autofs substructures.  The relevant flags are:

 (*) DCACHE_MOUNTED.

     (Already exists).  Indicates that this dentry has things mounted upon it.

 (*) DCACHE_NEED_AUTOMOUNT.

     This is a reflection of the S_AUTOMOUNT inode flag.  This is reflected by
     __d_instantiate() and similar.  follow_automount() is now keyed off of the
     dcache flag rather than being keyed off S_AUTOMOUNT directly.  Possibly
     S_AUTOMOUNT should shift to the dentry entirely.

     This may also be tweaked live (such as by the autofs4 filesystem) to alter
     the effect.

 (*) DCACHE_MANAGE_TRANSIT.

     This is an indicator that the filesystem that owns the dentry wants to
     manage processes transiting away from that dentry.  If this is set on a
     dentry, then a new dentry op:

		int (*d_manage)(struct path *);

     is invoked.  This is allowed to sleep and is allowed to return an error.

     This allows autofs to hold non-Oz-mode processes here without any
     filesystem locks being held.

__follow_mount() is replaced by managed_dentry() which now handles transit to a
mountpoint's root dentry, automount points and points that the filesystem wants
to manage.


==========================
WHAT THIS MEANS FOR AUTOFS
==========================

autofs currently uses the lookup() inode op and the d_revalidate() dentry op to
trigger the automounting of indirect mounts, and both of these can be called
with i_mutex held.

autofs knows that the i_mutex will be held by the caller in lookup(), and so
can drop it before invoking the daemon - but this isn't so for d_revalidate(),
since the lock is only held on _some_ of the code paths that call it.  This
means that autofs can't risk dropping i_mutex from its d_revalidate() function
before it calls the daemon.

The bug could manifest itself as, for example, a process that's trying to
validate an automount dentry that gets made to wait because that dentry is
expired and needs cleaning up:

	mkdir         S ffffffff8014e05a     0 32580  24956
	Call Trace:
	 [<ffffffff885371fd>] :autofs4:autofs4_wait+0x674/0x897
	 [<ffffffff80127f7d>] avc_has_perm+0x46/0x58
	 [<ffffffff8009fdcf>] autoremove_wake_function+0x0/0x2e
	 [<ffffffff88537be6>] :autofs4:autofs4_expire_wait+0x41/0x6b
	 [<ffffffff88535cfc>] :autofs4:autofs4_revalidate+0x91/0x149
	 [<ffffffff80036d96>] __lookup_hash+0xa0/0x12f
	 [<ffffffff80057a2f>] lookup_create+0x46/0x80
	 [<ffffffff800e6e31>] sys_mkdirat+0x56/0xe4

versus the automount daemon which wants to remove that dentry, but can't
because the normal process is holding the i_mutex lock:

	automount     D ffffffff8014e05a     0 32581      1              32561
	Call Trace:
	 [<ffffffff80063c3f>] __mutex_lock_slowpath+0x60/0x9b
	 [<ffffffff8000ccf1>] do_path_lookup+0x2ca/0x2f1
	 [<ffffffff80063c89>] .text.lock.mutex+0xf/0x14
	 [<ffffffff800e6d55>] do_rmdir+0x77/0xde
	 [<ffffffff8005d229>] tracesys+0x71/0xe0
	 [<ffffffff8005d28d>] tracesys+0xd5/0xe0

which means that the system is deadlocked.

This patch allows autofs to hold up normal processes whilst the daemon goes
ahead and does things to the dentry tree behind the automouter point without
risking a deadlock as no locks are held in d_manage() or d_automount().

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Ian Kent <raven@themaw.net>
---

 Documentation/filesystems/vfs.txt |   13 +++
 fs/dcache.c                       |    5 +
 fs/namei.c                        |  153 ++++++++++++++++++++++++++-----------
 include/linux/dcache.h            |   13 ++-
 4 files changed, 132 insertions(+), 52 deletions(-)


diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index bb8d277..99f0127 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -865,6 +865,7 @@ struct dentry_operations {
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
 	struct vfsmount *(*d_automount)(struct path *);
+	int (*d_manage)(struct path *);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -940,8 +941,16 @@ struct dentry_operations {
 	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.
+	This function is only used if DMANAGED_AUTOMOUNT is set on the dentry.
+	This is set by d_add() if S_AUTOMOUNT is set on the inode being added.
+
+  d_manage: called to allow the filesystem to manage the transition from a
+	dentry (optional).  This allows autofs, for example, to hold up clients
+	waiting to explore behind a 'mountpoint', whilst letting the daemon go
+	past and construct the subtree there.
+
+	This function is only used if DMANAGED_TRANSIT is set on the dentry
+	being transited from.
 
 Example :
 
diff --git a/fs/dcache.c b/fs/dcache.c
index 5699d4c..2ef5fcd 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1378,8 +1378,11 @@ EXPORT_SYMBOL(d_set_d_op);
 static void __d_instantiate(struct dentry *dentry, struct inode *inode)
 {
 	spin_lock(&dentry->d_lock);
-	if (inode)
+	if (inode) {
+		if (unlikely(IS_AUTOMOUNT(inode)))
+			dentry->d_flags |= DCACHE_NEED_AUTOMOUNT;
 		list_add(&dentry->d_alias, &inode->i_dentry);
+	}
 	dentry->d_inode = inode;
 	dentry_rcuwalk_barrier(dentry);
 	spin_unlock(&dentry->d_lock);
diff --git a/fs/namei.c b/fs/namei.c
index 7622825..1e31f96 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -878,7 +878,7 @@ int follow_up(struct path *path)
 
 /*
  * Perform an automount
- * - return -EISDIR to tell __follow_mount() to stop and return the path we
+ * - return -EISDIR to tell managed_dentry() to stop and return the path we
  *   were called with.
  */
 static int follow_automount(struct path *path, unsigned flags,
@@ -893,7 +893,7 @@ static int follow_automount(struct path *path, unsigned flags,
 	 * 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 */
+		return -EISDIR; /* 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
@@ -913,8 +913,20 @@ static int follow_automount(struct path *path, unsigned flags,
 		return -ELOOP;
 
 	mnt = path->dentry->d_op->d_automount(path);
-	if (IS_ERR(mnt))
+	if (IS_ERR(mnt)) {
+		/*
+		 * The filesystem is allowed to return -EISDIR here to indicate
+		 * it doesn'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) == -EISDIR && (flags & LOOKUP_CONTINUE))
+			return -EREMOTE;
 		return PTR_ERR(mnt);
+	}
 	if (!mnt) /* mount collision */
 		return 0;
 
@@ -934,46 +946,66 @@ static int follow_automount(struct path *path, unsigned flags,
 }
 
 /*
- * serialization is taken care of in namespace.c
+ * Handle a dentry that is managed in some way.
+ * - Flagged for transit management (autofs)
+ * - Flagged as mountpoint
+ * - Flagged as automount point
+ *
+ * This may only be called in refwalk mode.
  */
-static int __follow_mount(struct path *path, unsigned flags)
+static int managed_dentry(struct path *path, unsigned flags)
 {
-	struct vfsmount *mounted;
+	unsigned managed;
 	bool need_mntput = false;
 	int ret;
 
-	for (;;) {
-		while (d_mountpoint(path->dentry)) {
-			mounted = lookup_mnt(path);
-			if (!mounted)
-				break;
-			dput(path->dentry);
-			if (need_mntput)
-				mntput(path->mnt);
-			path->mnt = mounted;
-			path->dentry = dget(mounted->mnt_root);
-			need_mntput = true;
+	/* Given that we're not holding a lock here, we retain the value in a
+	 * local variable for each dentry as we look at it so that we don't see
+	 * the components of that value change under us */
+	while (managed = ACCESS_ONCE(path->dentry->d_flags),
+	       managed &= DCACHE_MANAGED_DENTRY,
+	       unlikely(managed != 0)) {
+		/* Allow the filesystem to manage the transit without i_mutex
+		 * being held. */
+		if (managed & DCACHE_MANAGE_TRANSIT) {
+			BUG_ON(!path->dentry->d_op);
+			BUG_ON(!path->dentry->d_op->d_manage);
+			ret = path->dentry->d_op->d_manage(path);
+			if (ret < 0)
+				return ret;
 		}
-		if (!d_automount_point(path->dentry))
-			break;
-		ret = follow_automount(path, flags, &need_mntput);
-		if (ret < 0)
-			return ret == -EISDIR ? 0 : ret;
-	}
-	return 0;
-}
 
-static void follow_mount(struct path *path)
-{
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
-			break;
-		dput(path->dentry);
-		mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
+		/* Transit to a mounted filesystem. */
+		if (managed & DCACHE_MOUNTED) {
+			struct vfsmount *mounted = lookup_mnt(path);
+			if (mounted) {
+				dput(path->dentry);
+				if (need_mntput)
+					mntput(path->mnt);
+				path->mnt = mounted;
+				path->dentry = dget(mounted->mnt_root);
+				need_mntput = true;
+				continue;
+			}
+
+			/* Something is mounted on this dentry in another
+			 * namespace and/or whatever was mounted there in this
+			 * namespace got unmounted before we managed to get the
+			 * vfsmount_lock */
+		}
+
+		/* Handle an automount point */
+		if (managed & DCACHE_NEED_AUTOMOUNT) {
+			ret = follow_automount(path, flags, &need_mntput);
+			if (ret < 0)
+				return ret == -EISDIR ? 0 : ret;
+			continue;
+		}
+
+		/* We didn't change the current path point */
+		break;
 	}
+	return 0;
 }
 
 int follow_down(struct path *path)
@@ -991,19 +1023,31 @@ int follow_down(struct path *path)
 	return 0;
 }
 
-static void __follow_mount_rcu(struct nameidata *nd, struct path *path,
-				struct inode **inode)
+/*
+ * Skip to top of mountpoint pile in rcuwalk mode.  If requested we abort the
+ * walk when we hit a dentry that has DCACHE_MANAGE_TRANSIT flagged (we don't
+ * want to take note of it when walking "..").
+ */
+static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
+			       struct inode **inode,
+			       bool abort_on_managed_dentry)
 {
+	unsigned abort_mask =
+		abort_on_managed_dentry ? DCACHE_MANAGE_TRANSIT : 0;
+
 	while (d_mountpoint(path->dentry)) {
 		struct vfsmount *mounted;
+		if (path->dentry->d_flags & abort_mask)
+			return true;
 		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
 		if (!mounted)
-			return;
+			break;
 		path->mnt = mounted;
 		path->dentry = mounted->mnt_root;
 		nd->seq = read_seqcount_begin(&path->dentry->d_seq);
 		*inode = path->dentry->d_inode;
 	}
+	return false;
 }
 
 static int follow_dotdot_rcu(struct nameidata *nd)
@@ -1012,7 +1056,7 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 
 	set_root_rcu(nd);
 
-	while(1) {
+	while (1) {
 		if (nd->path.dentry == nd->root.dentry &&
 		    nd->path.mnt == nd->root.mnt) {
 			break;
@@ -1035,12 +1079,28 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 		nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
 		inode = nd->path.dentry->d_inode;
 	}
-	__follow_mount_rcu(nd, &nd->path, &inode);
+	__follow_mount_rcu(nd, &nd->path, &inode, false);
 	nd->inode = inode;
 
 	return 0;
 }
 
+/*
+ * Skip to top of mountpoint pile in refwalk mode for follow_dotdot()
+ */
+static void follow_mount(struct path *path)
+{
+	while (d_mountpoint(path->dentry)) {
+		struct vfsmount *mounted = lookup_mnt(path);
+		if (!mounted)
+			break;
+		dput(path->dentry);
+		mntput(path->mnt);
+		path->mnt = mounted;
+		path->dentry = dget(mounted->mnt_root);
+	}
+}
+
 static void follow_dotdot(struct nameidata *nd)
 {
 	set_root(nd);
@@ -1105,13 +1165,14 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent = nd->path.dentry;
 	struct inode *dir;
+	int err;
 
 	/*
 	 * See if the low-level filesystem might want
 	 * to use its own hash..
 	 */
 	if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
-		int err = parent->d_op->d_hash(parent, nd->inode, name);
+		err = parent->d_op->d_hash(parent, nd->inode, name);
 		if (err < 0)
 			return err;
 	}
@@ -1140,7 +1201,9 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 			goto need_revalidate;
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount_rcu(nd, path, inode);
+		if (unlikely(__follow_mount_rcu(nd, path, inode, true)) &&
+		    nameidata_drop_rcu(nd))
+			return -ECHILD;
 	} else {
 		dentry = __d_lookup(parent, name);
 		if (!dentry)
@@ -1151,7 +1214,9 @@ found:
 done:
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount(path, nd->flags);
+		err = managed_dentry(path, nd->flags);
+		if (unlikely(err < 0))
+			return err;
 		*inode = path->dentry->d_inode;
 	}
 	return 0;
@@ -2236,7 +2301,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (open_flag & O_EXCL)
 		goto exit_dput;
 
-	error = __follow_mount(path, nd->flags);
+	error = managed_dentry(path, nd->flags);
 	if (error < 0)
 		goto exit_dput;
 
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 444614b..c6a4821 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -168,6 +168,7 @@ struct dentry_operations {
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
 	struct vfsmount *(*d_automount)(struct path *);
+	int (*d_manage)(struct path *);
 } ____cacheline_aligned;
 
 /*
@@ -206,13 +207,18 @@ struct dentry_operations {
 
 #define DCACHE_CANT_MOUNT	0x0100
 #define DCACHE_GENOCIDE		0x0200
-#define DCACHE_MOUNTED		0x0400	/* is a mountpoint */
 
 #define DCACHE_OP_HASH		0x1000
 #define DCACHE_OP_COMPARE	0x2000
 #define DCACHE_OP_REVALIDATE	0x4000
 #define DCACHE_OP_DELETE	0x8000
 
+#define DCACHE_MOUNTED		0x10000	/* is a mountpoint */
+#define DCACHE_NEED_AUTOMOUNT	0x20000	/* handle automount on this dir */
+#define DCACHE_MANAGE_TRANSIT	0x40000	/* manage transit from this dirent */
+#define DCACHE_MANAGED_DENTRY \
+	(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)
+
 extern seqlock_t rename_lock;
 
 static inline int dname_external(struct dentry *dentry)
@@ -400,14 +406,11 @@ static inline void dont_mount(struct dentry *dentry)
 
 extern void dput(struct dentry *);
 
-static inline int d_mountpoint(struct dentry *dentry)
+static inline bool d_mountpoint(struct dentry *dentry)
 {
 	return dentry->d_flags & DCACHE_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);
 

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

* Re: [PATCH 00/18] Introduce automount support in the VFS
  2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
                   ` (18 preceding siblings ...)
  2011-01-12 13:52 ` [PATCH 07/18] Add more dentry flags for special function directories [UPDATE] David Howells
@ 2011-01-12 19:16 ` David Howells
  19 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-12 19:16 UTC (permalink / raw)
  To: viro; +Cc: dhowells, npiggin, raven, autofs, linux-fsdevel


I've updated:

http://git.kernel.org/?p=linux/kernel/git/dhowells/linux-2.6-automount.git;a=summary

to Linus's latest GIT tree with my latest patches on top.

David

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
@ 2011-01-13  3:53   ` Nick Piggin
  2011-01-13  5:10     ` Nick Piggin
  2011-01-13 15:12     ` David Howells
  2011-01-13 12:20   ` David Howells
  1 sibling, 2 replies; 30+ messages in thread
From: Nick Piggin @ 2011-01-13  3:53 UTC (permalink / raw)
  To: David Howells; +Cc: npiggin, viro, raven, autofs, linux-fsdevel

On Wed, Jan 12, 2011 at 4:48 AM, David Howells <dhowells@redhat.com> wrote:
> 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.
>
> A new pathwalk subroutine, follow_automount() is added to handle mountpoints.
> It will return -EREMOTE if the S_AUTOMOUNT was set, but no d_automount() op was
> supplied, -ELOOP if we've encountered too many symlinks or mountpoints, -EISDIR
> if the walk point should be used without mounting and 0 if successful.  path
> will be updated if an automount took place to point to the mounted filesystem.
>
> I've only changed __follow_mount() to handle call follow_automount(), but it
> might be necessary to change follow_mount() too.  The latter is only called
> 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>
> Acked-by: Ian Kent <raven@themaw.net>

I would still prefer to see a .follow_mount API, and not tie in this automount
specific inode detail to what could be a more flexible dentry-only API.

The default NULL implementation would do nothing, and follow_automount
stuff can be in fs/libfs.c to be used by filesystem, rather than fs/namei.c.

I would rather that the automount call just attach the mount and then go
back to namei.c where it does the mount hash lookup.

That should take like a couple of lines in the path lookup code.


>  locking rules:
>                rename_lock     ->d_lock        may block       rcu-walk
> @@ -29,6 +30,7 @@ d_delete:     no              yes             no              no
>  d_release:     no              no              yes             no
>  d_iput:                no              no              yes             no
>  d_dname:       no              no              no              no
> +d_automount:   no              no              no              yes

> +static void __follow_mount_rcu(struct nameidata *nd, struct path *path,
> +                               struct inode **inode)
> +{
> +       while (d_mountpoint(path->dentry)) {
> +               struct vfsmount *mounted;
> +               mounted = __lookup_mnt(path->mnt, path->dentry, 1);
> +               if (!mounted)
> +                       return;
> +               path->mnt = mounted;
> +               path->dentry = mounted->mnt_root;
> +               nd->seq = read_seqcount_begin(&path->dentry->d_seq);
> +               *inode = path->dentry->d_inode;
> +       }
> +}

Where we have 2 functions, one with _rcu postfix, the _rcu version is used
for rcu-walk lookups, and the other one used for ref-walk lookups.

This means they have to provide the exact same functionality, or where
that isn't possible, the _rcu variant must return -ECHILD.

So something has gone wrong here. You have documented .d_automount
can be called in rcu-walk mode, but it doesn't seem to be the case. And in
the _rcu variant you skip checking automount functionality entirely. So at
least you'd have to check for d_automount_point and bail out if it exists. But
you need to be careful:


> +#define d_automount_point(dentry) \
> +       (dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))

> @@ -277,6 +278,7 @@ struct inodes_stat_t {
>  #define IS_SWAPFILE(inode)     ((inode)->i_flags & S_SWAPFILE)
>  #define IS_PRIVATE(inode)      ((inode)->i_flags & S_PRIVATE)
>  #define IS_IMA(inode)          ((inode)->i_flags & S_IMA)
> +#define IS_AUTOMOUNT(inode)    ((inode)->i_flags & S_AUTOMOUNT)

This guy will oops in rcu-walk mode, because dentry->d_inode can go
NULL at any time.

If you do want to do this kind of thing in rcu-walk mode, the path walking
code carries the inode around for use (instead of ->d_inode).

If you make sure to have dropped out from rcu-walk mode, then d_inode
can be relied on to be stable, of course.

Thanks,
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-13  3:53   ` Nick Piggin
@ 2011-01-13  5:10     ` Nick Piggin
  2011-01-13 15:12     ` David Howells
  1 sibling, 0 replies; 30+ messages in thread
From: Nick Piggin @ 2011-01-13  5:10 UTC (permalink / raw)
  To: David Howells; +Cc: npiggin, viro, raven, autofs, linux-fsdevel

On Thu, Jan 13, 2011 at 2:53 PM, Nick Piggin <npiggin@gmail.com> wrote:
> On Wed, Jan 12, 2011 at 4:48 AM, David Howells <dhowells@redhat.com> wrote:
>> 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.
>>
>> A new pathwalk subroutine, follow_automount() is added to handle mountpoints.
>> It will return -EREMOTE if the S_AUTOMOUNT was set, but no d_automount() op was
>> supplied, -ELOOP if we've encountered too many symlinks or mountpoints, -EISDIR
>> if the walk point should be used without mounting and 0 if successful.  path
>> will be updated if an automount took place to point to the mounted filesystem.
>>
>> I've only changed __follow_mount() to handle call follow_automount(), but it
>> might be necessary to change follow_mount() too.  The latter is only called
>> 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>
>> Acked-by: Ian Kent <raven@themaw.net>
>
> I would still prefer to see a .follow_mount API, and not tie in this automount
> specific inode detail to what could be a more flexible dentry-only API.
>
> The default NULL implementation would do nothing, and follow_automount
> stuff can be in fs/libfs.c to be used by filesystem, rather than fs/namei.c.

Looking further in the patchset at the d_managed "thing", that's almost what
I'm getting at.

But I don't see why any of this stuff has to happen in fs/namei.c. Just call the
function from path walk, and provide helpers in libfs or something if there is
a lot of common code between autofs4 and others (and leave it autofs specifc
when that is the case).

Of course, that would be the obvious and naive first approach. So really my
question is why did that not work? And can we make it work?

Second observation when adding rcu-walk/ref-walk operations. What we've
done now (which is what Linus preferred and I agree with now) is to make
functions always able to accept rcu-walk mode, and have filesystems bail
out as needed.

Unless there are really fundamental reasons why rcu-walk won't work (eg.
->lookup, if it is required to do allocations and IO), or if it really
doesn't matter
(eg. a function to be called once to set up a mount, and never again).
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
  2011-01-13  3:53   ` Nick Piggin
@ 2011-01-13 12:20   ` David Howells
  2011-01-14  1:04     ` Nick Piggin
  2011-01-14  9:24     ` David Howells
  1 sibling, 2 replies; 30+ messages in thread
From: David Howells @ 2011-01-13 12:20 UTC (permalink / raw)
  To: Nick Piggin; +Cc: dhowells, npiggin, viro, raven, autofs, linux-fsdevel

Nick Piggin <npiggin@gmail.com> wrote:

> So something has gone wrong here. You have documented .d_automount
> can be called in rcu-walk mode, but it doesn't seem to be the case.

Ah.  You removed a column and installed a new one, and I didn't notice.

Neither d_automount() and d_manage() should be entered in rcu-walk mode since
they're both expected to sleep.

Btw, should you add a fifth column for d_seq?

I should also add a column for namespace_sem as d_manage() may be called with
that held (and d_automount() must not be called with that held).

David

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-13  3:53   ` Nick Piggin
  2011-01-13  5:10     ` Nick Piggin
@ 2011-01-13 15:12     ` David Howells
  2011-01-14  1:19       ` Nick Piggin
  1 sibling, 1 reply; 30+ messages in thread
From: David Howells @ 2011-01-13 15:12 UTC (permalink / raw)
  To: Nick Piggin; +Cc: dhowells, npiggin, viro, raven, autofs, linux-fsdevel

Nick Piggin <npiggin@gmail.com> wrote:

> > I would still prefer to see a .follow_mount API, and not tie in this
> > automount specific inode detail to what could be a more flexible
> > dentry-only API.
> >
> > The default NULL implementation would do nothing, and follow_automount
> > stuff can be in fs/libfs.c to be used by filesystem, rather than
> > fs/namei.c.
> 
> Looking further in the patchset at the d_managed "thing", that's almost what
> I'm getting at.
> 
> But I don't see why any of this stuff has to happen in fs/namei.c. Just call
> the function from path walk, and provide helpers in libfs or something if
> there is a lot of common code between autofs4 and others (and leave it autofs
> specifc when that is the case).
> 
> Of course, that would be the obvious and naive first approach. So really my
> question is why did that not work? And can we make it work?

You have a strange idea of what is 'obvious and naive'.  These are parts of
pathwalk, and as such should be in fs/namei.c.  I'd rather not expose
pathwalking directly to the filesystem, though I acknowledge that sometimes it
is necessary to let the filesystem influence it.

You need to consider d_automount() and d_manage() separately as they provide
two quite different hooks with different properties.

Firstly, d_automount().  The following are my points of consideration.

 (0) You're only allowed to automount on a directory.

 (1) Automounting does not need to be done when we follow ".." to an automount
     point.

 (2) Automount points that are mounted upon within the current namespace can
     just be skipped over.  This is the fast path.

 (3) All the filesystem should need as a parameter to determine what it is
     allowed to mount is the inode and dentry of the automount point.  This
     holds true for all the things that currently do automounting (AFS, CIFS,
     NFS, autofs).

 (4) All the filesystem should need to do is set up a vfsmount struct and
     publish it or return an indication that there was a collision and the
     transit should be retried.

 (5) The filesystem is expected to sleep to achieve the automount, so
     spinlocks, RCU locks, preemption disablements or interrupt disablements
     may not be held across this function.

 (6) The filesystem is expected to need a write lock on namespace_sem at some
     point, so this must not be held across the call to d_automount().

 (7) The filesystem won't necessarily be calling do_add_mount() itself in
     d_automount() - in autofs's case, the construction is performed by the
     userspace daemon and then autofs4_d_automount() indicates a collision - so
     we can't move the do_add_mount() to the caller.  Additionally, the
     filesystem may want to use an expiration list.

 (8) There needs to be some limitation in place to prevent runaway
     automounting.  The ELOOP detection mechanism can be used for this.

Taking these considerations, it shows that a small amount of code can be
inserted into pathwalk and used for everything.  However, having worked with
Ian to try and get autofs4 to work with this, we came up with d_manage() to add
in a missing piece.

Note that autofs4 also uses d_automount() to build directory structures behind
the mountpoint rather than mounting on the mountpoint.  In this case, it clears
the AUTOMOUNT flag when construction is complete.

I've allowed d_automount() to return -EISDIR to follow_automount() to indicate
that no mount should be attempted here, and the directory should be given back
to pathwalk to treat as a directory.  This allows autofs's daemon access to the
directory.

Having follow_automount() update the path it has been given with the new
vfsmount and root dentry is purely an optimisation; we could instead simply
return and __follow_mount() will do lookup_mnt() again as it would if a
collision is reported.

In answer to why I haven't made __follow_mount_rcu() handle automount points, I
thought previously I saw a reason why it was unnecessary, but now I'm not so
sure.  It may be that if there are child objects of this dentry then it will
walk onto those rather than automounting - but for some reason it seems still
to automount rather than doing that.


Secondly, d_manage().  The following are the points of consideration:

 (1) A filesystem may want to hold up client processes that want to transit
     from directories in its control during pathwalk - such as when autofs is
     letting its userspace daemon tear down the stuff mounted on or created
     behind a directory.

 (2) A transit may be from a directory to a directory mounted over it, or from
     a directory to an object (file, dir, etc.) pointed to by an entry in that
     directoy.

 (3) The management of dentries in this fashion is a transient affair.

 (4) The mode in which the filesystem is normally entered for this purpose
     should be disabled as soon as possible, though it may be reenabled later
     if needed.

 (5) When the filesystem is ready it should let the held processes proceed or
     it may reject them.

 (6) The filesystem is expected to sleep to achieve this, so spinlocks, RCU
     locks, preemption disablements or interrupt disablements may not be held
     across this function.

 (7) The filesystem must be able to let some processes through whilst holding
     up others.  This allows autofs to let its daemon pass to construct or
     destroy stuff.

 (8) Because do_add_mount() and do_move_mount() transit through piles of
     mounted directories, the filesystem may have to contend with being called
     with namespace_sem held.  This means initiating (un)mounting, even
     indirectly via userspace, is not permitted from this function.

Taking the above into consideration, we determined that we couldn't use one
entry point for both automounting and holding up.  Autofs had the possibility
to deadlock against itself because of (8).

I've allowed autofs to return -EISDIR from d_manage() to indicate that this
directory is the one of interest, and that any directory mounted upon it should
be ignored.  This permits the autofs daemon to ignore the fact the automount
flag is set and it should go through d_automount().


> Second observation when adding rcu-walk/ref-walk operations. What we've done
> now (which is what Linus preferred and I agree with now) is to make functions
> always able to accept rcu-walk mode, and have filesystems bail out as needed.
> 
> Unless there are really fundamental reasons why rcu-walk won't work (eg.
> ->lookup, if it is required to do allocations and IO), or if it really
> doesn't matter (eg. a function to be called once to set up a mount, and never
> again).

d_automount() is expected to sleep, so there's no point not cancelling rcu-walk
mode before entering it.  Granted there will be rare occasions when the
cancellation proves unnecessary because there was a mount collision, but even
then it's very likely we'll be calling alloc_vfsmnt() - which may sleep - and
doing I/O - which may also sleep - before we realise we've got a collision.

Besides, d_automount() is skipped without cancelling rcu-walk mode if the
directory is mounted over.  That is the fast path, and I'd rather not route
that through the filesystem.

So after automounting has happened, all subsequent transits of that mountpoint
will maintain rcu-walk mode, assuming they don't meet DCACHE_MANAGE_TRANSIT.


I grant that I could extend RCU pathwalk through d_manage().  The autofs daemon
might benefit from that, but I contend that the benefit would be minor overall,
and any processes that do get held up will have to abandon RCU pathwalk so that
they can sleep.

David

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-13 12:20   ` David Howells
@ 2011-01-14  1:04     ` Nick Piggin
  2011-01-14  9:24     ` David Howells
  1 sibling, 0 replies; 30+ messages in thread
From: Nick Piggin @ 2011-01-14  1:04 UTC (permalink / raw)
  To: David Howells; +Cc: npiggin, viro, raven, autofs, linux-fsdevel

On Thu, Jan 13, 2011 at 11:20 PM, David Howells <dhowells@redhat.com> wrote:
> Nick Piggin <npiggin@gmail.com> wrote:
>
>> So something has gone wrong here. You have documented .d_automount
>> can be called in rcu-walk mode, but it doesn't seem to be the case.
>
> Ah.  You removed a column and installed a new one, and I didn't notice.

You still have to notice that it is .d_automount in rcu-walk mode, and bail
out if it is. I can't see where you do that.


> Neither d_automount() and d_manage() should be entered in rcu-walk mode since
> they're both expected to sleep.
>
> Btw, should you add a fifth column for d_seq?

rcu-walk means that rcu_read_lock is held, d_seq is open, vfsmount_lock is
held for read. So it's redundant.

> I should also add a column for namespace_sem as d_manage() may be called with
> that held (and d_automount() must not be called with that held).

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

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-13 15:12     ` David Howells
@ 2011-01-14  1:19       ` Nick Piggin
  2011-01-14 16:24         ` Al Viro
  0 siblings, 1 reply; 30+ messages in thread
From: Nick Piggin @ 2011-01-14  1:19 UTC (permalink / raw)
  To: David Howells; +Cc: npiggin, viro, raven, autofs, linux-fsdevel

On Fri, Jan 14, 2011 at 2:12 AM, David Howells <dhowells@redhat.com> wrote:
> Nick Piggin <npiggin@gmail.com> wrote:
>
>> > I would still prefer to see a .follow_mount API, and not tie in this
>> > automount specific inode detail to what could be a more flexible
>> > dentry-only API.
>> >
>> > The default NULL implementation would do nothing, and follow_automount
>> > stuff can be in fs/libfs.c to be used by filesystem, rather than
>> > fs/namei.c.
>>
>> Looking further in the patchset at the d_managed "thing", that's almost what
>> I'm getting at.
>>
>> But I don't see why any of this stuff has to happen in fs/namei.c. Just call
>> the function from path walk, and provide helpers in libfs or something if
>> there is a lot of common code between autofs4 and others (and leave it autofs
>> specifc when that is the case).
>>
>> Of course, that would be the obvious and naive first approach. So really my
>> question is why did that not work? And can we make it work?
>
> You have a strange idea of what is 'obvious and naive'.  These are parts of
> pathwalk, and as such should be in fs/namei.c.  I'd rather not expose
> pathwalking directly to the filesystem, though I acknowledge that sometimes it
> is necessary to let the filesystem influence it.

No, path walk would *not* be affected or implemented at all by the filesystem,
if you read what I wrote. The only difference is a function called at
follow_mount
time, which may decide to mount something on there.

All the autofs stuff would be constrained within auto mounting filesystems and
libraries. follow_automount belongs in automount code, not path walking.

Also, I don't like how you return a vfsmount * and use that in path walking. If
you just returned success and allowed the path walk to continue looking up
mounted directories, it would be cleaner code, IMO.


> You need to consider d_automount() and d_manage() separately as they provide
> two quite different hooks with different properties.
>
> Firstly, d_automount().  The following are my points of consideration.
>
>  (0) You're only allowed to automount on a directory.
>
>  (1) Automounting does not need to be done when we follow ".." to an automount
>     point.
>
>  (2) Automount points that are mounted upon within the current namespace can
>     just be skipped over.  This is the fast path.
>
>  (3) All the filesystem should need as a parameter to determine what it is
>     allowed to mount is the inode and dentry of the automount point.  This
>     holds true for all the things that currently do automounting (AFS, CIFS,
>     NFS, autofs).
>
>  (4) All the filesystem should need to do is set up a vfsmount struct and
>     publish it or return an indication that there was a collision and the
>     transit should be retried.
>
>  (5) The filesystem is expected to sleep to achieve the automount, so
>     spinlocks, RCU locks, preemption disablements or interrupt disablements
>     may not be held across this function.
>
>  (6) The filesystem is expected to need a write lock on namespace_sem at some
>     point, so this must not be held across the call to d_automount().
>
>  (7) The filesystem won't necessarily be calling do_add_mount() itself in
>     d_automount() - in autofs's case, the construction is performed by the
>     userspace daemon and then autofs4_d_automount() indicates a collision - so
>     we can't move the do_add_mount() to the caller.  Additionally, the
>     filesystem may want to use an expiration list.
>
>  (8) There needs to be some limitation in place to prevent runaway
>     automounting.  The ELOOP detection mechanism can be used for this.

I don't know why the dentry d_automount routine has to depend on inodes
at all. That should all be stuffed into the filesystem.


> Taking these considerations, it shows that a small amount of code can be
> inserted into pathwalk and used for everything.  However, having worked with
> Ian to try and get autofs4 to work with this, we came up with d_manage() to add
> in a missing piece.
>
> Note that autofs4 also uses d_automount() to build directory structures behind
> the mountpoint rather than mounting on the mountpoint.  In this case, it clears
> the AUTOMOUNT flag when construction is complete.

And all of this can be done in autofs4 private flags.


> I've allowed d_automount() to return -EISDIR to follow_automount() to indicate
> that no mount should be attempted here, and the directory should be given back
> to pathwalk to treat as a directory.  This allows autofs's daemon access to the
> directory.
>
> Having follow_automount() update the path it has been given with the new
> vfsmount and root dentry is purely an optimisation; we could instead simply
> return and __follow_mount() will do lookup_mnt() again as it would if a
> collision is reported.
>
> In answer to why I haven't made __follow_mount_rcu() handle automount points, I
> thought previously I saw a reason why it was unnecessary, but now I'm not so
> sure.  It may be that if there are child objects of this dentry then it will
> walk onto those rather than automounting - but for some reason it seems still
> to automount rather than doing that.
>
>
> Secondly, d_manage().  The following are the points of consideration:
>
>  (1) A filesystem may want to hold up client processes that want to transit
>     from directories in its control during pathwalk - such as when autofs is
>     letting its userspace daemon tear down the stuff mounted on or created
>     behind a directory.
>
>  (2) A transit may be from a directory to a directory mounted over it, or from
>     a directory to an object (file, dir, etc.) pointed to by an entry in that
>     directoy.
>
>  (3) The management of dentries in this fashion is a transient affair.
>
>  (4) The mode in which the filesystem is normally entered for this purpose
>     should be disabled as soon as possible, though it may be reenabled later
>     if needed.
>
>  (5) When the filesystem is ready it should let the held processes proceed or
>     it may reject them.
>
>  (6) The filesystem is expected to sleep to achieve this, so spinlocks, RCU
>     locks, preemption disablements or interrupt disablements may not be held
>     across this function.
>
>  (7) The filesystem must be able to let some processes through whilst holding
>     up others.  This allows autofs to let its daemon pass to construct or
>     destroy stuff.

So if you change .d_automount as I suggested above to just return success
and do "something", then it would seem to handle all these cases just as well.



>  (8) Because do_add_mount() and do_move_mount() transit through piles of
>     mounted directories, the filesystem may have to contend with being called
>     with namespace_sem held.  This means initiating (un)mounting, even
>     indirectly via userspace, is not permitted from this function.

With this little detail still to take care of. So as far as I can see,
they're not
completely different but very similar in that the filesystem wants a hook
before mounts are followed (better name might be .d_follow_mount).


> Taking the above into consideration, we determined that we couldn't use one
> entry point for both automounting and holding up.  Autofs had the possibility
> to deadlock against itself because of (8).

I'm not against 2 entry points, or a parameter to distinguish the cases. That
seems like a trivial detail compared with the main issues.


>> Unless there are really fundamental reasons why rcu-walk won't work (eg.
>> ->lookup, if it is required to do allocations and IO), or if it really
>> doesn't matter (eg. a function to be called once to set up a mount, and never
>> again).
>
> d_automount() is expected to sleep, so there's no point not cancelling rcu-walk
> mode before entering it.  Granted there will be rare occasions when the
> cancellation proves unnecessary because there was a mount collision, but even
> then it's very likely we'll be calling alloc_vfsmnt() - which may sleep - and
> doing I/O - which may also sleep - before we realise we've got a collision.
>
> Besides, d_automount() is skipped without cancelling rcu-walk mode if the
> directory is mounted over.  That is the fast path, and I'd rather not route
> that through the filesystem.

Fair enough. If it becomes a more general .d_follow(), then allowing rcu-walk is
probably a good idea.


> So after automounting has happened, all subsequent transits of that mountpoint
> will maintain rcu-walk mode, assuming they don't meet DCACHE_MANAGE_TRANSIT.
>
>
> I grant that I could extend RCU pathwalk through d_manage().  The autofs daemon
> might benefit from that, but I contend that the benefit would be minor overall,
> and any processes that do get held up will have to abandon RCU pathwalk so that
> they can sleep.

We'll see. Relatively minor detail to change the API for that case
after we agree
on the larger form of the API.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-13 12:20   ` David Howells
  2011-01-14  1:04     ` Nick Piggin
@ 2011-01-14  9:24     ` David Howells
  1 sibling, 0 replies; 30+ messages in thread
From: David Howells @ 2011-01-14  9:24 UTC (permalink / raw)
  To: Nick Piggin; +Cc: dhowells, npiggin, viro, raven, autofs, linux-fsdevel

Nick Piggin <npiggin@gmail.com> wrote:

> You still have to notice that it is .d_automount in rcu-walk mode, and bail
> out if it is. I can't see where you do that.

follow_managed(), and thus follow_automount() and ->d_automount(), are never
reached in rcu_walk mode, from what I can tell of the code.

There are two places follow_managed() is called:

 (1) do_lookup() - where follow_managed() is only called in the else-part of
     an if-statement contingent on a check of LOOKUP_RCU.

 (2) do_last() - where follow_managed() is subsequent to a mutex having been
     taken, so rcu-walk mode must have been exited prior to this as the
     process may have needed to sleep.

At least, I'm assuming you may not sleep whilst in rcu-walk mode.

David

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-14  1:19       ` Nick Piggin
@ 2011-01-14 16:24         ` Al Viro
  2011-01-14 22:14           ` Nick Piggin
  0 siblings, 1 reply; 30+ messages in thread
From: Al Viro @ 2011-01-14 16:24 UTC (permalink / raw)
  To: Nick Piggin; +Cc: David Howells, npiggin, raven, autofs, linux-fsdevel

On Fri, Jan 14, 2011 at 12:19:43PM +1100, Nick Piggin wrote:

> Also, I don't like how you return a vfsmount * and use that in path walking. If
> you just returned success and allowed the path walk to continue looking up
> mounted directories, it would be cleaner code, IMO.

No.  Filesystem has no business touching mount tree; leave that to core VFS.
Analysis of what's going on is messy enough as it is, let's *NOT* complicate
it even further.

I think we should take do_add_mount() calls into follow_automount(), making
the API
	NULL => nothing to do here, just follow mount
	ERR_PTR() => sod off
	mnt => here's the thing I want mounted, do it and go there.  If
something's allready got mounted here, just disregard this one and follow
mount as usual (== we'd lost the race)

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link()
  2011-01-14 16:24         ` Al Viro
@ 2011-01-14 22:14           ` Nick Piggin
  0 siblings, 0 replies; 30+ messages in thread
From: Nick Piggin @ 2011-01-14 22:14 UTC (permalink / raw)
  To: Al Viro; +Cc: David Howells, npiggin, raven, autofs, linux-fsdevel

On Sat, Jan 15, 2011 at 3:24 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Jan 14, 2011 at 12:19:43PM +1100, Nick Piggin wrote:
>
>> Also, I don't like how you return a vfsmount * and use that in path walking. If
>> you just returned success and allowed the path walk to continue looking up
>> mounted directories, it would be cleaner code, IMO.
>
> No.  Filesystem has no business touching mount tree; leave that to core VFS.
> Analysis of what's going on is messy enough as it is, let's *NOT* complicate
> it even further.
>
> I think we should take do_add_mount() calls into follow_automount(), making
> the API
>        NULL => nothing to do here, just follow mount
>        ERR_PTR() => sod off
>        mnt => here's the thing I want mounted, do it and go there.  If
> something's allready got mounted here, just disregard this one and follow
> mount as usual (== we'd lost the race)

That would be fine. I just want the path walk to follow the mount in the
usual way, rather than following something that the filesystem hands back
directly.

David explains that was a micro optimisation to avoid the lookup, and that
the vfsmount should always be mounted at that point, which basically solved
my conceptual problem with it.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2011-01-14 22:14 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-11 17:48 [PATCH 00/18] Introduce automount support in the VFS David Howells
2011-01-11 17:48 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() David Howells
2011-01-13  3:53   ` Nick Piggin
2011-01-13  5:10     ` Nick Piggin
2011-01-13 15:12     ` David Howells
2011-01-14  1:19       ` Nick Piggin
2011-01-14 16:24         ` Al Viro
2011-01-14 22:14           ` Nick Piggin
2011-01-13 12:20   ` David Howells
2011-01-14  1:04     ` Nick Piggin
2011-01-14  9:24     ` David Howells
2011-01-11 17:48 ` [PATCH 02/18] AFS: Use d_automount() " David Howells
2011-01-11 17:48 ` [PATCH 03/18] NFS: " David Howells
2011-01-11 17:48 ` [PATCH 04/18] CIFS: " David Howells
2011-01-11 17:48 ` [PATCH 05/18] Remove the automount through follow_link() kludge code from pathwalk David Howells
2011-01-11 17:48 ` [PATCH 06/18] Add an AT_NO_AUTOMOUNT flag to suppress terminal automount David Howells
2011-01-11 17:48 ` [PATCH 07/18] Add more dentry flags for special function directories David Howells
2011-01-11 17:48 ` [PATCH 08/18] Make follow_down() handle d_manage() David Howells
2011-01-11 17:48 ` [PATCH 09/18] autofs4: Add d_automount() dentry operation David Howells
2011-01-11 17:49 ` [PATCH 10/18] autofs4: Add d_manage() " David Howells
2011-01-11 17:49 ` [PATCH 11/18] autofs4: Remove unused code David Howells
2011-01-11 17:49 ` [PATCH 12/18] autofs4: Clean up inode operations David Howells
2011-01-11 17:49 ` [PATCH 13/18] autofs4: Clean up dentry operations David Howells
2011-01-11 17:49 ` [PATCH 14/18] autofs4: Clean up autofs4_free_ino() David Howells
2011-01-11 17:49 ` [PATCH 15/18] autofs4: Fix wait validation David Howells
2011-01-11 17:49 ` [PATCH 16/18] autofs4: Add v4 pseudo direct mount support David Howells
2011-01-11 17:49 ` [PATCH 17/18] autofs4: Bump version David Howells
2011-01-11 17:49 ` [PATCH 18/18] Remove a further kludge from __do_follow_link() David Howells
2011-01-12 13:52 ` [PATCH 07/18] Add more dentry flags for special function directories [UPDATE] David Howells
2011-01-12 19:16 ` [PATCH 00/18] Introduce automount support in the VFS 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.