Add a flag to struct pfs_vdata to mark the vnode as dead (e.g. process-

specific nodes when the process exits)

Move the vnode-cache-walking loop which was duplicated in pfs_exit() and
pfs_disable() into its own function, pfs_purge(), which looks for vnodes
marked as dead and / or belonging to the specified pfs_node and reclaims
them.  Note that this loop is still extremely inefficient.

Add a comment in pfs_vncache_alloc() explaining why we have to purge the
vnode from the vnode cache before returning, in case anyone should be
tempted to remove the call to cache_purge().

Move the special handling for pfstype_root nodes into pfs_fileno_alloc()
and pfs_fileno_free() (the root node's fileno must always be 2).  This
also fixes a bug where pfs_fileno_free() would reclaim the root node's
fileno, triggering a panic in the unr code, as that fileno was never
allocated from unr to begin with.

When destroying a pfs_node, release its fileno and purge it from the
vnode cache.  I wish we could put off the call to pfs_purge() until
after the entire tree had been destroyed, but then we'd have vnodes
referencing freed pfs nodes.  This probably doesn't matter while we're
still under Giant, but might become an issue later.

When destroying a pseudofs instance, destroy the tree before tearing
down the fileno allocator.

In pfs_mount(), acquire the mountpoint interlock when required.

MFC after:	3 weeks

5 files changed, 74 insertions, 51 deletions

diff --git a/sys/fs/pseudofs/pseudofs.c b/sys/fs/pseudofs/pseudofs.c
index 049afc5..9464e9c 100644
--- a/sys/fs/pseudofs/pseudofs.c
+++ b/sys/fs/pseudofs/pseudofs.c
@@ -258,8 +258,10 @@ pfs_destroy(struct pfs_node *node)
 	if (node->pn_destroy != NULL)
 		(node->pn_destroy)(node);
 
-	/* revoke vnodes and release memory */
-	pfs_disable(node);
+	/* revoke fileno and vnodes and release memory */
+	if (node->pn_fileno)
+		pfs_fileno_free(node->pn_info, node);
+	pfs_purge(node);
 	FREE(node, M_PFSNODES);
 
 	return (0);
@@ -276,7 +278,13 @@ pfs_mount(struct pfs_info *pi, struct mount *mp, struct thread *td)
 	if (mp->mnt_flag & MNT_UPDATE)
 		return (EOPNOTSUPP);
 
+	MNT_ILOCK(mp);
 	mp->mnt_flag |= MNT_LOCAL;
+#if 0
+	/* not quite ready for this yet */
+	mp->mnt_kern_flag |= MNTK_MPSAFE;
+#endif
+	MNT_IUNLOCK(mp);
 	mp->mnt_data = (qaddr_t)pi;
 	vfs_getnewfsid(mp);
 
@@ -387,9 +395,9 @@ pfs_uninit(struct pfs_info *pi, struct vfsconf *vfc)
 {
 	int error;
 
-	pfs_fileno_uninit(pi);
 	pfs_destroy(pi->pi_root);
 	pi->pi_root = NULL;
+	pfs_fileno_uninit(pi);
 	mtx_destroy(&pi->pi_mutex);
 	if (bootverbose)
 		printf("%s unregistered\n", pi->pi_name);
diff --git a/sys/fs/pseudofs/pseudofs.h b/sys/fs/pseudofs/pseudofs.h
index 49ba64a..bb1efd1 100644
--- a/sys/fs/pseudofs/pseudofs.h
+++ b/sys/fs/pseudofs/pseudofs.h
@@ -236,6 +236,7 @@ struct pfs_node	*pfs_create_link(struct pfs_node *parent, const char *name,
 				 pfs_vis_t vis, pfs_destroy_t destroy,
 				 int flags);
 struct pfs_node	*pfs_find_node	(struct pfs_node *parent, const char *name);
+void		 pfs_purge	(struct pfs_node *pn);
 int		 pfs_disable	(struct pfs_node *pn);
 int		 pfs_enable	(struct pfs_node *pn);
 int		 pfs_destroy	(struct pfs_node *pn);
diff --git a/sys/fs/pseudofs/pseudofs_fileno.c b/sys/fs/pseudofs/pseudofs_fileno.c
index 9ddca9d..10c1617 100644
--- a/sys/fs/pseudofs/pseudofs_fileno.c
+++ b/sys/fs/pseudofs/pseudofs_fileno.c
@@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/malloc.h>
 #include <sys/mutex.h>
 #include <sys/sysctl.h>
+#include <sys/systm.h>
 
 #include <fs/pseudofs/pseudofs.h>
 #include <fs/pseudofs/pseudofs_internal.h>
@@ -54,7 +55,6 @@ pfs_fileno_init(struct pfs_info *pi)
 	up = new_unrhdr(3, INT_MAX, &pi->pi_mutex);
 	mtx_lock(&pi->pi_mutex);
 	pi->pi_unrhdr = up;
-	pi->pi_root->pn_fileno = 2;
 	mtx_unlock(&pi->pi_mutex);
 }
 
@@ -82,12 +82,18 @@ pfs_fileno_uninit(struct pfs_info *pi)
 void
 pfs_fileno_alloc(struct pfs_info *pi, struct pfs_node *pn)
 {
+	/* pi is not really necessary as it can be derived */
+	KASSERT(pi == pn->pn_info, ("pn / pi mismatch"));
+
 	/* make sure our parent has a file number */
 	if (pn->pn_parent && !pn->pn_parent->pn_fileno)
 		pfs_fileno_alloc(pi, pn->pn_parent);
 
 	switch (pn->pn_type) {
 	case pfstype_root:
+		/* root must always be 2 */
+		pn->pn_fileno = 2;
+		break;
 	case pfstype_dir:
 	case pfstype_file:
 	case pfstype_symlink:
@@ -134,8 +140,13 @@ pfs_fileno_alloc(struct pfs_info *pi, struct pfs_node *pn)
 void
 pfs_fileno_free(struct pfs_info *pi, struct pfs_node *pn)
 {
+	/* pi is not really necessary as it can be derived */
+	KASSERT(pi == pn->pn_info, ("pn / pi mismatch"));
+
 	switch (pn->pn_type) {
 	case pfstype_root:
+		/* not allocated from unrhdr */
+		return;
 	case pfstype_dir:
 	case pfstype_file:
 	case pfstype_symlink:
diff --git a/sys/fs/pseudofs/pseudofs_internal.h b/sys/fs/pseudofs/pseudofs_internal.h
index 7cbc86d..6abd704 100644
--- a/sys/fs/pseudofs/pseudofs_internal.h
+++ b/sys/fs/pseudofs/pseudofs_internal.h
@@ -44,6 +44,7 @@ struct pfs_vdata {
 	pid_t		 pvd_pid;
 	struct vnode	*pvd_vnode;
 	struct pfs_vdata*pvd_prev, *pvd_next;
+	int		 pvd_dead:1;
 };
 
 /*
diff --git a/sys/fs/pseudofs/pseudofs_vncache.c b/sys/fs/pseudofs/pseudofs_vncache.c
index 2412e32..e4d8db8 100644
--- a/sys/fs/pseudofs/pseudofs_vncache.c
+++ b/sys/fs/pseudofs/pseudofs_vncache.c
@@ -127,8 +127,16 @@ retry:
 			if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, curthread) == 0) {
 				++pfs_vncache_hits;
 				*vpp = vp;
-				/* XXX see comment at top of pfs_lookup() */
-				cache_purge(vp);	
+				/*
+				 * Some callers cache_enter(vp) later, so
+				 * we have to make sure it's not in the
+				 * VFS cache so it doesn't get entered
+				 * twice.  A better solution would be to
+				 * make pfs_vncache_alloc() responsible
+				 * for entering the vnode in the VFS
+				 * cache.
+				 */
+				cache_purge(vp);
 				return (0);
 			}
 			goto retry;
@@ -224,38 +232,28 @@ pfs_vncache_free(struct vnode *vp)
 }
 
 /*
- * Free all vnodes associated with a defunct process
+ * Purge the cache of dead / disabled entries
  *
- * XXXRW: It is unfortunate that pfs_exit() always acquires and releases two
- * mutexes (one of which is Giant) for every process exit, even if procfs
- * isn't mounted.
+ * This is extremely inefficient due to the fact that vgone() not only
+ * indirectly modifies the vnode cache, but may also sleep.  We can
+ * neither hold pfs_vncache_mutex across a vgone() call, nor make any
+ * assumptions about the state of the cache after vgone() returns.  In
+ * consequence, we must start over after every vgone() call, and keep
+ * trying until we manage to traverse the entire cache.
+ *
+ * The only way to improve this situation is to change the data structure
+ * used to implement the cache.
  */
-static void
-pfs_exit(void *arg, struct proc *p)
+void
+pfs_purge(struct pfs_node *pn)
 {
 	struct pfs_vdata *pvd;
 	struct vnode *vnp;
 
-	if (pfs_vncache == NULL)
-		return;
-	mtx_lock(&Giant);
-	/*
-	 * This is extremely inefficient due to the fact that vgone() not
-	 * only indirectly modifies the vnode cache, but may also sleep.
-	 * We can neither hold pfs_vncache_mutex across a vgone() call,
-	 * nor make any assumptions about the state of the cache after
-	 * vgone() returns.  In consequence, we must start over after
-	 * every vgone() call, and keep trying until we manage to traverse
-	 * the entire cache.
-	 *
-	 * The only way to improve this situation is to change the data
-	 * structure used to implement the cache.  An obvious choice in
-	 * this particular case would be a BST sorted by PID.
-	 */
 	mtx_lock(&pfs_vncache_mutex);
 	pvd = pfs_vncache;
 	while (pvd != NULL) {
-		if (pvd->pvd_pid == p->p_pid) {
+		if (pvd->pvd_dead || (pn != NULL && pvd->pvd_pn == pn)) {
 			vnp = pvd->pvd_vnode;
 			vhold(vnp);
 			mtx_unlock(&pfs_vncache_mutex);
@@ -270,6 +268,31 @@ pfs_exit(void *arg, struct proc *p)
 		}
 	}
 	mtx_unlock(&pfs_vncache_mutex);
+}
+
+/*
+ * Free all vnodes associated with a defunct process
+ *
+ * XXXRW: It is unfortunate that pfs_exit() always acquires and releases two
+ * mutexes (one of which is Giant) for every process exit, even if procfs
+ * isn't mounted.
+ */
+static void
+pfs_exit(void *arg, struct proc *p)
+{
+	struct pfs_vdata *pvd;
+	int dead;
+
+	if (pfs_vncache == NULL)
+		return;
+	mtx_lock(&Giant);
+	mtx_lock(&pfs_vncache_mutex);
+	for (pvd = pfs_vncache, dead = 0; pvd != NULL; pvd = pvd->pvd_next)
+		if (pvd->pvd_pid == p->p_pid)
+			dead = pvd->pvd_dead = 1;
+	mtx_unlock(&pfs_vncache_mutex);
+	if (dead)
+		pfs_purge(NULL);
 	mtx_unlock(&Giant);
 }
 
@@ -279,31 +302,10 @@ pfs_exit(void *arg, struct proc *p)
 int
 pfs_disable(struct pfs_node *pn)
 {
-	struct pfs_vdata *pvd;
-	struct vnode *vnp;
-
 	if (pn->pn_flags & PFS_DISABLED)
 		return (0);
 	pn->pn_flags |= PFS_DISABLED;
-	/* XXX see comment above nearly identical code in pfs_exit() */
-	mtx_lock(&pfs_vncache_mutex);
-	pvd = pfs_vncache;
-	while (pvd != NULL) {
-		if (pvd->pvd_pn == pn) {
-			vnp = pvd->pvd_vnode;
-			vhold(vnp);
-			mtx_unlock(&pfs_vncache_mutex);
-			VOP_LOCK(vnp, LK_EXCLUSIVE, curthread);
-			vgone(vnp);
-			VOP_UNLOCK(vnp, 0, curthread);
-			vdrop(vnp);
-			mtx_lock(&pfs_vncache_mutex);
-			pvd = pfs_vncache;
-		} else {
-			pvd = pvd->pvd_next;
-		}
-	}
-	mtx_unlock(&pfs_vncache_mutex);
+	pfs_purge(pn);
 	return (0);
 }