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


Hi Ian, Al,

I've done some updating to consolidate the patches and update the
documentation.  I've also added a patch to attempt to make d_manage() capable
of handling RCU-walk (can you test that Ian?).

The patches are committed to:

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

and should appear there eventually.

---
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 DCACHE_NEED_AUTOMOUNT (which is propagated from the
     S_AUTOMOUNT inode flag) is flagged on a dentry and if that 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.  ->d_automount() must have
     	 in some manner mounted this before returning.

     (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) With the final (and provisional) patch of the series, -ECHILD to ask
     	 the caller to break out of RCU-pathwalk mode (d_manage() acquires a
     	 third parameter which is true if it is being called in RCU-walk
     	 mode).

     (d) Another 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
follow_managed() 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 #4]
   - Rearranged and merged the patches a bit and updated the documentation,
     both that to be added into the kernel and the patch descriptions.
   - Looked at making d_manage() able to handle RCU-walk mode pathwalk.

 [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):
      Allow d_manage() to be used in RCU-walk mode
      Remove a further kludge from __do_follow_link()
      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()
      From: David Howells <dhowells@redhat.com>
      Add a dentry op to allow processes to be held during pathwalk transit
      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 |    3 
 Documentation/filesystems/vfs.txt |   38 ++
 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                 |  685 ++++++++++++++++---------------------
 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                       |    5 
 fs/namei.c                        |  307 +++++++++++++----
 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, 894 insertions(+), 695 deletions(-)


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

* [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-16  0:09   ` Al Viro
  2011-01-13 21:54 ` [PATCH 02/18] Add a dentry op to allow processes to be held during pathwalk transit " David Howells
                   ` (19 subsequent siblings)
  20 siblings, 1 reply; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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
dentry flag (DCACHE_NEED_AUTOMOUNT).

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

The ->d_automount() dentry operation:

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

takes a pointer to the directory to be mounted upon, which is expected to
provide sufficient data to determine what should be mounted.  If successful, it
should return the vfsmount struct it creates (which it should also have added
to the namespace using do_add_mount() or similar).  If there's a collision with
another automount attempt, NULL should be returned.  If the directory specified
by the parameter should be used directly rather than being mounted upon,
-EISDIR should be returned.  In any other case, an error code should be
returned.

The ->d_automount() operation is called with no locks held and may sleep.  At
this point the pathwalk algorithm will be in ref-walk mode.


Within fs/namei.c itself, a new pathwalk subroutine (follow_automount()) is
added to handle mountpoints.  It will return -EREMOTE if the automount flag 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.  The path will be updated to point to the mounted
filesystem if a successful automount took place.

__follow_mount() is replaced by follow_managed() which is more generic
(especially with the patch that adds ->d_manage()).  This handles transits from
directories during pathwalk, including automounting and skipping over
mountpoints (and holding processes with the next patch).

__follow_mount_rcu() will jump out of RCU-walk mode if it encounters an
automount point with nothing mounted on it.

follow_dotdot*() does not handle automounts as you don't want to trigger them
whilst following "..".

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.

I've also added an inode flag (S_AUTOMOUNT) so that filesystems can mark their
inodes as automount points.  This flag is automatically propagated to the
dentry as DCACHE_NEED_AUTOMOUNT by __d_instantiate().  This saves NFS and could
save AFS a private flag bit apiece, but is not strictly necessary.  It would be
preferable to do the propagation in d_set_d_op(), but that doesn't normally
have access to the inode.

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

 Documentation/filesystems/Locking |    2 
 Documentation/filesystems/vfs.txt |   14 +++
 fs/dcache.c                       |    5 +
 fs/namei.c                        |  205 +++++++++++++++++++++++++++++--------
 include/linux/dcache.h            |    7 +
 include/linux/fs.h                |    2 
 6 files changed, 187 insertions(+), 48 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 977d891..5f0c52a 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		yes		no
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index fbb324e..992cf74 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,19 @@ 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 DCACHE_NEED_AUTOMOUNT is set on the
+	dentry.  This is set by __d_instantiate() if S_AUTOMOUNT is set on the
+	inode being added.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
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 24ece10..f55c11a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -877,51 +877,120 @@ int follow_up(struct path *path)
 }
 
 /*
- * serialization is taken care of in namespace.c
+ * Perform an automount
+ * - return -EISDIR to tell follow_managed() 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)) {
+		/*
+		 * 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;
 
-static int __follow_mount(struct path *path)
-{
-	int res = 0;
-	while (d_mountpoint(path->dentry)) {
-		struct vfsmount *mounted = lookup_mnt(path);
-		if (!mounted)
-			break;
-		dput(path->dentry);
-		if (res)
-			mntput(path->mnt);
-		path->mnt = mounted;
-		path->dentry = dget(mounted->mnt_root);
-		res = 1;
+	if (mnt->mnt_sb == path->mnt->mnt_sb &&
+	    mnt->mnt_root == path->dentry) {
+		mntput(mnt);
+		return -ELOOP;
 	}
-	return res;
+
+	dput(path->dentry);
+	if (*need_mntput)
+		mntput(path->mnt);
+	path->mnt = mnt;
+	path->dentry = dget(mnt->mnt_root);
+	*need_mntput = true;
+	return 0;
 }
 
-static void follow_mount(struct path *path)
+/*
+ * Handle a dentry that is managed in some way.
+ * - Flagged as mountpoint
+ * - Flagged as automount point
+ *
+ * This may only be called in refwalk mode.
+ *
+ * Serialization is taken care of in namespace.c
+ */
+static int follow_managed(struct path *path, unsigned flags)
 {
-	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);
+	unsigned managed;
+	bool need_mntput = false;
+	int ret;
+
+	/* 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)) {
+		/* 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)
@@ -939,13 +1008,37 @@ int follow_down(struct path *path)
 	return 0;
 }
 
+/*
+ * Skip to top of mountpoint pile in rcuwalk mode.  We abort the rcu-walk if we
+ * meet an automount point and we're not walking to "..".  True is returned to
+ * continue, false to abort.
+ */
+static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
+			       struct inode **inode, bool reverse_transit)
+{
+	while (d_mountpoint(path->dentry)) {
+		struct vfsmount *mounted;
+		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
+		if (!mounted)
+			break;
+		path->mnt = mounted;
+		path->dentry = mounted->mnt_root;
+		nd->seq = read_seqcount_begin(&path->dentry->d_seq);
+		*inode = path->dentry->d_inode;
+	}
+
+	if (unlikely(path->dentry->d_flags & DCACHE_NEED_AUTOMOUNT))
+		return reverse_transit;
+	return true;
+}
+
 static int follow_dotdot_rcu(struct nameidata *nd)
 {
 	struct inode *inode = nd->inode;
 
 	set_root_rcu(nd);
 
-	while(1) {
+	while (1) {
 		if (nd->path.dentry == nd->root.dentry &&
 		    nd->path.mnt == nd->root.mnt) {
 			break;
@@ -968,12 +1061,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, true);
 	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);
@@ -1038,12 +1147,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;
 	}
@@ -1072,7 +1183,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, false)) &&
+		    nameidata_drop_rcu(nd))
+			return -ECHILD;
 	} else {
 		dentry = __d_lookup(parent, name);
 		if (!dentry)
@@ -1083,7 +1196,9 @@ found:
 done:
 		path->mnt = mnt;
 		path->dentry = dentry;
-		__follow_mount(path);
+		err = follow_managed(path, nd->flags);
+		if (unlikely(err < 0))
+			return err;
 		*inode = path->dentry->d_inode;
 	}
 	return 0;
@@ -2178,11 +2293,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_managed(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..ee6c26d 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;
 
 /*
@@ -205,13 +206,17 @@ 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_MANAGED_DENTRY \
+	(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT)
+
 extern seqlock_t rename_lock;
 
 static inline int dname_external(struct dentry *dentry)
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] 46+ messages in thread

* [PATCH 02/18] Add a dentry op to allow processes to be held during pathwalk transit [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
  2011-01-13 21:54 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 03/18] From: David Howells <dhowells@redhat.com> " David Howells
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +Cc: dhowells, autofs, linux-fsdevel

Add a dentry op (d_manage) to permit a filesystem to hold a process and make it
sleep when it tries to transit away from one of that filesystem's directories
during a pathwalk.  The operation is keyed off a new dentry flag
(DCACHE_MANAGE_TRANSIT).

The filesystem is allowed to be selective about which processes it holds and
which it permits to continue on or prohibits from transiting from each flagged
directory.  This will allow autofs to hold up client processes whilst letting
its userspace daemon through to maintain the directory or the stuff behind it
or mounted upon it.

The ->d_manage() dentry operation:

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

takes a pointer to the directory about to be transited away from and a flag
indicating whether the transit is undertaken by do_add_mount() or
do_move_mount() skipping through a pile of filesystems mounted on a mountpoint.

It should return 0 if successful and to let the process continue on its way;
-EISDIR to prohibit the caller from skipping to overmounted filesystems or
automounting, and to use this directory; or some other error code to return to
the user.

->d_manage() is called with namespace_sem writelocked if mounting_here is true
and no other locks held, so it may sleep.  However, if mounting_here is true,
it may not initiate or wait for a mount or unmount upon the parameter
directory, even if the act is actually performed by userspace.


Within fs/namei.c, follow_managed() is extended to check with d_manage() first
on each managed directory, before transiting away from it or attempting to
automount upon it.

follow_down() is renamed follow_down_one() and should only be used where the
filesystem deliberately intends to avoid management steps (e.g. autofs).

A new follow_down() is added that incorporates the loop done by all other
callers of follow_down() (do_add/move_mount(), autofs and NFSD; whilst AFS, NFS
and CIFS do use it, their use is removed by converting them to use
d_automount()).  The new follow_down() calls d_manage() as appropriate.  It
also takes an extra parameter to indicate if it is being called from mount code
(with namespace_sem writelocked) which it passes to d_manage().  follow_down()
ignores automount points so that it can be used to mount on them.

__follow_mount_rcu() is made to abort rcu-walk mode if it hits a directory with
DCACHE_MANAGE_TRANSIT set on the basis that we're probably going to have to
sleep.  It would be possible to enter d_manage() in rcu-walk mode too, and have
that determine whether to abort or not itself.  That would allow the autofs
daemon to continue on in rcu-walk mode.


Note that DCACHE_MANAGE_TRANSIT on a directory should be cleared when it isn't
required as every tranist from that directory will cause d_manage() to be
invoked.  It can always be set again when necessary.


==========================
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 almost no locks are held in d_manage() and none in
d_automount().

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

 Documentation/filesystems/Locking |    1 +
 Documentation/filesystems/vfs.txt |   21 ++++++++++-
 drivers/staging/autofs/dirhash.c  |    5 +--
 fs/afs/mntpt.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/cifs/cifs_dfs_ref.c            |    5 +--
 fs/namei.c                        |   72 ++++++++++++++++++++++++++++++++++++-
 fs/namespace.c                    |   14 ++++---
 fs/nfs/namespace.c                |    5 +--
 fs/nfsd/vfs.c                     |    5 ++-
 include/linux/dcache.h            |   11 +++++-
 include/linux/namei.h             |    3 +-
 15 files changed, 125 insertions(+), 50 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 5f0c52a..621ee98 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -31,6 +31,7 @@ d_release:	no		no		yes		no
 d_iput:		no		no		yes		no
 d_dname:	no		no		no		no
 d_automount:	no		no		yes		no
+d_manage:	no		no		yes		no
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 992cf74..3d68c2e 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 *, bool);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -938,12 +939,30 @@ struct dentry_operations {
 	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.
+	should be returned.  If -EISDIR is returned, then the directory will
+	be treated as an ordinary directory and returned to pathwalk to
+	continue walking.
 
 	This function is only used if DCACHE_NEED_AUTOMOUNT is set on the
 	dentry.  This is set by __d_instantiate() 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.  0 should be returned to let the
+	calling process continue.  -EISDIR can be returned to tell pathwalk to
+	use this directory as an ordinary directory and to ignore anything
+	mounted on it and not to check the automount flag.  Any other error
+	code will abort pathwalk completely.
+
+	If the 'mounting_here' parameter is true, then namespace_sem is being
+	held by the caller and the function should not initiate any mounts or
+	unmounts that it will then wait for.
+
+	This function is only used if DCACHE_MANAGE_TRANSIT is set on the
+	dentry being transited from.
+
 Example :
 
 static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
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/afs/mntpt.c b/fs/afs/mntpt.c
index 6153417..b8e7fb8 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -273,10 +273,7 @@ static void *afs_mntpt_follow_link(struct dentry *dentry, struct nameidata *nd)
 		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;
+		err = follow_down(&nd->path, false);
 	default:
 		mntput(newmnt);
 		break;
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/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index c68a056..83479cf 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -273,10 +273,7 @@ static int add_mount_helper(struct vfsmount *newmnt, struct nameidata *nd,
 		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;
+		err = follow_down(&nd->path, false);
 	default:
 		mntput(newmnt);
 		break;
diff --git a/fs/namei.c b/fs/namei.c
index f55c11a..e46a56b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -941,6 +941,7 @@ static int follow_automount(struct path *path, unsigned flags,
 
 /*
  * Handle a dentry that is managed in some way.
+ * - Flagged for transit management (autofs)
  * - Flagged as mountpoint
  * - Flagged as automount point
  *
@@ -960,6 +961,16 @@ static int follow_managed(struct path *path, unsigned flags)
 	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, false);
+			if (ret < 0)
+				return ret == -EISDIR ? 0 : ret;
+		}
+
 		/* Transit to a mounted filesystem. */
 		if (managed & DCACHE_MOUNTED) {
 			struct vfsmount *mounted = lookup_mnt(path);
@@ -993,7 +1004,7 @@ static int follow_managed(struct path *path, unsigned flags)
 	return 0;
 }
 
-int follow_down(struct path *path)
+int follow_down_one(struct path *path)
 {
 	struct vfsmount *mounted;
 
@@ -1010,14 +1021,19 @@ int follow_down(struct path *path)
 
 /*
  * Skip to top of mountpoint pile in rcuwalk mode.  We abort the rcu-walk if we
- * meet an automount point and we're not walking to "..".  True is returned to
+ * meet a managed dentry and we're not walking to "..".  True is returned to
  * continue, false to abort.
  */
 static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
 			       struct inode **inode, bool reverse_transit)
 {
+	unsigned abort_mask =
+		reverse_transit ? 0 : DCACHE_MANAGE_TRANSIT;
+
 	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)
 			break;
@@ -1068,6 +1084,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)
@@ -3504,6 +3571,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/nfs/namespace.c b/fs/nfs/namespace.c
index 74aaf39..bfcb933 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -176,10 +176,7 @@ out_err:
 	path_put(&nd->path);
 	goto out;
 out_follow:
-	while (d_mountpoint(nd->path.dentry) &&
-	       follow_down(&nd->path))
-		;
-	err = 0;
+	err = follow_down(&nd->path, false);
 	goto out;
 }
 
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 ee6c26d..946521d 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 *, bool);
 } ____cacheline_aligned;
 
 /*
@@ -214,8 +215,9 @@ struct dentry_operations {
 
 #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_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)
 
 extern seqlock_t rename_lock;
 
@@ -404,7 +406,12 @@ 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_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 18d06ad..8ef2c78 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -79,7 +79,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] 46+ messages in thread

* [PATCH 03/18] From: David Howells <dhowells@redhat.com> [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
  2011-01-13 21:54 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() " David Howells
  2011-01-13 21:54 ` [PATCH 02/18] Add a dentry op to allow processes to be held during pathwalk transit " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 04/18] AFS: Use d_automount() rather than abusing follow_link() " David Howells
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +Cc: dhowells, autofs, linux-fsdevel

Add an AT_NO_AUTOMOUNT flag to suppress terminal automount

Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of automount
point directories.  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 e46a56b..249d0f2 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 -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
 	 * 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 8ef2c78..f276d4f 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] 46+ messages in thread

* [PATCH 04/18] AFS: Use d_automount() rather than abusing follow_link() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (2 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 03/18] From: David Howells <dhowells@redhat.com> " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 05/18] NFS: " David Howells
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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    |   44 +++++++++++++++-----------------------------
 4 files changed, 19 insertions(+), 30 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 b8e7fb8..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,49 +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 */
-		err = follow_down(&nd->path, false);
+		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] 46+ messages in thread

* [PATCH 05/18] NFS: Use d_automount() rather than abusing follow_link() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (3 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 04/18] AFS: Use d_automount() rather than abusing follow_link() " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 06/18] CIFS: " David Howells
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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     |   84 +++++++++++++++++++++++-------------------------
 include/linux/nfs_fs.h |    1 -
 5 files changed, 46 insertions(+), 48 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index abe4f0c..39f6a42 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -971,7 +971,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 */
@@ -1174,6 +1174,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)
@@ -1249,6 +1250,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 ce00b70..d851242 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 bfa3a34..4644f04 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -252,6 +252,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 bfcb933..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,84 +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:
-	err = follow_down(&nd->path, false);
-	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] 46+ messages in thread

* [PATCH 06/18] CIFS: Use d_automount() rather than abusing follow_link() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (4 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 05/18] NFS: " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 07/18] Remove the automount through follow_link() kludge code from pathwalk " David Howells
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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 |  131 +++++++++++++++++++++++++-----------------------
 fs/cifs/cifsfs.h       |    6 ++
 fs/cifs/dir.c          |    2 +
 fs/cifs/inode.c        |    8 ++-
 4 files changed, 80 insertions(+), 67 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 83479cf..ddd0b3e 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -255,32 +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 */
-		err = follow_down(&nd->path, false);
-	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);
@@ -290,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;
 
@@ -338,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] 46+ messages in thread

* [PATCH 07/18] Remove the automount through follow_link() kludge code from pathwalk [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (5 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 06/18] CIFS: " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 08/18] autofs4: Add d_automount() dentry operation " David Howells
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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 249d0f2..f6a5e74 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1319,17 +1319,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.
@@ -1467,7 +1456,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);
@@ -2516,8 +2506,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] 46+ messages in thread

* [PATCH 08/18] autofs4: Add d_automount() dentry operation [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (6 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 07/18] Remove the automount through follow_link() kludge code from pathwalk " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 09/18] autofs4: Add d_manage() " David Howells
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (7 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 08/18] autofs4: Add d_automount() dentry operation " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-14 13:51   ` Ian Kent
  2011-01-13 21:54 ` [PATCH 10/18] autofs4: Remove unused code " David Howells
                   ` (11 subsequent siblings)
  20 siblings, 1 reply; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 10/18] autofs4: Remove unused code [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (8 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 09/18] autofs4: Add d_manage() " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:54 ` [PATCH 11/18] autofs4: Clean up inode operations " David Howells
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 11/18] autofs4: Clean up inode operations [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (9 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 10/18] autofs4: Remove unused code " David Howells
@ 2011-01-13 21:54 ` David Howells
  2011-01-13 21:55 ` [PATCH 12/18] autofs4: Clean up dentry " David Howells
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:54 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 12/18] autofs4: Clean up dentry operations [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (10 preceding siblings ...)
  2011-01-13 21:54 ` [PATCH 11/18] autofs4: Clean up inode operations " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-13 21:55 ` [PATCH 13/18] autofs4: Clean up autofs4_free_ino() " David Howells
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 13/18] autofs4: Clean up autofs4_free_ino() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (11 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 12/18] autofs4: Clean up dentry " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-14 16:03   ` Al Viro
  2011-01-13 21:55 ` [PATCH 14/18] autofs4: Fix wait validation " David Howells
                   ` (7 subsequent siblings)
  20 siblings, 1 reply; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 14/18] autofs4: Fix wait validation [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (12 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 13/18] autofs4: Clean up autofs4_free_ino() " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-13 21:55 ` [PATCH 15/18] autofs4: Add v4 pseudo direct mount support " David Howells
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 15/18] autofs4: Add v4 pseudo direct mount support [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (13 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 14/18] autofs4: Fix wait validation " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-13 21:55 ` [PATCH 16/18] autofs4: Bump version " David Howells
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 16/18] autofs4: Bump version [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (14 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 15/18] autofs4: Add v4 pseudo direct mount support " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-13 21:55 ` [PATCH 17/18] Remove a further kludge from __do_follow_link() " David Howells
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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] 46+ messages in thread

* [PATCH 17/18] Remove a further kludge from __do_follow_link() [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (15 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 16/18] autofs4: Bump version " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-13 21:55 ` [PATCH 18/18] Allow d_manage() to be used in RCU-walk mode " David Howells
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +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 f6a5e74..c983c92 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] 46+ messages in thread

* [PATCH 18/18] Allow d_manage() to be used in RCU-walk mode [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (16 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 17/18] Remove a further kludge from __do_follow_link() " David Howells
@ 2011-01-13 21:55 ` David Howells
  2011-01-14  7:02 ` [PATCH 00/18] Introduce automount support in the VFS " Al Viro
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-13 21:55 UTC (permalink / raw)
  To: viro, raven, npiggin; +Cc: dhowells, autofs, linux-fsdevel

Allow d_manage() to be called from pathwalk when it is in RCU-walk mode as well
as when it is in Ref-walk mode.  This permits __follow_mount_rcu() to call
d_manage() directly.  d_manage() needs a parameter to indicate that it is in
RCU-walk mode as it isn't allowed to sleep if in that mode (but should return
-ECHILD instead).

autofs4_d_manage() can then be set to retain RCU-walk mode if the daemon
accesses it and otherwise request dropping back to ref-walk mode.

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

 Documentation/filesystems/Locking |    2 +-
 Documentation/filesystems/vfs.txt |    7 ++++++-
 fs/autofs4/root.c                 |    8 ++++++--
 fs/namei.c                        |   14 +++++++-------
 include/linux/dcache.h            |    2 +-
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 621ee98..b3b0ac4 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -31,7 +31,7 @@ d_release:	no		no		yes		no
 d_iput:		no		no		yes		no
 d_dname:	no		no		no		no
 d_automount:	no		no		yes		no
-d_manage:	no		no		yes		no
+d_manage:	no		no		yes (ref-walk)	maybe
 
 --------------------------- inode_operations --------------------------- 
 prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 3d68c2e..9bc28c2f 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -865,7 +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 *, bool);
+	int (*d_manage)(struct path *, bool, bool);
 };
 
   d_revalidate: called when the VFS needs to revalidate a dentry. This
@@ -960,6 +960,11 @@ struct dentry_operations {
 	held by the caller and the function should not initiate any mounts or
 	unmounts that it will then wait for.
 
+	If the 'rcu_walk' parameter is true, then the caller is doing a
+	pathwalk in RCU-walk mode.  Sleeping is not permitted in this mode,
+	and the caller can be asked to leave it and call again by returing
+	-ECHILD.
+
 	This function is only used if DCACHE_MANAGE_TRANSIT is set on the
 	dentry being transited from.
 
diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 752693f..c46d6fb 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -36,7 +36,7 @@ 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 *);
 static struct vfsmount *autofs4_d_automount(struct path *);
-static int autofs4_d_manage(struct path *, bool);
+static int autofs4_d_manage(struct path *, bool, bool);
 
 const struct file_operations autofs4_root_operations = {
 	.open		= dcache_dir_open,
@@ -454,7 +454,7 @@ done:
 	return NULL;
 }
 
-int autofs4_d_manage(struct path *path, bool mounting_here)
+int autofs4_d_manage(struct path *path, bool mounting_here, bool rcu_walk)
 {
 	struct dentry *dentry = path->dentry;
 	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
@@ -469,6 +469,10 @@ int autofs4_d_manage(struct path *path, bool mounting_here)
 		return 0;
 	}
 
+	/* We need to sleep, so we need pathwalk to be in ref-mode */
+	if (rcu_walk)
+		return -ECHILD;
+
 	/* Wait for pending expires */
 	do_expire_wait(dentry);
 
diff --git a/fs/namei.c b/fs/namei.c
index c983c92..5c15daa 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -967,7 +967,7 @@ static int follow_managed(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, false);
+			ret = path->dentry->d_op->d_manage(path, false, false);
 			if (ret < 0)
 				return ret == -EISDIR ? 0 : ret;
 		}
@@ -1028,13 +1028,12 @@ int follow_down_one(struct path *path)
 static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
 			       struct inode **inode, bool reverse_transit)
 {
-	unsigned abort_mask =
-		reverse_transit ? 0 : DCACHE_MANAGE_TRANSIT;
-
 	while (d_mountpoint(path->dentry)) {
 		struct vfsmount *mounted;
-		if (path->dentry->d_flags & abort_mask)
-			return true;
+		if (unlikely(path->dentry->d_flags & DCACHE_MANAGE_TRANSIT) &&
+		    !reverse_transit &&
+		    path->dentry->d_op->d_manage(path, false, true) < 0)
+			return false;
 		mounted = __lookup_mnt(path->mnt, path->dentry, 1);
 		if (!mounted)
 			break;
@@ -1112,7 +1111,8 @@ int follow_down(struct path *path, bool mounting_here)
 		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);
+			ret = path->dentry->d_op->d_manage(path, mounting_here,
+							   false);
 			if (ret < 0)
 				return ret == -EISDIR ? 0 : ret;
 		}
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 946521d..236a384 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 *, bool);
+	int (*d_manage)(struct path *, bool, bool);
 } ____cacheline_aligned;
 
 /*


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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (17 preceding siblings ...)
  2011-01-13 21:55 ` [PATCH 18/18] Allow d_manage() to be used in RCU-walk mode " David Howells
@ 2011-01-14  7:02 ` Al Viro
  2011-01-14  7:05   ` Al Viro
  2011-01-14 11:20   ` David Howells
  2011-01-14 11:43 ` David Howells
  2011-01-14 11:54   ` David Howells
  20 siblings, 2 replies; 46+ messages in thread
From: Al Viro @ 2011-01-14  7:02 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Thu, Jan 13, 2011 at 09:53:59PM +0000, David Howells wrote:

>  (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 DCACHE_NEED_AUTOMOUNT (which is propagated from the
>      S_AUTOMOUNT inode flag) is flagged on a dentry and if that 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.  ->d_automount() must have
>      	 in some manner mounted this before returning.
> 
>      (b) NULL if something was already mounted there, in which case pathwalk
>      	 will loop around and recheck the mountings.

That makes very little sense as-is.  Look:
	* autofs4 never does (a)
	* everybody else could replace (a) with (b) just fine - we do (a)
only when we'd just mounted new vfsmount on top of path.  So (b) would lead
to follow_managed() looping over, finding DCACHE_MOUNTED and cheerfully
transiting into the root of that vfsmount.

Note that it's not subtle at all - *all* cases doing (a) lack ->d_manage(),
so behaviour of follow_automount() is as simple as it gets.

Now, I'd like to have (a) and (b) distinct, but not in that fashion.  Namely,
let's take do_add_mount() et.al. into follow_automount().  Leave autofs4
as in your series; it'll be completely unaffected.  But switch all (b) in
nfs/cifs/afs over to modified (a).  That is,
	* have vfsmount created as it's done in your series
	* grab extra reference and put it on chosen list.  That'd be done
by helper in fs/namespace.c under namespace_sem.  Extra ref would make sure
that nobody walking the list would decide that it's expirable.
	* schedule whatever expiry activity we currently do.
	* return vfsmount
In follow_automount() we'd see that we have non-NULL and non-ERR_PTR.  Then
we'd attempt do_add_mount(), without bothering to pass it expiry list.  And
do the same checks for return value, etc. we currently do in the method
instances; just remember that we have an extra vfsmount reference that will
need to be dropped and that we'll need to take the sucker off the expiry list
in case we decide we don't need it (again, namespace.c helper).

As the result, we stop abusing do_add_mount() in there.  Moreover, with
pending mnt_devname nfs rework we will be able to get rid of passing
vfsmount to ->d_automount(), AFAICT, which would be nice...

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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-14  7:02 ` [PATCH 00/18] Introduce automount support in the VFS " Al Viro
@ 2011-01-14  7:05   ` Al Viro
  2011-01-14 11:20   ` David Howells
  1 sibling, 0 replies; 46+ messages in thread
From: Al Viro @ 2011-01-14  7:05 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Fri, Jan 14, 2011 at 07:02:24AM +0000, Al Viro wrote:

> As the result, we stop abusing do_add_mount() in there.  Moreover, with
> pending mnt_devname nfs rework we will be able to get rid of passing
> vfsmount to ->d_automount(), AFAICT, which would be nice...

BTW, what do you need vfsmount for in case of the only ->d_manage() instance
you've got?

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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-14  7:02 ` [PATCH 00/18] Introduce automount support in the VFS " Al Viro
  2011-01-14  7:05   ` Al Viro
@ 2011-01-14 11:20   ` David Howells
  1 sibling, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-14 11:20 UTC (permalink / raw)
  To: Al Viro; +Cc: dhowells, raven, npiggin, autofs, linux-fsdevel

Al Viro <viro@ZenIV.linux.org.uk> wrote:

> > As the result, we stop abusing do_add_mount() in there.  Moreover, with
> > pending mnt_devname nfs rework we will be able to get rid of passing
> > vfsmount to ->d_automount(), AFAICT, which would be nice...
> 
> BTW, what do you need vfsmount for in case of the only ->d_manage() instance
> you've got?

I'm not sure it's strictly necessary.  However, is it or will it be possible
for autofs to have per-namespace daemons?  I suspect I can probably downgrade
the path pointer to a dentry pointer.  It can always be upgraded later if we
find a need for it...

David

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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
                   ` (18 preceding siblings ...)
  2011-01-14  7:02 ` [PATCH 00/18] Introduce automount support in the VFS " Al Viro
@ 2011-01-14 11:43 ` David Howells
  2011-01-14 15:46   ` Al Viro
                     ` (2 more replies)
  2011-01-14 11:54   ` David Howells
  20 siblings, 3 replies; 46+ messages in thread
From: David Howells @ 2011-01-14 11:43 UTC (permalink / raw)
  To: Al Viro; +Cc: dhowells, raven, npiggin, autofs, linux-fsdevel

Al Viro <viro@ZenIV.linux.org.uk> wrote:

> >      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.  ->d_automount() must
> >          have in some manner mounted this before returning.
> > 
> >      (b) NULL if something was already mounted there, in which case
> >          pathwalk will loop around and recheck the mountings.
> 
> That makes very little sense as-is.  Look:
> 	* autofs4 never does (a)
> 	* everybody else could replace (a) with (b) just fine - we do (a)
> only when we'd just mounted new vfsmount on top of path.  So (b) would lead
> to follow_managed() looping over, finding DCACHE_MOUNTED and cheerfully
> transiting into the root of that vfsmount.

Indeed.  It's merely an optimisation and not strictly necessary.  I suppose
that, given the amount of time that's probably spent in performing the mount
part of the automount, repeating the check is negligible cost.

> Now, I'd like to have (a) and (b) distinct, but not in that fashion.  Namely,
> let's take do_add_mount() et.al. into follow_automount().

I presume that people aren't expected to do things like do_move_mount() here,
but might they want to do a bind mount?  Or do we just say if they want to do
something more exotic than do_add_mount(), they have to follow path (b)?

> Leave autofs4 as in your series; it'll be completely unaffected.  But switch
> all (b) in nfs/cifs/afs over to modified (a).  That is,
> 	* have vfsmount created as it's done in your series
> 	* grab extra reference and put it on chosen list.  That'd be done
> by helper in fs/namespace.c under namespace_sem.  Extra ref would make sure
> that nobody walking the list would decide that it's expirable.

It would be simpler, perhaps, to allow d_automount() to return the list also:

   struct vfsmount *(*d_automount)(struct path *mountpoint,
				   struct list_head **expiry_list_to_use);

This pointer can then be passed directly to do_add_mount() and we don't have
to worry about having an extra reference or cleaning up the list on error.

> 	* schedule whatever expiry activity we currently do.
> 	* return vfsmount
> In follow_automount() we'd see that we have non-NULL and non-ERR_PTR.  Then
> we'd attempt do_add_mount(), without bothering to pass it expiry list.  And
> do the same checks for return value, etc. we currently do in the method
> instances; just remember that we have an extra vfsmount reference that will
> need to be dropped and that we'll need to take the sucker off the expiry list
> in case we decide we don't need it (again, namespace.c helper).
> 
> As the result, we stop abusing do_add_mount() in there.  Moreover, with
> pending mnt_devname nfs rework we will be able to get rid of passing
> vfsmount to ->d_automount(), AFAICT, which would be nice...

:-)

David

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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
@ 2011-01-14 11:54   ` David Howells
  2011-01-13 21:54 ` [PATCH 02/18] Add a dentry op to allow processes to be held during pathwalk transit " David Howells
                     ` (19 subsequent siblings)
  20 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-14 11:54 UTC (permalink / raw)
  Cc: dhowells, Al Viro, raven, npiggin, autofs, linux-fsdevel

David Howells <dhowells@redhat.com> wrote:

> It would be simpler, perhaps, to allow d_automount() to return the list also:
> 
>    struct vfsmount *(*d_automount)(struct path *mountpoint,
> 				   struct list_head **expiry_list_to_use);
> 
> This pointer can then be passed directly to do_add_mount() and we don't have
> to worry about having an extra reference or cleaning up the list on error.

However, that isn't good enough as the filesystem may also need to start up
the time-based expirer, which in the case of AFS, NFS and CIFS doesn't repeat
if the list is empty - so if the mounting process gets preempted...

David

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

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
@ 2011-01-14 11:54   ` David Howells
  0 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-14 11:54 UTC (permalink / raw)
  Cc: dhowells, Al Viro, raven, npiggin, autofs, linux-fsdevel

David Howells <dhowells@redhat.com> wrote:

> It would be simpler, perhaps, to allow d_automount() to return the list also:
> 
>    struct vfsmount *(*d_automount)(struct path *mountpoint,
> 				   struct list_head **expiry_list_to_use);
> 
> This pointer can then be passed directly to do_add_mount() and we don't have
> to worry about having an extra reference or cleaning up the list on error.

However, that isn't good enough as the filesystem may also need to start up
the time-based expirer, which in the case of AFS, NFS and CIFS doesn't repeat
if the list is empty - so if the mounting process gets preempted...

David

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

* Re: [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-13 21:54 ` [PATCH 09/18] autofs4: Add d_manage() " David Howells
@ 2011-01-14 13:51   ` Ian Kent
  2011-01-14 14:37     ` Nick Piggin
  2011-01-14 15:35     ` David Howells
  0 siblings, 2 replies; 46+ messages in thread
From: Ian Kent @ 2011-01-14 13:51 UTC (permalink / raw)
  To: David Howells; +Cc: viro, npiggin, autofs, linux-fsdevel

On Thu, 2011-01-13 at 21:54 +0000, David Howells wrote:
> 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);

In this case I think the dcache_lock needs to be deleted and the d_lock
moved out of the if to protect the d_subdirs access.

Ian



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

* Re: [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-14 13:51   ` Ian Kent
@ 2011-01-14 14:37     ` Nick Piggin
  2011-01-14 15:47       ` Nick Piggin
  2011-01-14 15:35     ` David Howells
  1 sibling, 1 reply; 46+ messages in thread
From: Nick Piggin @ 2011-01-14 14:37 UTC (permalink / raw)
  To: Ian Kent; +Cc: David Howells, viro, npiggin, autofs, linux-fsdevel

On Sat, Jan 15, 2011 at 12:51 AM, Ian Kent <raven@themaw.net> wrote:
> On Thu, 2011-01-13 at 21:54 +0000, David Howells wrote:
>> From: Ian Kent <raven@themaw.net>
>> +     //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);
>
> In this case I think the dcache_lock needs to be deleted and the d_lock
> moved out of the if to protect the d_subdirs access.

Right. If you follow the vfs-scale-working git branch series of
patches leading up to dcache_lock removal, it gives a pretty
good template of how to convert old dcache_lock using code
to new locking.

Although you can also just look at locking in fs/dcache.c and
convert code from that.

Any time you are dealing with just a *single* dentry, then
->d_lock would be enough to replace dcache_lock (it
actually protects more than dcache_lock alone did).
--
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] 46+ messages in thread

* Re: [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-14 13:51   ` Ian Kent
  2011-01-14 14:37     ` Nick Piggin
@ 2011-01-14 15:35     ` David Howells
  2011-01-14 15:46       ` Nick Piggin
  1 sibling, 1 reply; 46+ messages in thread
From: David Howells @ 2011-01-14 15:35 UTC (permalink / raw)
  To: Nick Piggin; +Cc: dhowells, Ian Kent, viro, npiggin, autofs, linux-fsdevel

Nick Piggin <npiggin@gmail.com> wrote:

> > On Thu, 2011-01-13 at 21:54 +0000, David Howells wrote:
> >> From: Ian Kent <raven@themaw.net>
> >> +     //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);
> >
> > In this case I think the dcache_lock needs to be deleted and the d_lock
> > moved out of the if to protect the d_subdirs access.
> 
> Right. If you follow the vfs-scale-working git branch series of
> patches leading up to dcache_lock removal, it gives a pretty
> good template of how to convert old dcache_lock using code
> to new locking.
> 
> Although you can also just look at locking in fs/dcache.c and
> convert code from that.
> 
> Any time you are dealing with just a *single* dentry, then
> ->d_lock would be enough to replace dcache_lock (it
> actually protects more than dcache_lock alone did).

Does it make sense to leave the lock where it is and repeat the outer test
after we've taken the lock?

David

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

* Re: [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-14 15:35     ` David Howells
@ 2011-01-14 15:46       ` Nick Piggin
  0 siblings, 0 replies; 46+ messages in thread
From: Nick Piggin @ 2011-01-14 15:46 UTC (permalink / raw)
  To: David Howells; +Cc: Ian Kent, viro, npiggin, autofs, linux-fsdevel

On Sat, Jan 15, 2011 at 2:35 AM, David Howells <dhowells@redhat.com> wrote:
> Nick Piggin <npiggin@gmail.com> wrote:
>
>> > On Thu, 2011-01-13 at 21:54 +0000, David Howells wrote:
>> >> From: Ian Kent <raven@themaw.net>
>> >> +     //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);
>> >
>> > In this case I think the dcache_lock needs to be deleted and the d_lock
>> > moved out of the if to protect the d_subdirs access.
>>
>> Right. If you follow the vfs-scale-working git branch series of
>> patches leading up to dcache_lock removal, it gives a pretty
>> good template of how to convert old dcache_lock using code
>> to new locking.
>>
>> Although you can also just look at locking in fs/dcache.c and
>> convert code from that.
>>
>> Any time you are dealing with just a *single* dentry, then
>> ->d_lock would be enough to replace dcache_lock (it
>> actually protects more than dcache_lock alone did).
>
> Does it make sense to leave the lock where it is and repeat the outer test
> after we've taken the lock?

I'll make the usual suggestion to make the locking simple, and
even avoiding all unlocked-load-then-recheck type of access,
unless there is a good reason to need it.

Many cases of ordering bugs I've found are due to these
seemingly simple and obvious optimisations.

It's up to you of course, is it worth having to think a little bit
harder about? Ian and yourself could probably answer that
better than me.
--
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] 46+ messages in thread

* Re: [PATCH 00/18] Introduce automount support in the VFS [ver #4]
  2011-01-14 11:43 ` David Howells
@ 2011-01-14 15:46   ` Al Viro
  2011-01-14 17:26   ` [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount() David Howells
  2011-01-14 17:30   ` David Howells
  2 siblings, 0 replies; 46+ messages in thread
From: Al Viro @ 2011-01-14 15:46 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Fri, Jan 14, 2011 at 11:43:48AM +0000, David Howells wrote:

> I presume that people aren't expected to do things like do_move_mount() here,
> but might they want to do a bind mount?  Or do we just say if they want to do
> something more exotic than do_add_mount(), they have to follow path (b)?

bind can be done just fine - all you need is to clone the vfsmount (or subtree)
you want to bind there and return the sucker.

> It would be simpler, perhaps, to allow d_automount() to return the list also:
> 
>    struct vfsmount *(*d_automount)(struct path *mountpoint,
> 				   struct list_head **expiry_list_to_use);
> 
> This pointer can then be passed directly to do_add_mount() and we don't have
> to worry about having an extra reference or cleaning up the list on error.

Let's not.  For one thing, getting rid of that argument in do_add_mount() is
a Good Thing(tm).  For another, I don't see any problem with having
	mnt_set_expiry(mnt, list)
and
	mnt_clear_expiry(mnt)
in fs/namespace.c.  Moreover, as you've mentioned in another followup, we
want the list to be non-empty until we either put the vfsmount in place
or decide to discard it, since we don't want fs to decide that the list is
empty and no expiry runs need to be scheduled anymore.

One more thing: do_add_mount() would happily get unexported and go to
fs/internal.h, which is also a good thing; it's too easy to abuse.

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

* Re: [PATCH 09/18] autofs4: Add d_manage() dentry operation [ver #4]
  2011-01-14 14:37     ` Nick Piggin
@ 2011-01-14 15:47       ` Nick Piggin
  0 siblings, 0 replies; 46+ messages in thread
From: Nick Piggin @ 2011-01-14 15:47 UTC (permalink / raw)
  To: Ian Kent; +Cc: David Howells, viro, npiggin, autofs, linux-fsdevel

On Sat, Jan 15, 2011 at 1:37 AM, Nick Piggin <npiggin@gmail.com> wrote:
> Although you can also just look at locking in fs/dcache.c and
> convert code from that.

Sorry, when I say this I mean locking *documentation* there. If anything
is unclear or unobvious, let me know and I'll try to improve documentation.

Thanks,
Nick

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

* Re: [PATCH 13/18] autofs4: Clean up autofs4_free_ino() [ver #4]
  2011-01-13 21:55 ` [PATCH 13/18] autofs4: Clean up autofs4_free_ino() " David Howells
@ 2011-01-14 16:03   ` Al Viro
  0 siblings, 0 replies; 46+ messages in thread
From: Al Viro @ 2011-01-14 16:03 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Thu, Jan 13, 2011 at 09:55:09PM +0000, David Howells wrote:
> 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>

BTW, you might want to look at pending stuff in #autofs in vfs-2.6.git.
A lot will probably overlap, but...

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

* [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 11:43 ` David Howells
  2011-01-14 15:46   ` Al Viro
@ 2011-01-14 17:26   ` David Howells
  2011-01-14 17:43     ` Al Viro
  2011-01-14 17:30   ` David Howells
  2 siblings, 1 reply; 46+ messages in thread
From: David Howells @ 2011-01-14 17:26 UTC (permalink / raw)
  To: Al Viro; +Cc: dhowells, raven, npiggin, autofs, linux-fsdevel


Unexport do_add_mount() and make ->d_automount() return the vfsmount to be
added rather than calling do_add_mount() itself.  follow_automount() will then
do the addition.

This slightly complicates things as ->d_automount() normally wants to add the
new vfsmount to an expiration list and start an expiration timer.  The problem
with that is that the vfsmount will be deleted if it has a refcount of 1 and
the timer will not repeat if the expiration list is empty.

To this end, we require the vfsmount to be returned from d_automount() with a
refcount of (at least) 2.  One of these refs will be dropped unconditionally.
In addition, follow_automount() must get a 3rd ref around the call to
do_add_mount() lest it eat a ref and return an error, leaving the mount we
have open to being expired as we would otherwise have only 1 ref on it.  This
would mean the currently upstream code is buggy for AFS, CIFS and NFS.

d_automount() should also add the the vfsmount to the expiration list (by
calling mnt_set_expiry()) and start the expiration timer before returning, if
this mechanism is to be used.  The vfsmount will be unlinked from the
expiration list by follow_automount() if do_add_mount() fails.

This patch also fixes the call to do_add_mount() for AFS and CIFS to propagate
the mount flags from the parent vfsmount.

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

 Documentation/filesystems/vfs.txt |   23 ++++++++++++--------
 fs/afs/mntpt.c                    |   25 +++++-----------------
 fs/cifs/cifs_dfs_ref.c            |   26 +++++------------------
 fs/internal.h                     |    2 ++
 fs/namei.c                        |   42 +++++++++++++++++++++++++++++++------
 fs/namespace.c                    |   41 +++++++++++++++++++++++++++++-------
 fs/nfs/namespace.c                |   24 ++++-----------------
 include/linux/mount.h             |    7 +-----
 8 files changed, 101 insertions(+), 89 deletions(-)


diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 3c4b2f1..94cf97b 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -933,15 +933,20 @@ struct dentry_operations {
 	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.  If -EISDIR is returned, then the directory will
-	be treated as an ordinary directory and returned to pathwalk to
-	continue walking.
+	This should create a new VFS mount record 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 vfsmount creation failed, then an error code should be returned.
+	If -EISDIR is returned, then the directory will be treated as an
+	ordinary directory and returned to pathwalk to continue walking.
+
+	If a vfsmount is returned, the caller will attempt to mount it on the
+	mountpoint and will remove the vfsmount from its expiration list in
+	the case of failure.  The vfsmount should be returned with 2 refs on
+	it to prevent automatic expiration - the caller will clean up the
+	additional ref.
 
 	This function is only used if DCACHE_NEED_AUTOMOUNT is set on the
 	dentry.  This is set by __d_instantiate() if S_AUTOMOUNT is set on the
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 0f7dd7a..0d74c2c 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -241,7 +241,6 @@ error_no_devname:
 struct vfsmount *afs_d_automount(struct path *path)
 {
 	struct vfsmount *newmnt;
-	int err;
 
 	_enter("{%s,%s}", path->mnt->mnt_devname, path->dentry->d_name.name);
 
@@ -249,24 +248,12 @@ struct vfsmount *afs_d_automount(struct path *path)
 	if (IS_ERR(newmnt))
 		return newmnt;
 
-	mntget(newmnt);
-	err = do_add_mount(newmnt, path, MNT_SHRINKABLE, &afs_vfsmounts);
-	switch (err) {
-	case 0:
-		schedule_delayed_work(&afs_mntpt_expiry_timer,
-				      afs_mntpt_expiry_timeout * HZ);
-		_leave(" = %p {%s}", newmnt, newmnt->mnt_devname);
-		return newmnt;
-	case -EBUSY:
-		/* someone else made a mount here whilst we were busy */
-		mntput(newmnt);
-		_leave(" = NULL [EBUSY]");
-		return NULL;
-	default:
-		mntput(newmnt);
-		_leave(" = %d", err);
-		return ERR_PTR(err);
-	}
+	mntget(newmnt); /* prevent immediate expiration */
+	mnt_set_expiry(newmnt, &afs_vfsmounts);
+	schedule_delayed_work(&afs_mntpt_expiry_timer,
+			      afs_mntpt_expiry_timeout * HZ);
+	_leave(" = %p {%s}", newmnt, newmnt->mnt_devname);
+	return newmnt;
 }
 
 /*
diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index ddd0b3e..7ed3653 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -351,7 +351,6 @@ free_xid:
 struct vfsmount *cifs_dfs_d_automount(struct path *path)
 {
 	struct vfsmount *newmnt;
-	int err;
 
 	cFYI(1, "in %s", __func__);
 
@@ -361,25 +360,12 @@ struct vfsmount *cifs_dfs_d_automount(struct path *path)
 		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);
-	}
+	mntget(newmnt); /* prevent immediate expiration */
+	mnt_set_expiry(newmnt, &cifs_dfs_automount_list);
+	schedule_delayed_work(&cifs_dfs_automount_task,
+			      cifs_dfs_mountpoint_expiry_timeout);
+	cFYI(1, "leaving %s [ok]" , __func__);
+	return newmnt;
 }
 
 const struct inode_operations cifs_dfs_referral_inode_operations = {
diff --git a/fs/internal.h b/fs/internal.h
index 9687c2e..4931060 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -70,6 +70,8 @@ extern void mnt_set_mountpoint(struct vfsmount *, struct dentry *,
 extern void release_mounts(struct list_head *);
 extern void umount_tree(struct vfsmount *, int, struct list_head *);
 extern struct vfsmount *copy_tree(struct vfsmount *, struct dentry *, int);
+extern int do_add_mount(struct vfsmount *, struct path *, int);
+extern void mnt_clear_expiry(struct vfsmount *);
 
 extern void __init mnt_init(void);
 
diff --git a/fs/namei.c b/fs/namei.c
index b099541..cd7b7e4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -898,6 +898,7 @@ static int follow_automount(struct path *path, unsigned flags,
 			    bool *need_mntput)
 {
 	struct vfsmount *mnt;
+	int err;
 
 	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
 		return -EREMOTE;
@@ -940,22 +941,49 @@ static int follow_automount(struct path *path, unsigned flags,
 			return -EREMOTE;
 		return PTR_ERR(mnt);
 	}
+
 	if (!mnt) /* mount collision */
 		return 0;
 
+	/* The new mount record should have at least 2 refs to prevent it being
+	 * expired before we get a chance to add it
+	 */
+	BUG_ON(mnt_get_count(mnt) < 2);
+
 	if (mnt->mnt_sb == path->mnt->mnt_sb &&
 	    mnt->mnt_root == path->dentry) {
+		mnt_clear_expiry(mnt);
+		mntput(mnt);
 		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;
+	/* We need to add the mountpoint to the parent.  The filesystem may
+	 * have placed it on an expiry list, and so we need to make sure it
+	 * won't be expired under us if do_add_mount() fails (do_add_mount()
+	 * will eat a reference unconditionally).
+	 */
+	mntget(mnt);
+	err = do_add_mount(mnt, path, path->mnt->mnt_flags | MNT_SHRINKABLE);
+	switch (err) {
+	case -EBUSY:
+		/* Someone else made a mount here whilst we were busy */
+		err = 0;
+	default:
+		mnt_clear_expiry(mnt);
+		mntput(mnt);
+		mntput(mnt);
+		return err;
+	case 0:
+		mntput(mnt);
+		dput(path->dentry);
+		if (*need_mntput)
+			mntput(path->mnt);
+		path->mnt = mnt;
+		path->dentry = dget(mnt->mnt_root);
+		*need_mntput = true;
+		return 0;
+	}
 }
 
 /*
diff --git a/fs/namespace.c b/fs/namespace.c
index d94ccd6..bfcb701 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1925,15 +1925,14 @@ static int do_new_mount(struct path *path, char *type, int flags,
 	if (IS_ERR(mnt))
 		return PTR_ERR(mnt);
 
-	return do_add_mount(mnt, path, mnt_flags, NULL);
+	return do_add_mount(mnt, path, mnt_flags);
 }
 
 /*
  * add a mount into a namespace's mount tree
- * - provide the option of adding the new mount to an expiration list
+ * - this unconditionally eats one of the caller's references to newmnt.
  */
-int do_add_mount(struct vfsmount *newmnt, struct path *path,
-		 int mnt_flags, struct list_head *fslist)
+int do_add_mount(struct vfsmount *newmnt, struct path *path, int mnt_flags)
 {
 	int err;
 
@@ -1963,9 +1962,6 @@ int do_add_mount(struct vfsmount *newmnt, struct path *path,
 	if ((err = graft_tree(newmnt, path)))
 		goto unlock;
 
-	if (fslist) /* add to the specified expiration list */
-		list_add_tail(&newmnt->mnt_expire, fslist);
-
 	up_write(&namespace_sem);
 	return 0;
 
@@ -1975,7 +1971,36 @@ unlock:
 	return err;
 }
 
-EXPORT_SYMBOL_GPL(do_add_mount);
+/**
+ * mnt_set_expiry - Put a mount on an expiration list
+ * @mnt: The mount to list.
+ * @expiry_list: The list to add the mount to.
+ */
+void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list)
+{
+	down_write(&namespace_sem);
+	br_write_lock(vfsmount_lock);
+
+	list_add_tail(&mnt->mnt_expire, expiry_list);
+
+	br_write_unlock(vfsmount_lock);
+	up_write(&namespace_sem);
+}
+EXPORT_SYMBOL(mnt_set_expiry);
+
+/*
+ * Remove a vfsmount from any expiration list it may be on
+ */
+void mnt_clear_expiry(struct vfsmount *mnt)
+{
+	if (!list_empty(&mnt->mnt_expire)) {
+		down_write(&namespace_sem);
+		br_write_lock(vfsmount_lock);
+		list_del_init(&mnt->mnt_expire);
+		br_write_unlock(vfsmount_lock);
+		up_write(&namespace_sem);
+	}
+}
 
 /*
  * process a list of expirable mountpoints with the intent of discarding any
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index f3fbb1b..f32b860 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -149,26 +149,10 @@ struct vfsmount *nfs_d_automount(struct path *path)
 	if (IS_ERR(mnt))
 		goto out;
 
-	mntget(mnt);
-	err = do_add_mount(mnt, path, path->mnt->mnt_flags | MNT_SHRINKABLE,
-			   &nfs_automount_list);
-	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);
-		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;
-	}
+	dprintk("%s: done, success\n", __func__);
+	mntget(mnt); /* prevent immediate expiration */
+	mnt_set_expiry(mnt, &nfs_automount_list);
+	schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout);
 
 out:
 	nfs_free_fattr(fattr);
diff --git a/include/linux/mount.h b/include/linux/mount.h
index 1869ea2..af4765e 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -110,12 +110,7 @@ extern struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 				      int flags, const char *name,
 				      void *data);
 
-struct nameidata;
-
-struct path;
-extern int do_add_mount(struct vfsmount *newmnt, struct path *path,
-			int mnt_flags, struct list_head *fslist);
-
+extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list);
 extern void mark_mounts_for_expiry(struct list_head *mounts);
 
 extern dev_t name_to_dev_t(char *name);

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 11:43 ` David Howells
  2011-01-14 15:46   ` Al Viro
  2011-01-14 17:26   ` [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount() David Howells
@ 2011-01-14 17:30   ` David Howells
  2 siblings, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-14 17:30 UTC (permalink / raw)
  To: Al Viro; +Cc: dhowells, raven, npiggin, autofs, linux-fsdevel

David Howells <dhowells@redhat.com> wrote:

> This would mean the currently upstream code is buggy for AFS, CIFS and NFS.

Actually, no it wouldn't, since do_add_mount() is what adds the mount to the
expiration list.

David

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 17:26   ` [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount() David Howells
@ 2011-01-14 17:43     ` Al Viro
  2011-01-14 17:56       ` Al Viro
  0 siblings, 1 reply; 46+ messages in thread
From: Al Viro @ 2011-01-14 17:43 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Fri, Jan 14, 2011 at 05:26:09PM +0000, David Howells wrote:
> 
> Unexport do_add_mount() and make ->d_automount() return the vfsmount to be
> added rather than calling do_add_mount() itself.  follow_automount() will then
> do the addition.
> 
> This slightly complicates things as ->d_automount() normally wants to add the
> new vfsmount to an expiration list and start an expiration timer.  The problem
> with that is that the vfsmount will be deleted if it has a refcount of 1 and
> the timer will not repeat if the expiration list is empty.
> 
> To this end, we require the vfsmount to be returned from d_automount() with a
> refcount of (at least) 2.  One of these refs will be dropped unconditionally.
> In addition, follow_automount() must get a 3rd ref around the call to
> do_add_mount() lest it eat a ref and return an error, leaving the mount we
> have open to being expired as we would otherwise have only 1 ref on it.

Yechh...  I'd rather take mntput_long() (yuck) out into callers (all two
of them) and lose the extra mntget/mntput in follow_automount().

> This patch also fixes the call to do_add_mount() for AFS and CIFS to propagate
> the mount flags from the parent vfsmount.

CIFS actually used to be OK; that breakage was introduced earlier in the
series.  AFS was broken wrt flags.  I'll fix the CIFS patch and adjust
that one accordingly.


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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 17:43     ` Al Viro
@ 2011-01-14 17:56       ` Al Viro
  2011-01-14 18:06         ` Al Viro
  0 siblings, 1 reply; 46+ messages in thread
From: Al Viro @ 2011-01-14 17:56 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

	BTW, speaking of mntput_long(): I really hate that API.  It's
asking for subtle leaks - use mntget_long() in pair with mntput() and
you are fucked.  Worse, these suckers are created with long reference
now, so any code that used to use mntput() to kill a cloned/freshly
created vfsmount got silently b0rken.  A quick look already caught
one such case - fs/nfsctl.c do_open() uses mntput() in pair with
do_kern_mount(), which leads to longrefs left at 1.

	Rationale for that mess, please...

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 17:56       ` Al Viro
@ 2011-01-14 18:06         ` Al Viro
  2011-01-14 22:07           ` Nick Piggin
  0 siblings, 1 reply; 46+ messages in thread
From: Al Viro @ 2011-01-14 18:06 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Fri, Jan 14, 2011 at 05:56:48PM +0000, Al Viro wrote:
> 	BTW, speaking of mntput_long(): I really hate that API.  It's
> asking for subtle leaks - use mntget_long() in pair with mntput() and
> you are fucked.  Worse, these suckers are created with long reference
> now, so any code that used to use mntput() to kill a cloned/freshly
> created vfsmount got silently b0rken.  A quick look already caught
> one such case - fs/nfsctl.c do_open() uses mntput() in pair with
> do_kern_mount(), which leads to longrefs left at 1.
> 
> 	Rationale for that mess, please...

FWIW, what I intend to do is to keep these longrefs _only_ for
cwd/root/attached or possibly hashed and set them alongside the
normal ref.  I.e. require the callers of mntput_long() (not exported,
local to core VFS) to keep the normal reference.

That way it would turn into hint for mntput() - "we have persistent
refs, just decrement count on this CPU and piss off", with mntput_long()
never going into the whole "and now we are dropping the last ref" part.

AFAICS, it keeps your write-free objectives and gets much saner API.
Shout if you have problems with that...

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 18:06         ` Al Viro
@ 2011-01-14 22:07           ` Nick Piggin
  2011-01-15 13:30             ` Al Viro
  0 siblings, 1 reply; 46+ messages in thread
From: Nick Piggin @ 2011-01-14 22:07 UTC (permalink / raw)
  To: Al Viro
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Sat, Jan 15, 2011 at 5:06 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Jan 14, 2011 at 05:56:48PM +0000, Al Viro wrote:
>>       BTW, speaking of mntput_long(): I really hate that API.  It's
>> asking for subtle leaks - use mntget_long() in pair with mntput() and
>> you are fucked.  Worse, these suckers are created with long reference
>> now, so any code that used to use mntput() to kill a cloned/freshly
>> created vfsmount got silently b0rken.  A quick look already caught
>> one such case - fs/nfsctl.c do_open() uses mntput() in pair with
>> do_kern_mount(), which leads to longrefs left at 1.

Hmm ok. I agree it's not the nicest scheme.

>>
>>       Rationale for that mess, please...
>
> FWIW, what I intend to do is to keep these longrefs _only_ for
> cwd/root/attached or possibly hashed and set them alongside the
> normal ref.  I.e. require the callers of mntput_long() (not exported,
> local to core VFS) to keep the normal reference.

OK that sounds better... I didn't have a good rationale beyond
those cases.


> That way it would turn into hint for mntput() - "we have persistent
> refs, just decrement count on this CPU and piss off", with mntput_long()
> never going into the whole "and now we are dropping the last ref" part.

I don't see the point of that, though. That's what it essentially
already is, but the check is done by mntput rather than the caller
passing it in as a hint.


> AFAICS, it keeps your write-free objectives and gets much saner API.
> Shout if you have problems with that...

No that sounds good, I don't have a problem with it, although I don't
exactly understand what you're getting at in the second paragraph.
--
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] 46+ messages in thread

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-14 22:07           ` Nick Piggin
@ 2011-01-15 13:30             ` Al Viro
  2011-01-15 18:33               ` Nick Piggin
  2011-01-15 18:46               ` Nick Piggin
  0 siblings, 2 replies; 46+ messages in thread
From: Al Viro @ 2011-01-15 13:30 UTC (permalink / raw)
  To: Nick Piggin
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds


> > AFAICS, it keeps your write-free objectives and gets much saner API.
> > Shout if you have problems with that...
> 
> No that sounds good, I don't have a problem with it, although I don't
> exactly understand what you're getting at in the second paragraph.

OK, I have a hopefully sane implementation in tip of #untested.

There's a fun problem with what you do in do_lookup(), BTW.  Look:
we enter do_lookup() with LOOKUP_RCU.  We find dentry in dcache,
everything's beautiful.  The sucker has ->d_revalidate().  We go
to need_revalidate.  There we call do_revalidate().  It calls
d_revalidate(), which calls ->d_revalidate() and instead of spitting
into your eye and returning -ECHILD it happily returns 1.  So
do d_revalidate() and then do_revalidate(), without any further
actions.  do_revalidate() has returned our dentry, which is neither
NULL nor ERR_PTR(), so back in do_lookup() we go to done.

There we set path->mnt and path->dentry and call __follow_mount().
And damn, it *is* a mountpoint.  So we
	* do dput() on dentry we'd never grabbed a reference to
	* grab a reference to a different dentry (and remain in happy
belief that we are in LOOKUP_RCU mode, and thus don't need to drop it)
	* grab a reference to vfsmount (via lookup_mnt()).  Ditto (and
I haven't checked if grabbing vfsmount_lock twice shared isn't a recipe
for a deadlocky race with something grabbing it exclusive between these
nested shared grabs).
	* if we are really unlucky and that mountpoint is, in turn,
overmounted, we'll hit mntput().  While under vfsmount_lock.

AFAICS, it's badly b0rken.  And autofs really steps into that mess.

As minimum, we'd need to split need_revalidate: and done: in RCU and non-RCU
variants.  I'm about to fall down right now after an all-nighter (and then
some); if you put something together before I get up, please throw it
my way.

Note that the problem exists both in mainline and in mainline+automount
patches; in the latter it's a bit nastier, but in principle the situation
is the same, so I'd rather see a fix for that in front of automount queue.

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-15 13:30             ` Al Viro
@ 2011-01-15 18:33               ` Nick Piggin
  2011-01-16  0:24                 ` Al Viro
  2011-01-15 18:46               ` Nick Piggin
  1 sibling, 1 reply; 46+ messages in thread
From: Nick Piggin @ 2011-01-15 18:33 UTC (permalink / raw)
  To: Al Viro
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Sun, Jan 16, 2011 at 12:30 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>> > AFAICS, it keeps your write-free objectives and gets much saner API.
>> > Shout if you have problems with that...
>>
>> No that sounds good, I don't have a problem with it, although I don't
>> exactly understand what you're getting at in the second paragraph.
>
> OK, I have a hopefully sane implementation in tip of #untested.
>
> There's a fun problem with what you do in do_lookup(), BTW.  Look:
> we enter do_lookup() with LOOKUP_RCU.  We find dentry in dcache,
> everything's beautiful.  The sucker has ->d_revalidate().  We go
> to need_revalidate.  There we call do_revalidate().  It calls
> d_revalidate(), which calls ->d_revalidate() and instead of spitting
> into your eye and returning -ECHILD it happily returns 1.  So
> do d_revalidate() and then do_revalidate(), without any further
> actions.  do_revalidate() has returned our dentry, which is neither
> NULL nor ERR_PTR(), so back in do_lookup() we go to done.
>
> There we set path->mnt and path->dentry and call __follow_mount().
> And damn, it *is* a mountpoint.  So we
>        * do dput() on dentry we'd never grabbed a reference to
>        * grab a reference to a different dentry (and remain in happy
> belief that we are in LOOKUP_RCU mode, and thus don't need to drop it)
>        * grab a reference to vfsmount (via lookup_mnt()).  Ditto (and
> I haven't checked if grabbing vfsmount_lock twice shared isn't a recipe
> for a deadlocky race with something grabbing it exclusive between these
> nested shared grabs).
>        * if we are really unlucky and that mountpoint is, in turn,
> overmounted, we'll hit mntput().  While under vfsmount_lock.
>
> AFAICS, it's badly b0rken.  And autofs really steps into that mess.

Ah, I think you're right there, good catch.


> As minimum, we'd need to split need_revalidate: and done: in RCU and non-RCU
> variants.  I'm about to fall down right now after an all-nighter (and then
> some); if you put something together before I get up, please throw it
> my way.

It also has a problem with jumping back to need_lookup case there too.
I think separating out those two cases is reasonable.

>
> Note that the problem exists both in mainline and in mainline+automount
> patches; in the latter it's a bit nastier, but in principle the situation
> is the same, so I'd rather see a fix for that in front of automount queue.

Definitely agree.

I'm on the road at the moment so would much appreciate if you can
cut the patch, but I could suggest something along the lines of:

	if (nd->flags & LOOKUP_RCU) {
		unsigned seq;

		*inode = nd->inode;
		dentry = __d_lookup_rcu(parent, name, &seq, inode);
		if (!dentry)
			goto need_lookup_rcu;
		/* Memory barrier in read_seqcount_begin of child is enough */
		if (__read_seqcount_retry(&parent->d_seq, nd->seq))
			return -ECHILD;

		nd->seq = seq;
		if (dentry->d_flags & DCACHE_OP_REVALIDATE) {
                        	dentry = do_revalidate(dentry, nd);
                                if (!dentry)
		                       goto need_lookup_rcu;
                                if (IS_ERR(dentry))
		                       goto fail;
                }
		path->mnt = mnt;
		path->dentry = dentry;
		__follow_mount_rcu(nd, path, inode);
	} else {
		dentry = __d_lookup(parent, name);
		if (!dentry)
			goto need_lookup;
found:
		if (dentry->d_flags & DCACHE_OP_REVALIDATE) {
                        	dentry = do_revalidate(dentry, nd);
                                if (!dentry)
		                       goto need_lookup;
                                if (IS_ERR(dentry))
		                       goto fail;
                }
		path->mnt = mnt;
		path->dentry = dentry;
		__follow_mount(path);
		*inode = path->dentry->d_inode;
	}
        return 0;

need_lookup_rcu:
        if (nameidata_drop_rcu(nd))
                return -ECHILD;
need_lookup:
       ...
--
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] 46+ messages in thread

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-15 13:30             ` Al Viro
  2011-01-15 18:33               ` Nick Piggin
@ 2011-01-15 18:46               ` Nick Piggin
  1 sibling, 0 replies; 46+ messages in thread
From: Nick Piggin @ 2011-01-15 18:46 UTC (permalink / raw)
  To: Al Viro
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Sun, Jan 16, 2011 at 12:30 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>> > AFAICS, it keeps your write-free objectives and gets much saner API.
>> > Shout if you have problems with that...
>>
>> No that sounds good, I don't have a problem with it, although I don't
>> exactly understand what you're getting at in the second paragraph.
>
> OK, I have a hopefully sane implementation in tip of #untested.

This looks fine to me. Care to add a little comment above
mnt_make_longterm that it requires switching to a short term
mount before mntput()?

Thanks,
Nick

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() [ver #4]
  2011-01-13 21:54 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() " David Howells
@ 2011-01-16  0:09   ` Al Viro
  2011-01-16  1:17     ` Al Viro
  2011-01-16 18:12     ` David Howells
  0 siblings, 2 replies; 46+ messages in thread
From: Al Viro @ 2011-01-16  0:09 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Thu, Jan 13, 2011 at 09:54:05PM +0000, David Howells wrote:
> @@ -1038,12 +1147,14 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,

> -		__follow_mount_rcu(nd, path, inode);
> +		if (unlikely(!__follow_mount_rcu(nd, path, inode, false)) &&
> +		    nameidata_drop_rcu(nd))
> +			return -ECHILD;

Gotcha.  That's what'd been causing fun with autofs.  Look: we run into
automount.  __follow_mount_rcu() fails.  nd->path *AND* path contain
vfsmounts/dentries we hadn't grabbed references to.  nameidata_drop_rcu()
brings nd->path to normal.  Good for it, but path is *still* not grabbed.
And LOOKUP_RCU is gone, so when the caller feeds it to path_to_nameidata()
the fun things start happening.  We drop nd->dentry, for starters.  And
replace it with ungrabbed dentry we just got from dcache.  Better yet,
if automount dentry had been sitting on top of mountpoint (e.g. it was
a direct autofs mountpoint), we'll drop nd->mnt and replace it with
vfsmount we hadn't grabbed.  Then we'll proceed to treat both as if they'd
been grabbed, eventually doing path_put() on the suckers.

vfsmount will stay around, since it's mounted and thus mntput() optimizations
will assume we are not dropping the last reference and not free the sucker.
dentry, OTOH, will be freed after a few time through that.  At which point
it gets reused and we are left with complete crap.  E.g. with vfsmount
mounted on that mountpoint, its ->mnt_root pointing to dentry from completely
unrelated fs.  Keep walking into that and you'll get _that_ dentry freed
under whoever might be using it, etc.

Fix is trivial: if __follow_mount_rcu() succeeds (the normal case) just
return 0.  Otherwise, try nameidata_drop_rcu(); if it fails, we bugger off.
So far it's equivalent to your variant, but if nameidata_drop_rcu() succeeds,
we fall through to non-RCU case instead of just returning 0.

I've dropped the following into for-next; autofs seems to be working fine
with it.  I'll fold that into your first patch in #untested.

AFS issue is completely separate story; that one is mine and it's old.
Just that now it became more visible; earlier it only messed the expiry
logics (by screwing ->mnt_ghosts accounting).  I've a half-finished fix
for that, but it needs more beating (and will be stable@ fodder, BTW).
Later...

commit c737c31d6f1ba4eb3f1aec523741ed49d5f6a22a
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Sat Jan 15 18:10:31 2011 -0500

    [foldme] fix do_lookup() handling of automount in RCU case
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namei.c b/fs/namei.c
index 9367fea..8f7b41a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1277,24 +1277,25 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
 done2:
 		path->mnt = mnt;
 		path->dentry = dentry;
-		if (unlikely(!__follow_mount_rcu(nd, path, inode, false)) &&
-		    nameidata_drop_rcu(nd))
+		if (likely(__follow_mount_rcu(nd, path, inode, false)))
+			return 0;
+		if (nameidata_drop_rcu(nd))
 			return -ECHILD;
-	} else {
-		dentry = __d_lookup(parent, name);
-		if (!dentry)
-			goto need_lookup;
+		/* fallthru */
+	}
+	dentry = __d_lookup(parent, name);
+	if (!dentry)
+		goto need_lookup;
 found:
-		if (dentry->d_flags & DCACHE_OP_REVALIDATE)
-			goto need_revalidate;
+	if (dentry->d_flags & DCACHE_OP_REVALIDATE)
+		goto need_revalidate;
 done:
-		path->mnt = mnt;
-		path->dentry = dentry;
-		err = follow_managed(path, nd->flags);
-		if (unlikely(err < 0))
-			return err;
-		*inode = path->dentry->d_inode;
-	}
+	path->mnt = mnt;
+	path->dentry = dentry;
+	err = follow_managed(path, nd->flags);
+	if (unlikely(err < 0))
+		return err;
+	*inode = path->dentry->d_inode;
 	return 0;
 
 need_lookup:

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-15 18:33               ` Nick Piggin
@ 2011-01-16  0:24                 ` Al Viro
  2011-01-16  1:21                   ` Nick Piggin
  0 siblings, 1 reply; 46+ messages in thread
From: Al Viro @ 2011-01-16  0:24 UTC (permalink / raw)
  To: Nick Piggin
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Sun, Jan 16, 2011 at 05:33:24AM +1100, Nick Piggin wrote:

> Definitely agree.
> 
> I'm on the road at the moment so would much appreciate if you can
> cut the patch, but I could suggest something along the lines of:

Easier than that, actually.  All it takes is (in #for-next, yet to
be reordered in front of queue)
        if (IS_ERR(dentry))
                goto fail;
+       if (nd->flags & LOOKUP_RCU)
+               goto done2;
        goto done;
in the end of need_revalidate and
                if (dentry->d_flags & DCACHE_OP_REVALIDATE)
                        goto need_revalidate;
+done2:
in LOOKUP_RCU side of if ()...

need_lookup does *not* suffer the same problem; you can't get there without
dropping LOOKUP_RCU - the first thing it does is grabbing a mutex, for fsck
sake...  And yes, you do drop RCU on all paths leading there.  I certainly
don't like the ugliness of that code, but rewriting that to something less...
unpleasant is a separate story.

Again, see #for-next; I'll reorder it shortly.

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

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() [ver #4]
  2011-01-16  0:09   ` Al Viro
@ 2011-01-16  1:17     ` Al Viro
  2011-01-16 18:12     ` David Howells
  1 sibling, 0 replies; 46+ messages in thread
From: Al Viro @ 2011-01-16  1:17 UTC (permalink / raw)
  To: David Howells; +Cc: raven, npiggin, autofs, linux-fsdevel

On Sun, Jan 16, 2011 at 12:09:47AM +0000, Al Viro wrote:

> AFS issue is completely separate story; that one is mine and it's old.
> Just that now it became more visible; earlier it only messed the expiry
> logics (by screwing ->mnt_ghosts accounting).  I've a half-finished fix
> for that, but it needs more beating (and will be stable@ fodder, BTW).
> Later...

OK, umount_tree bug (the source of AFS leak) got presumably fixed in
#untested.  Have fun...

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

* Re: [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
  2011-01-16  0:24                 ` Al Viro
@ 2011-01-16  1:21                   ` Nick Piggin
  0 siblings, 0 replies; 46+ messages in thread
From: Nick Piggin @ 2011-01-16  1:21 UTC (permalink / raw)
  To: Al Viro
  Cc: David Howells, raven, npiggin, autofs, linux-fsdevel, Linus Torvalds

On Sun, Jan 16, 2011 at 11:24 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Jan 16, 2011 at 05:33:24AM +1100, Nick Piggin wrote:
>
>> Definitely agree.
>>
>> I'm on the road at the moment so would much appreciate if you can
>> cut the patch, but I could suggest something along the lines of:
>
> Easier than that, actually.  All it takes is (in #for-next, yet to
> be reordered in front of queue)
>        if (IS_ERR(dentry))
>                goto fail;
> +       if (nd->flags & LOOKUP_RCU)
> +               goto done2;
>        goto done;
> in the end of need_revalidate and
>                if (dentry->d_flags & DCACHE_OP_REVALIDATE)
>                        goto need_revalidate;
> +done2:
> in LOOKUP_RCU side of if ()...
>
> need_lookup does *not* suffer the same problem; you can't get there without
> dropping LOOKUP_RCU - the first thing it does is grabbing a mutex, for fsck
> sake...  And yes, you do drop RCU on all paths leading there.

Oh right it's do_revalidate, not d_revalidate, so it won't return NULL
still in rcu-walk :P
--
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] 46+ messages in thread

* Re: [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() [ver #4]
  2011-01-16  0:09   ` Al Viro
  2011-01-16  1:17     ` Al Viro
@ 2011-01-16 18:12     ` David Howells
  1 sibling, 0 replies; 46+ messages in thread
From: David Howells @ 2011-01-16 18:12 UTC (permalink / raw)
  To: Al Viro; +Cc: dhowells, raven, npiggin, autofs, linux-fsdevel

Al Viro <viro@ZenIV.linux.org.uk> wrote:

> OK, umount_tree bug (the source of AFS leak) got presumably fixed in
> #untested.  Have fun...

Works for me.

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

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

end of thread, other threads:[~2011-01-16 18:13 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-13 21:53 [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
2011-01-13 21:54 ` [PATCH 01/18] Add a dentry op to handle automounting rather than abusing follow_link() " David Howells
2011-01-16  0:09   ` Al Viro
2011-01-16  1:17     ` Al Viro
2011-01-16 18:12     ` David Howells
2011-01-13 21:54 ` [PATCH 02/18] Add a dentry op to allow processes to be held during pathwalk transit " David Howells
2011-01-13 21:54 ` [PATCH 03/18] From: David Howells <dhowells@redhat.com> " David Howells
2011-01-13 21:54 ` [PATCH 04/18] AFS: Use d_automount() rather than abusing follow_link() " David Howells
2011-01-13 21:54 ` [PATCH 05/18] NFS: " David Howells
2011-01-13 21:54 ` [PATCH 06/18] CIFS: " David Howells
2011-01-13 21:54 ` [PATCH 07/18] Remove the automount through follow_link() kludge code from pathwalk " David Howells
2011-01-13 21:54 ` [PATCH 08/18] autofs4: Add d_automount() dentry operation " David Howells
2011-01-13 21:54 ` [PATCH 09/18] autofs4: Add d_manage() " David Howells
2011-01-14 13:51   ` Ian Kent
2011-01-14 14:37     ` Nick Piggin
2011-01-14 15:47       ` Nick Piggin
2011-01-14 15:35     ` David Howells
2011-01-14 15:46       ` Nick Piggin
2011-01-13 21:54 ` [PATCH 10/18] autofs4: Remove unused code " David Howells
2011-01-13 21:54 ` [PATCH 11/18] autofs4: Clean up inode operations " David Howells
2011-01-13 21:55 ` [PATCH 12/18] autofs4: Clean up dentry " David Howells
2011-01-13 21:55 ` [PATCH 13/18] autofs4: Clean up autofs4_free_ino() " David Howells
2011-01-14 16:03   ` Al Viro
2011-01-13 21:55 ` [PATCH 14/18] autofs4: Fix wait validation " David Howells
2011-01-13 21:55 ` [PATCH 15/18] autofs4: Add v4 pseudo direct mount support " David Howells
2011-01-13 21:55 ` [PATCH 16/18] autofs4: Bump version " David Howells
2011-01-13 21:55 ` [PATCH 17/18] Remove a further kludge from __do_follow_link() " David Howells
2011-01-13 21:55 ` [PATCH 18/18] Allow d_manage() to be used in RCU-walk mode " David Howells
2011-01-14  7:02 ` [PATCH 00/18] Introduce automount support in the VFS " Al Viro
2011-01-14  7:05   ` Al Viro
2011-01-14 11:20   ` David Howells
2011-01-14 11:43 ` David Howells
2011-01-14 15:46   ` Al Viro
2011-01-14 17:26   ` [PATCH 19/18] Unexport do_add_mount() and add in follow_automount(), not ->d_automount() David Howells
2011-01-14 17:43     ` Al Viro
2011-01-14 17:56       ` Al Viro
2011-01-14 18:06         ` Al Viro
2011-01-14 22:07           ` Nick Piggin
2011-01-15 13:30             ` Al Viro
2011-01-15 18:33               ` Nick Piggin
2011-01-16  0:24                 ` Al Viro
2011-01-16  1:21                   ` Nick Piggin
2011-01-15 18:46               ` Nick Piggin
2011-01-14 17:30   ` David Howells
2011-01-14 11:54 ` [PATCH 00/18] Introduce automount support in the VFS [ver #4] David Howells
2011-01-14 11:54   ` 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.