[PATCH] autofs4: expiry refcount fixes

From: Ian Kent <raven@themaw.net>

This patch is the result of an e-mail discussion with Soni Maneesh.  He felt
that the use of reference counts in the expire module is unreliable (in the
presence of rcu) and suggested it should use standard VFS calls where
possible.  This has been done.  Once the boundary in autofs is reached we
have no choice but to resort using reference counts (but under the
vfsmount_lock).


After review by hch:

- renamed autofs4_may_umount to __may_umount_tree, made it static and moved
  it to namespace.c.

- added stub function may_umount_tree with description

- altered may_umount to use above stub function and added little description

- added may_umount_tree prototype to fs.h

- removed the EXPORT_SYMBOL for vfsmount_lock

- updated expire.c to suit

fs/autofs4/autofs_i.h

@@ -25,6 +25,8 @@
 #include <linux/string.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
+#include <linux/mount.h>
+#include <linux/namei.h>
 #include <asm/current.h>
 #include <asm/uaccess.h>
 
@@ -160,3 +162,21 @@ enum autofs_notify
 int autofs4_wait(struct autofs_sb_info *,struct qstr *, 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 simple_positive(struct dentry *dentry)
+{
+	return dentry->d_inode && !d_unhashed(dentry);
+}
+
+static inline int simple_empty_nolock(struct dentry *dentry)
+{
+	struct dentry *child;
+	int ret = 0;
+
+	list_for_each_entry(child, &dentry->d_subdirs, d_child)
+		if (simple_positive(child))
+			goto out;
+	ret = 1;
+out:
+	return ret;
+}

fs/autofs4/expire.c

@@ -4,6 +4,7 @@
  *
  *  Copyright 1997-1998 Transmeta Corporation -- All Rights Reserved
  *  Copyright 1999-2000 Jeremy Fitzhardinge <jeremy@goop.org>
+ *  Copyright 2001-2003 Ian Kent <raven@themaw.net>
  *
  * This file is part of the Linux kernel and is made available under
  * the terms of the GNU General Public License, version 2, or at your
@@ -12,139 +13,198 @@
  * ------------------------------------------------------------------------- */
 
 #include "autofs_i.h"
-#include <linux/mount.h>
 
-/*
- * Determine if a subtree of the namespace is busy.
- *
- * mnt is the mount tree under the autofs mountpoint
+static unsigned long now;
+
+/* Check if a dentry can be expired return 1 if it can else return 0 */
+static inline int autofs4_can_expire(struct dentry *dentry,
+					unsigned long timeout, int do_now)
+{
+	struct autofs_info *ino = autofs4_dentry_ino(dentry);
+
+	/* dentry in the process of being deleted */
+	if (ino == NULL)
+		return 0;
+
+	/* No point expiring a pending mount */
+	if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
+		return 0;
+
+	if (!do_now) {
+		/* Too young to die */
+		if (time_after(ino->last_used + timeout, now))
+			return 0;
+
+		/* update last_used here :-
+		   - obviously makes sense if it is in use now
+		   - less obviously, prevents rapid-fire expire
+		     attempts if expire fails the first time */
+		ino->last_used = now;
+	}
+
+	return 1;
+}
+
+/* Check a mount point for busyness return 1 if not busy, otherwise */
+static int autofs4_check_mount(struct vfsmount *mnt, struct dentry *dentry)
+{
+	int status = 0;
+
+	DPRINTK(("autofs4_check_mount: dentry %p %.*s\n",
+		 dentry, (int)dentry->d_name.len, dentry->d_name.name));
+
+	mntget(mnt);
+	dget(dentry);
+
+	if (!follow_down(&mnt, &dentry))
+		goto done;
+
+	while (d_mountpoint(dentry) && follow_down(&mnt, &dentry))
+		;
+
+	/* This is an autofs submount, we can't expire it */
+	if (is_autofs4_dentry(dentry))
+		goto done;
+
+	/* The big question */
+	if (may_umount_tree(mnt) == 0)
+		status = 1;
+done:
+	DPRINTK(("autofs4_check_mount: returning = %d\n", status));
+	mntput(mnt);
+	dput(dentry);
+	return status;
+}
+
+/* Check a directory tree of mount points for busyness
+ * The tree is not busy iff no mountpoints are busy
+ * Return 1 if the tree is busy or 0 otherwise
  */
-static inline int is_vfsmnt_tree_busy(struct vfsmount *mnt)
+static int autofs4_check_tree(struct vfsmount *mnt,
+	       		      struct dentry *top,
+			      unsigned long timeout,
+			      int do_now)
 {
-	struct vfsmount *this_parent = mnt;
+	struct dentry *this_parent = top;
 	struct list_head *next;
-	int count;
 
-	count = atomic_read(&mnt->mnt_count) - 1;
+	DPRINTK(("autofs4_check_tree: parent %p %.*s\n",
+		 top, (int)top->d_name.len, top->d_name.name));
+
+	/* Negative dentry - give up */
+	if (!simple_positive(top))
+		return 0;
+
+	/* Timeout of a tree mount is determined by its top dentry */
+	if (!autofs4_can_expire(top, timeout, do_now))
+		return 0;
 
+	spin_lock(&dcache_lock);
 repeat:
-	next = this_parent->mnt_mounts.next;
-	DPRINTK(("is_vfsmnt_tree_busy: mnt=%p, this_parent=%p, next=%p\n",
-		 mnt, this_parent, next));
+	next = this_parent->d_subdirs.next;
 resume:
-	for( ; next != &this_parent->mnt_mounts; next = next->next) {
-		struct vfsmount *p = list_entry(next, struct vfsmount,
-						mnt_child);
+	while (next != &this_parent->d_subdirs) {
+		struct dentry *dentry = list_entry(next, struct dentry, d_child);
 
-		/* -1 for struct vfs_mount's normal count, 
-		   -1 to compensate for child's reference to parent */
-		count += atomic_read(&p->mnt_count) - 1 - 1;
+		/* Negative dentry - give up */
+		if (!simple_positive(dentry)) {
+			next = next->next;
+			continue;
+		}
 
-		DPRINTK(("is_vfsmnt_tree_busy: p=%p, count now %d\n",
-			 p, count));
+		DPRINTK(("autofs4_check_tree: dentry %p %.*s\n",
+			 dentry, (int)dentry->d_name.len, dentry->d_name.name));
 
-		if (!list_empty(&p->mnt_mounts)) {
-			this_parent = p;
+		if (!simple_empty_nolock(dentry)) {
+			this_parent = dentry;
 			goto repeat;
 		}
-		/* root is busy if any leaf is busy */
-		if (atomic_read(&p->mnt_count) > 1)
-			return 1;
-	}
 
-	/* All done at this level ... ascend and resume the search. */
-	if (this_parent != mnt) {
-		next = this_parent->mnt_child.next; 
-		this_parent = this_parent->mnt_parent;
-		goto resume;
-	}
+		dentry = dget(dentry);
+		spin_unlock(&dcache_lock);
 
-	DPRINTK(("is_vfsmnt_tree_busy: count=%d\n", count));
-	return count != 0; /* remaining users? */
-}
+		if (d_mountpoint(dentry)) {
+			/* First busy => tree busy */
+			if (!autofs4_check_mount(mnt, dentry)) {
+				dput(dentry);
+				return 0;
+			}
+		}
 
-/* Traverse a dentry's list of vfsmounts and return the number of
-   non-busy mounts */
-static int check_vfsmnt(struct vfsmount *mnt, struct dentry *dentry)
-{
-	int ret = dentry->d_mounted;
-	struct vfsmount *vfs = lookup_mnt(mnt, dentry);
+		dput(dentry);
+		spin_lock(&dcache_lock);
+		next = next->next;
+	}
 
-	if (vfs) {
-		mntput(vfs);
-		if (is_vfsmnt_tree_busy(vfs))
-			ret--;
+	if (this_parent != top) {
+		next = this_parent->d_child.next;
+		this_parent = this_parent->d_parent;
+		goto resume;
 	}
-	DPRINTK(("check_vfsmnt: ret=%d\n", ret));
-	return ret;
+	spin_unlock(&dcache_lock);
+
+	return 1;
 }
 
-/* Check dentry tree for busyness.  If a dentry appears to be busy
-   because it is a mountpoint, check to see if the mounted
-   filesystem is busy. */
-static int is_tree_busy(struct vfsmount *topmnt, struct dentry *top)
+struct dentry *autofs4_check_leaves(struct vfsmount *mnt,
+				    struct dentry *parent,
+				    unsigned long timeout,
+				    int do_now)
 {
-	struct dentry *this_parent;
+	struct dentry *this_parent = parent;
 	struct list_head *next;
-	int count;
-
-	count = atomic_read(&top->d_count);
-	
-	DPRINTK(("is_tree_busy: top=%p initial count=%d\n", 
-		 top, count));
-	this_parent = top;
 
-	if (is_autofs4_dentry(top)) {
-		count--;
-		DPRINTK(("is_tree_busy: autofs; count=%d\n", count));
-	}
-
-	if (d_mountpoint(top))
-		count -= check_vfsmnt(topmnt, top);
+	DPRINTK(("autofs4_check_leaves: parent %p %.*s\n",
+		parent, (int)parent->d_name.len, parent->d_name.name));
 
- repeat:
+	spin_lock(&dcache_lock);
+repeat:
 	next = this_parent->d_subdirs.next;
- resume:
+resume:
 	while (next != &this_parent->d_subdirs) {
-		int adj = 0;
-		struct dentry *dentry = list_entry(next, struct dentry,
-						   d_child);
-		next = next->next;
-
-		count += atomic_read(&dentry->d_count) - 1;
+		struct dentry *dentry = list_entry(next, struct dentry, d_child);
 
-		if (d_mountpoint(dentry))
-			adj += check_vfsmnt(topmnt, dentry);
-
-		if (is_autofs4_dentry(dentry)) {
-			adj++;
-			DPRINTK(("is_tree_busy: autofs; adj=%d\n",
-				 adj));
+		/* Negative dentry - give up */
+		if (!simple_positive(dentry)) {
+			next = next->next;
+			continue;
 		}
 
-		count -= adj;
+		DPRINTK(("autofs4_check_leaves: dentry %p %.*s\n",
+			dentry, (int)dentry->d_name.len, dentry->d_name.name));
 
 		if (!list_empty(&dentry->d_subdirs)) {
 			this_parent = dentry;
 			goto repeat;
 		}
 
-		if (atomic_read(&dentry->d_count) != adj) {
-			DPRINTK(("is_tree_busy: busy leaf (d_count=%d adj=%d)\n",
-				 atomic_read(&dentry->d_count), adj));
-			return 1;
+		dentry = dget(dentry);
+		spin_unlock(&dcache_lock);
+
+		if (d_mountpoint(dentry)) {
+			/* Can we expire this guy */
+			if (!autofs4_can_expire(dentry, timeout, do_now))
+				goto cont;
+
+			/* Can we umount this guy */
+			if (autofs4_check_mount(mnt, dentry))
+				return dentry;
+
 		}
+cont:
+		dput(dentry);
+		spin_lock(&dcache_lock);
+		next = next->next;
 	}
 
-	/* All done at this level ... ascend and resume the search. */
-	if (this_parent != top) {
+	if (this_parent != parent) {
 		next = this_parent->d_child.next;
 		this_parent = this_parent->d_parent;
 		goto resume;
 	}
+	spin_unlock(&dcache_lock);
 
-	DPRINTK(("is_tree_busy: count=%d\n", count));
-	return count != 0; /* remaining users? */
+	return NULL;
 }
 
 /*
@@ -156,62 +216,87 @@ static int is_tree_busy(struct vfsmount *topmnt, struct dentry *top)
 static struct dentry *autofs4_expire(struct super_block *sb,
 				     struct vfsmount *mnt,
 				     struct autofs_sb_info *sbi,
-				     int do_now)
+				     int how)
 {
-	unsigned long now = jiffies;
 	unsigned long timeout;
 	struct dentry *root = sb->s_root;
-	struct list_head *tmp;
+	struct dentry *expired = NULL;
+	struct list_head *next;
+	int do_now = how & AUTOFS_EXP_IMMEDIATE;
+	int exp_leaves = how & AUTOFS_EXP_LEAVES;
 
-	if (!sbi->exp_timeout || !root)
+	if ( !sbi->exp_timeout || !root )
 		return NULL;
 
+	now = jiffies;
 	timeout = sbi->exp_timeout;
 
 	spin_lock(&dcache_lock);
-	for(tmp = root->d_subdirs.next;
-	    tmp != &root->d_subdirs; 
-	    tmp = tmp->next) {
-		struct autofs_info *ino;
-		struct dentry *dentry = list_entry(tmp, struct dentry, d_child);
+	next = root->d_subdirs.next;
 
-		if (dentry->d_inode == NULL)
-			continue;
+	/* On exit from the loop expire is set to a dgot dentry
+	 * to expire or it's NULL */
+	while ( next != &root->d_subdirs ) {
+		struct dentry *dentry = list_entry(next, struct dentry, d_child);
 
-		ino = autofs4_dentry_ino(dentry);
-
-		if (ino == NULL) {
-			/* dentry in the process of being deleted */
+		/* Negative dentry - give up */
+		if ( !simple_positive(dentry) ) {
+			next = next->next;
 			continue;
 		}
 
-		/* No point expiring a pending mount */
-		if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
-			continue;
+		dentry = dget(dentry);
+		spin_unlock(&dcache_lock);
 
-		if (!do_now) {
-			/* Too young to die */
-			if (time_after(ino->last_used + timeout, now))
-				continue;
+		/* Case 1: indirect mount or top level direct mount */
+		if (d_mountpoint(dentry)) {
+			DPRINTK(("autofs4_expire: checking mountpoint %p %.*s\n",
+			 dentry, (int)dentry->d_name.len, dentry->d_name.name));
 
-			/* update last_used here :- 
-			   - obviously makes sense if it is in use now
-			   - less obviously, prevents rapid-fire expire
-			     attempts if expire fails the first time */
-			ino->last_used = now;
+			/* Can we expire this guy */
+			if (!autofs4_can_expire(dentry, timeout, do_now))
+				goto next;
+
+			/* Can we umount this guy */
+			if (autofs4_check_mount(mnt, dentry)) {
+				expired = dentry;
+				break;
+			}
+			goto next;
 		}
-		if (!is_tree_busy(mnt, dentry)) {
-			DPRINTK(("autofs_expire: returning %p %.*s\n",
-				 dentry, (int)dentry->d_name.len, dentry->d_name.name));
-			/* Start from here next time */
-			list_del(&root->d_subdirs);
-			list_add(&root->d_subdirs, &dentry->d_child);
-			dget(dentry);
-			spin_unlock(&dcache_lock);
 
-			return dentry;
+		if ( simple_empty(dentry) )
+			goto next;
+
+		/* Case 2: tree mount, expire iff entire tree is not busy */
+		if (!exp_leaves) {
+			if (autofs4_check_tree(mnt, dentry, timeout, do_now)) {
+			expired = dentry;
+			break;
+			}
+		/* Case 3: direct mount, expire individual leaves */
+		} else {
+			expired = autofs4_check_leaves(mnt, dentry, timeout, do_now);
+			if (expired) {
+				dput(dentry);
+				break;
 			}
 		}
+next:
+		dput(dentry);
+		spin_lock(&dcache_lock);
+		next = next->next;
+	}
+
+	if ( expired ) {
+		DPRINTK(("autofs4_expire: returning %p %.*s\n",
+			 expired, (int)expired->d_name.len, expired->d_name.name));
+		spin_lock(&dcache_lock);
+		list_del(&expired->d_parent->d_subdirs);
+		list_add(&expired->d_parent->d_subdirs, &expired->d_child);
+		spin_unlock(&dcache_lock);
+		return expired;
+	}
 	spin_unlock(&dcache_lock);
 
 	return NULL;

fs/namespace.c

@@ -36,6 +36,7 @@ static inline int sysfs_init(void)
 
 /* spinlock for vfsmount related operations, inplace of dcache_lock */
 spinlock_t vfsmount_lock __cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
+
 static struct list_head *mount_hashtable;
 static int hash_mask, hash_bits;
 static kmem_cache_t *mnt_cache; 
@@ -259,16 +260,85 @@ struct seq_operations mounts_op = {
 	.show	= show_vfsmnt
 };
 
-/*
+static int __may_umount_tree(struct vfsmount *mnt, int root_mnt_only)
+{
+	struct list_head *next;
+	struct vfsmount *this_parent = mnt;
+	int actual_refs;
+	int minimum_refs;
+
+	spin_lock(&vfsmount_lock);
+	actual_refs = atomic_read(&mnt->mnt_count);
+	minimum_refs = 2;
+
+	if (root_mnt_only) {
+ 		spin_unlock(&vfsmount_lock);
+		if (actual_refs > minimum_refs)
+			return -EBUSY;
+		return 0;
+	}
+
+repeat:
+	next = this_parent->mnt_mounts.next;
+resume:
+	while (next != &this_parent->mnt_mounts) {
+		struct vfsmount *p = list_entry(next, struct vfsmount, mnt_child);
+
+		next = next->next;
+
+		actual_refs += atomic_read(&p->mnt_count);
+		minimum_refs += 2;
+
+		if (!list_empty(&p->mnt_mounts)) {
+			this_parent = p;
+			goto repeat;
+		}
+	}
+
+	if (this_parent != mnt) {
+		next = this_parent->mnt_child.next;
+		this_parent = this_parent->mnt_parent;
+		goto resume;
+	}
+	spin_unlock(&vfsmount_lock);
+
+	if (actual_refs > minimum_refs)
+		return -EBUSY;
+
+	return 0;
+}
+
+/**
+ * may_umount_tree - check if a mount tree is busy
+ * @mnt: root of mount tree
+ *
+ * This is called to check if a tree of mounts has any
+ * open files, pwds, chroots or sub mounts that are
+ * busy.
+ */
+int may_umount_tree(struct vfsmount *mnt)
+{
+	return __may_umount_tree(mnt, 0);
+}
+
+EXPORT_SYMBOL(may_umount_tree);
+
+/**
+ * may_umount - check if a mount point is busy
+ * @mnt: root of mount
+ *
+ * This is called to check if a mount point has any
+ * open files, pwds, chroots or sub mounts. If the
+ * mount has sub mounts this will return busy
+ * regardless of whether the sub mounts are busy.
+ *
  * Doesn't take quota and stuff into account. IOW, in some cases it will
  * give false negatives. The main reason why it's here is that we need
  * a non-destructive way to look for easily umountable filesystems.
  */
 int may_umount(struct vfsmount *mnt)
 {
-	if (atomic_read(&mnt->mnt_count) > 2)
-		return -EBUSY;
-	return 0;
+	return __may_umount_tree(mnt, 1);
 }
 
 EXPORT_SYMBOL(may_umount);

include/linux/auto_fs4.h

@@ -23,6 +23,10 @@
 #define AUTOFS_MIN_PROTO_VERSION	3
 #define AUTOFS_MAX_PROTO_VERSION	4
 
+/* Mask for expire behaviour */
+#define AUTOFS_EXP_IMMEDIATE		1
+#define AUTOFS_EXP_LEAVES		2
+
 /* New message type */
 #define autofs_ptype_expire_multi	2	/* Expire entry (umount request) */
 

include/linux/fs.h

@@ -1126,6 +1126,7 @@ void unnamed_dev_init(void);
 extern int register_filesystem(struct file_system_type *);
 extern int unregister_filesystem(struct file_system_type *);
 extern struct vfsmount *kern_mount(struct file_system_type *);
+extern int may_umount_tree(struct vfsmount *);
 extern int may_umount(struct vfsmount *);
 extern long do_mount(char *, char *, char *, unsigned long, void *);