Make pseudofs (and consequently procfs, linprocfs and linsysfs) MPSAFE.

6 files changed, 553 insertions, 340 deletions

diff --git a/sys/fs/pseudofs/pseudofs.c b/sys/fs/pseudofs/pseudofs.c
index 82ec971..de0d1f1 100644
--- a/sys/fs/pseudofs/pseudofs.c
+++ b/sys/fs/pseudofs/pseudofs.c
@@ -52,18 +52,48 @@ static MALLOC_DEFINE(M_PFSNODES, "pfs_nodes", "pseudofs nodes");
 SYSCTL_NODE(_vfs, OID_AUTO, pfs, CTLFLAG_RW, 0,
     "pseudofs");
 
+int pfs_trace;
+SYSCTL_INT(_vfs_pfs, OID_AUTO, trace, CTLFLAG_RW, &pfs_trace, 0,
+    "enable tracing of pseudofs vnode operations");
+
 #if PFS_FSNAMELEN != MFSNAMELEN
 #error "PFS_FSNAMELEN is not equal to MFSNAMELEN"
 #endif
 
 /*
+ * Allocate and initialize a node
+ */
+static struct pfs_node *
+pfs_alloc_node(struct pfs_info *pi, const char *name, pfs_type_t type)
+{
+	struct pfs_node *pn;
+
+	KASSERT(strlen(name) < PFS_NAMELEN,
+	    ("%s(): node name is too long", __func__));
+
+	MALLOC(pn, struct pfs_node *, sizeof *pn,
+	    M_PFSNODES, M_WAITOK|M_ZERO);
+	mtx_init(&pn->pn_mutex, "pfs_node", NULL, MTX_DEF | MTX_DUPOK);
+	strlcpy(pn->pn_name, name, sizeof pn->pn_name);
+	pn->pn_type = type;
+	pn->pn_info = pi;
+	return (pn);
+}
+
+/*
  * Add a node to a directory
  */
-static int
-_pfs_add_node(struct pfs_node *parent, struct pfs_node *node)
+static void
+pfs_add_node(struct pfs_node *parent, struct pfs_node *pn)
 {
+#ifdef INVARIANTS
+	struct pfs_node *iter;
+#endif
+
 	KASSERT(parent != NULL,
 	    ("%s(): parent is NULL", __func__));
+	KASSERT(pn->pn_parent == NULL,
+	    ("%s(): node already has a parent", __func__));
 	KASSERT(parent->pn_info != NULL,
 	    ("%s(): parent has no pn_info", __func__));
 	KASSERT(parent->pn_type == pfstype_dir ||
@@ -71,48 +101,70 @@ _pfs_add_node(struct pfs_node *parent, struct pfs_node *node)
 	    parent->pn_type == pfstype_root,
 	    ("%s(): parent is not a directory", __func__));
 
-	/* XXX should check for duplicate names etc. */
+#ifdef INVARIANTS
+	/* XXX no locking! */
+	if (pn->pn_type == pfstype_procdir)
+		for (iter = parent; iter != NULL; iter = iter->pn_parent)
+			KASSERT(iter->pn_type != pfstype_procdir,
+			    ("%s(): nested process directories", __func__));
+	for (iter = parent->pn_nodes; iter != NULL; iter = iter->pn_next) {
+		KASSERT(strcmp(pn->pn_name, iter->pn_name) != 0,
+		    ("%s(): homonymous siblings", __func__));
+		if (pn->pn_type == pfstype_procdir)
+			KASSERT(iter->pn_type != pfstype_procdir,
+			    ("%s(): sibling process directories", __func__));
+	}
+#endif
 
-	node->pn_info = parent->pn_info;
-	node->pn_parent = parent;
-	node->pn_next = parent->pn_nodes;
-	parent->pn_nodes = node;
-	/* Propagate flag to all child nodes (and thus their vnodes) */
-	if ((parent->pn_flags & PFS_PROCDEP) != 0)
-		node->pn_flags |= PFS_PROCDEP;
+	pn->pn_parent = parent;
+	pfs_fileno_alloc(pn);
 
-	return (0);
+	pfs_lock(parent);
+	pn->pn_next = parent->pn_nodes;
+	if ((parent->pn_flags & PFS_PROCDEP) != 0)
+		pn->pn_flags |= PFS_PROCDEP;
+	parent->pn_nodes = pn;
+	pfs_unlock(parent);
 }
 
 /*
- * Add . and .. to a directory
+ * Detach a node from its aprent
  */
-static int
-_pfs_fixup_dir(struct pfs_node *parent)
+static void
+pfs_detach_node(struct pfs_node *pn)
 {
-	struct pfs_node *dir;
-
-	MALLOC(dir, struct pfs_node *, sizeof *dir,
-	    M_PFSNODES, M_WAITOK|M_ZERO);
-	dir->pn_name[0] = '.';
-	dir->pn_type = pfstype_this;
-
-	if (_pfs_add_node(parent, dir) != 0) {
-		FREE(dir, M_PFSNODES);
-		return (-1);
+	struct pfs_node *parent = pn->pn_parent;
+	struct pfs_node **iter;
+
+	KASSERT(parent != NULL, ("%s(): node has no parent", __func__));
+	KASSERT(parent->pn_info == pn->pn_info,
+	    ("%s(): parent has different pn_info", __func__));
+
+	pfs_lock(parent);
+	iter = &parent->pn_nodes;
+	while (*iter != NULL) {
+		if (*iter == pn) {
+			*iter = pn->pn_next;
+			break;
+		}
+		iter = &(*iter)->pn_next;
 	}
+	pn->pn_parent = NULL;
+	pfs_unlock(parent);
+}
 
-	MALLOC(dir, struct pfs_node *, sizeof *dir,
-	    M_PFSNODES, M_WAITOK|M_ZERO);
-	dir->pn_name[0] = dir->pn_name[1] = '.';
-	dir->pn_type = pfstype_parent;
-
-	if (_pfs_add_node(parent, dir) != 0) {
-		FREE(dir, M_PFSNODES);
-		return (-1);
-	}
+/*
+ * Add . and .. to a directory
+ */
+static void
+pfs_fixup_dir(struct pfs_node *parent)
+{
+	struct pfs_node *pn;
 
-	return (0);
+	pn = pfs_alloc_node(parent->pn_info, ".", pfstype_this);
+	pfs_add_node(parent, pn);
+	pn = pfs_alloc_node(parent->pn_info, "..", pfstype_parent);
+	pfs_add_node(parent, pn);
 }
 
 /*
@@ -123,31 +175,18 @@ pfs_create_dir(struct pfs_node *parent, const char *name,
 	       pfs_attr_t attr, pfs_vis_t vis, pfs_destroy_t destroy,
 	       int flags)
 {
-	struct pfs_node *dir;
-
-	KASSERT(strlen(name) < PFS_NAMELEN,
-	    ("%s(): node name is too long", __func__));
-
-	MALLOC(dir, struct pfs_node *, sizeof *dir,
-	    M_PFSNODES, M_WAITOK|M_ZERO);
-	strcpy(dir->pn_name, name);
-	dir->pn_type = (flags & PFS_PROCDEP) ? pfstype_procdir : pfstype_dir;
-	dir->pn_attr = attr;
-	dir->pn_vis = vis;
-	dir->pn_destroy = destroy;
-	dir->pn_flags = flags;
-
-	if (_pfs_add_node(parent, dir) != 0) {
-		FREE(dir, M_PFSNODES);
-		return (NULL);
-	}
-
-	if (_pfs_fixup_dir(dir) != 0) {
-		pfs_destroy(dir);
-		return (NULL);
-	}
-
-	return (dir);
+	struct pfs_node *pn;
+
+	pn = pfs_alloc_node(parent->pn_info, name,
+	    (flags & PFS_PROCDEP) ? pfstype_procdir : pfstype_dir);
+	pn->pn_attr = attr;
+	pn->pn_vis = vis;
+	pn->pn_destroy = destroy;
+	pn->pn_flags = flags;
+	pfs_add_node(parent, pn);
+	pfs_fixup_dir(pn);
+
+	return (pn);
 }
 
 /*
@@ -158,27 +197,17 @@ pfs_create_file(struct pfs_node *parent, const char *name, pfs_fill_t fill,
 		pfs_attr_t attr, pfs_vis_t vis, pfs_destroy_t destroy,
 		int flags)
 {
-	struct pfs_node *node;
-
-	KASSERT(strlen(name) < PFS_NAMELEN,
-	    ("%s(): node name is too long", __func__));
+	struct pfs_node *pn;
 
-	MALLOC(node, struct pfs_node *, sizeof *node,
-	    M_PFSNODES, M_WAITOK|M_ZERO);
-	strcpy(node->pn_name, name);
-	node->pn_type = pfstype_file;
-	node->pn_func = fill;
-	node->pn_attr = attr;
-	node->pn_vis = vis;
-	node->pn_destroy = destroy;
-	node->pn_flags = flags;
-
-	if (_pfs_add_node(parent, node) != 0) {
-		FREE(node, M_PFSNODES);
-		return (NULL);
-	}
+	pn = pfs_alloc_node(parent->pn_info, name, pfstype_file);
+	pn->pn_fill = fill;
+	pn->pn_attr = attr;
+	pn->pn_vis = vis;
+	pn->pn_destroy = destroy;
+	pn->pn_flags = flags;
+	pfs_add_node(parent, pn);
 
-	return (node);
+	return (pn);
 }
 
 /*
@@ -189,13 +218,17 @@ pfs_create_link(struct pfs_node *parent, const char *name, pfs_fill_t fill,
 		pfs_attr_t attr, pfs_vis_t vis, pfs_destroy_t destroy,
 		int flags)
 {
-	struct pfs_node *node;
+	struct pfs_node *pn;
 
-	node = pfs_create_file(parent, name, fill, attr, vis, destroy, flags);
-	if (node == NULL)
-		return (NULL);
-	node->pn_type = pfstype_symlink;
-	return (node);
+	pn = pfs_alloc_node(parent->pn_info, name, pfstype_symlink);
+	pn->pn_fill = fill;
+	pn->pn_attr = attr;
+	pn->pn_vis = vis;
+	pn->pn_destroy = destroy;
+	pn->pn_flags = flags;
+	pfs_add_node(parent, pn);
+
+	return (pn);
 }
 
 /*
@@ -204,57 +237,60 @@ pfs_create_link(struct pfs_node *parent, const char *name, pfs_fill_t fill,
 struct pfs_node *
 pfs_find_node(struct pfs_node *parent, const char *name)
 {
-	struct pfs_node *node;
+	struct pfs_node *pn;
 
-	for (node = parent->pn_nodes; node != NULL; node = node->pn_next)
-		if (strcmp(node->pn_name, name) == 0)
+	pfs_lock(parent);
+	for (pn = parent->pn_nodes; pn != NULL; pn = pn->pn_next)
+		if (strcmp(pn->pn_name, name) == 0)
 			break;
-	return (node);
+	pfs_unlock(parent);
+	return (pn);
 }
 
 /*
- * Destroy a node or a tree of nodes
+ * Destroy a node and all its descendants.  If the node to be destroyed
+ * has a parent, the parent's mutex must be held.
  */
 int
-pfs_destroy(struct pfs_node *node)
+pfs_destroy(struct pfs_node *pn)
 {
-	struct pfs_node *parent, **rover;
+	struct pfs_node *iter;
 
-	KASSERT(node != NULL,
+	KASSERT(pn != NULL,
 	    ("%s(): node is NULL", __func__));
-	KASSERT(node->pn_info != NULL,
+	KASSERT(pn->pn_info != NULL,
 	    ("%s(): node has no pn_info", __func__));
 
+	if (pn->pn_parent)
+		pfs_detach_node(pn);
+
 	/* destroy children */
-	if (node->pn_type == pfstype_dir ||
-	    node->pn_type == pfstype_procdir ||
-	    node->pn_type == pfstype_root)
-		while (node->pn_nodes != NULL)
-			pfs_destroy(node->pn_nodes);
-
-	/* unlink from parent */
-	if ((parent = node->pn_parent) != NULL) {
-		KASSERT(parent->pn_info == node->pn_info,
-		    ("%s(): parent has different pn_info", __func__));
-		rover = &parent->pn_nodes;
-		while (*rover != NULL) {
-			if (*rover == node) {
-				*rover = node->pn_next;
-				break;
-			}
-			rover = &(*rover)->pn_next;
+	if (pn->pn_type == pfstype_dir ||
+	    pn->pn_type == pfstype_procdir ||
+	    pn->pn_type == pfstype_root) {
+		pfs_lock(pn);
+		while (pn->pn_nodes != NULL) {
+			iter = pn->pn_nodes;
+			pn->pn_nodes = iter->pn_next;
+			iter->pn_parent = NULL;
+			pfs_unlock(pn);
+			pfs_destroy(iter);
+			pfs_lock(pn);
 		}
+		pfs_unlock(pn);
 	}
 
+	/* revoke vnodes and fileno */
+	pfs_purge(pn);
+
 	/* callback to free any private resources */
-	if (node->pn_destroy != NULL)
-		(node->pn_destroy)(node);
+	if (pn->pn_destroy != NULL)
+		pn_destroy(pn);
 
-	/* revoke fileno and vnodes and release memory */
-	if (node->pn_fileno)
-		pfs_fileno_free(node);
-	pfs_purge(node);
-	FREE(node, M_PFSNODES);
+	/* destroy the node */
+	pfs_fileno_free(pn);
+	mtx_destroy(&pn->pn_mutex);
+	FREE(pn, M_PFSNODES);
 
 	return (0);
 }
@@ -272,10 +308,7 @@ pfs_mount(struct pfs_info *pi, struct mount *mp, struct thread *td)
 
 	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);
@@ -294,12 +327,15 @@ pfs_mount(struct pfs_info *pi, struct mount *mp, struct thread *td)
 }
 
 /*
- * Compatibility shim for old mount(2) system call.
+ * Compatibility shim for old mount(2) system call
  */
 int
 pfs_cmount(struct mntarg *ma, void *data, int flags, struct thread *td)
 {
-	return kernel_mount(ma, flags);
+	int error;
+
+	error = kernel_mount(ma, flags);
+	return (error);
 }
 
 /*
@@ -308,13 +344,8 @@ pfs_cmount(struct mntarg *ma, void *data, int flags, struct thread *td)
 int
 pfs_unmount(struct mount *mp, int mntflags, struct thread *td)
 {
-	struct pfs_info *pi;
 	int error;
 
-	pi = (struct pfs_info *)mp->mnt_data;
-
-	/* XXX do stuff with pi... */
-
 	error = vflush(mp, 0, (mntflags & MNT_FORCE) ?  FORCECLOSE : 0, td);
 	return (error);
 }
@@ -328,7 +359,7 @@ pfs_root(struct mount *mp, int flags, struct vnode **vpp, struct thread *td)
 	struct pfs_info *pi;
 
 	pi = (struct pfs_info *)mp->mnt_data;
-	return pfs_vncache_alloc(mp, vpp, pi->pi_root, NO_PID);
+	return (pfs_vncache_alloc(mp, vpp, pi->pi_root, NO_PID));
 }
 
 /*
@@ -352,17 +383,13 @@ pfs_init(struct pfs_info *pi, struct vfsconf *vfc)
 
 	mtx_assert(&Giant, MA_OWNED);
 
+	pfs_fileno_init(pi);
+
 	/* set up the root diretory */
-	MALLOC(root, struct pfs_node *, sizeof *root,
-	    M_PFSNODES, M_WAITOK|M_ZERO);
-	root->pn_type = pfstype_root;
-	root->pn_name[0] = '/';
-	root->pn_info = pi;
-	if (_pfs_fixup_dir(root) != 0) {
-		FREE(root, M_PFSNODES);
-		return (ENODEV); /* XXX not really the right errno */
-	}
+	root = pfs_alloc_node(pi, "/", pfstype_root);
 	pi->pi_root = root;
+	pfs_fileno_alloc(root);
+	pfs_fixup_dir(root);
 
 	/* construct file hierarchy */
 	error = (pi->pi_init)(pi, vfc);
@@ -372,7 +399,6 @@ pfs_init(struct pfs_info *pi, struct vfsconf *vfc)
 		return (error);
 	}
 
-	pfs_fileno_init(pi);
 	if (bootverbose)
 		printf("%s registered\n", pi->pi_name);
 	return (0);
diff --git a/sys/fs/pseudofs/pseudofs.h b/sys/fs/pseudofs/pseudofs.h
index 4fa3311..c9e1697 100644
--- a/sys/fs/pseudofs/pseudofs.h
+++ b/sys/fs/pseudofs/pseudofs.h
@@ -73,7 +73,6 @@ typedef enum {
 #define	PFS_RAWWR	0x0008	/* raw writer */
 #define PFS_RAW		(PFS_RAWRD|PFS_RAWWR)
 #define PFS_PROCDEP	0x0010	/* process-dependent */
-#define PFS_DISABLED	0x8000	/* node is disabled */
 
 /*
  * Data structures
@@ -87,27 +86,35 @@ struct pfs_bitmap;
  */
 #define PFS_INIT_ARGS \
 	struct pfs_info *pi, struct vfsconf *vfc
+#define PFS_INIT_ARGNAMES \
+	pi, vfc
 #define PFS_INIT_PROTO(name) \
 	int name(PFS_INIT_ARGS);
 typedef int (*pfs_init_t)(PFS_INIT_ARGS);
 
 /*
  * Filler callback
+ * Called with proc held but unlocked
  */
 #define PFS_FILL_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn, \
 	struct sbuf *sb, struct uio *uio
+#define PFS_FILL_ARGNAMES \
+	td, p, pn, sb, uio
 #define PFS_FILL_PROTO(name) \
 	int name(PFS_FILL_ARGS);
 typedef int (*pfs_fill_t)(PFS_FILL_ARGS);
 
 /*
  * Attribute callback
+ * Called with proc locked
  */
 struct vattr;
 #define PFS_ATTR_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn, \
 	struct vattr *vap
+#define PFS_ATTR_ARGNAMES \
+	td, p, pn, vap
 #define PFS_ATTR_PROTO(name) \
 	int name(PFS_ATTR_ARGS);
 typedef int (*pfs_attr_t)(PFS_ATTR_ARGS);
@@ -116,30 +123,39 @@ struct pfs_bitmap;		/* opaque */
 
 /*
  * Visibility callback
+ * Called with proc locked
  */
 #define PFS_VIS_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn
+#define PFS_VIS_ARGNAMES \
+	td, p, pn
 #define PFS_VIS_PROTO(name) \
 	int name(PFS_VIS_ARGS);
 typedef int (*pfs_vis_t)(PFS_VIS_ARGS);
 
 /*
  * Ioctl callback
+ * Called with proc locked
  */
 #define PFS_IOCTL_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn, \
 	unsigned long cmd, void *data
+#define PFS_IOCTL_ARGNAMES \
+	td, p, pn, cmd, data
 #define PFS_IOCTL_PROTO(name) \
 	int name(PFS_IOCTL_ARGS);
 typedef int (*pfs_ioctl_t)(PFS_IOCTL_ARGS);
 
 /*
  * Getextattr callback
+ * Called with proc locked
  */
 #define PFS_GETEXTATTR_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn, \
 	int attrnamespace, const char *name, struct uio *uio,	\
 	size_t *size, struct ucred *cred
+#define PFS_GETEXTATTR_ARGNAMES \
+	td, p, pn, attrnamespace, name, uio, size, cred
 #define PFS_GETEXTATTR_PROTO(name) \
 	int name(PFS_GETEXTATTR_ARGS);
 struct ucred;
@@ -147,9 +163,12 @@ typedef int (*pfs_getextattr_t)(PFS_GETEXTATTR_ARGS);
 
 /*
  * Last-close callback
+ * Called with proc locked
  */
 #define PFS_CLOSE_ARGS \
 	struct thread *td, struct proc *p, struct pfs_node *pn
+#define PFS_CLOSE_ARGNAMES \
+	td, p, pn
 #define PFS_CLOSE_PROTO(name) \
 	int name(PFS_CLOSE_ARGS);
 typedef int (*pfs_close_t)(PFS_CLOSE_ARGS);
@@ -159,6 +178,8 @@ typedef int (*pfs_close_t)(PFS_CLOSE_ARGS);
  */
 #define PFS_DESTROY_ARGS \
 	struct pfs_node *pn
+#define PFS_DESTROY_ARGNAMES \
+	pn
 #define PFS_DESTROY_PROTO(name) \
 	int name(PFS_DESTROY_ARGS);
 typedef int (*pfs_destroy_t)(PFS_DESTROY_ARGS);
@@ -183,30 +204,38 @@ struct pfs_info {
 
 /*
  * pfs_node: describes a node (file or directory) within a pseudofs
+ *
+ * - Fields marked (o) are protected by the node's own mutex.
+ * - Fields marked (p) are protected by the node's parent's mutex.
+ * - Remaining fields are not protected by any lock and are assumed to be
+ *   immutable once the node has been created.
+ *
+ * To prevent deadlocks, if a node's mutex is to be held at the same time
+ * as its parent's (e.g. when adding or removing nodes to a directory),
+ * the parent's mutex must always be acquired first.  Unfortunately, this
+ * is not enforcable by WITNESS.
  */
 struct pfs_node {
 	char			 pn_name[PFS_NAMELEN];
 	pfs_type_t		 pn_type;
-	union {
-		void		*_pn_dummy;
-		pfs_fill_t	 _pn_func;
-		struct pfs_node	*_pn_nodes;
-	} u1;
-#define pn_func		u1._pn_func
-#define pn_nodes	u1._pn_nodes
+	int			 pn_flags;
+	struct mtx		 pn_mutex;
+	void			*pn_data;		/* (o) */
+
+	pfs_fill_t		 pn_fill;
 	pfs_ioctl_t		 pn_ioctl;
 	pfs_close_t		 pn_close;
 	pfs_attr_t		 pn_attr;
 	pfs_vis_t		 pn_vis;
 	pfs_getextattr_t	 pn_getextattr;
 	pfs_destroy_t		 pn_destroy;
-	void			*pn_data;
-	int			 pn_flags;
 
 	struct pfs_info		*pn_info;
-	struct pfs_node		*pn_parent;
-	struct pfs_node		*pn_next;
-	u_int32_t		 pn_fileno;
+	u_int32_t		 pn_fileno;		/* (o) */
+
+	struct pfs_node		*pn_parent;		/* (o) */
+	struct pfs_node		*pn_nodes;		/* (o) */
+	struct pfs_node		*pn_next;		/* (p) */
 };
 
 /*
@@ -241,8 +270,6 @@ struct pfs_node	*pfs_create_link(struct pfs_node *parent, const char *name,
 				 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 155b0f5..d9dc6d2 100644
--- a/sys/fs/pseudofs/pseudofs_fileno.c
+++ b/sys/fs/pseudofs/pseudofs_fileno.c
@@ -77,9 +77,11 @@ void
 pfs_fileno_alloc(struct pfs_node *pn)
 {
 
-	/* make sure our parent has a file number */
-	if (pn->pn_parent && !pn->pn_parent->pn_fileno)
-		pfs_fileno_alloc(pn->pn_parent);
+	if (pn->pn_parent)
+		PFS_TRACE(("%s/%s", pn->pn_parent->pn_name, pn->pn_name));
+	else
+		PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	switch (pn->pn_type) {
 	case pfstype_root:
@@ -94,28 +96,28 @@ pfs_fileno_alloc(struct pfs_node *pn)
 		break;
 	case pfstype_this:
 		KASSERT(pn->pn_parent != NULL,
-		    ("pfstype_this node has no parent"));
+		    ("%s(): pfstype_this node has no parent", __func__));
 		pn->pn_fileno = pn->pn_parent->pn_fileno;
 		break;
 	case pfstype_parent:
 		KASSERT(pn->pn_parent != NULL,
-		    ("pfstype_parent node has no parent"));
-		if (pn->pn_parent == pn->pn_info->pi_root) {
+		    ("%s(): pfstype_parent node has no parent", __func__));
+		if (pn->pn_parent->pn_type == pfstype_root) {
 			pn->pn_fileno = pn->pn_parent->pn_fileno;
 			break;
 		}
 		KASSERT(pn->pn_parent->pn_parent != NULL,
-		    ("pfstype_parent node has no grandparent"));
+		    ("%s(): pfstype_parent node has no grandparent", __func__));
 		pn->pn_fileno = pn->pn_parent->pn_parent->pn_fileno;
 		break;
 	case pfstype_none:
 		KASSERT(0,
-		    ("pfs_fileno_alloc() called for pfstype_none node"));
+		    ("%s(): pfstype_none node", __func__));
 		break;
 	}
 
 #if 0
-	printf("pfs_fileno_alloc(): %s: ", pn->pn_info->pi_name);
+	printf("%s(): %s: ", __func__, pn->pn_info->pi_name);
 	if (pn->pn_parent) {
 		if (pn->pn_parent->pn_parent) {
 			printf("%s/", pn->pn_parent->pn_parent->pn_name);
@@ -133,6 +135,8 @@ void
 pfs_fileno_free(struct pfs_node *pn)
 {
 
+	pfs_assert_not_owned(pn);
+
 	switch (pn->pn_type) {
 	case pfstype_root:
 		/* not allocated from unrhdr */
diff --git a/sys/fs/pseudofs/pseudofs_internal.h b/sys/fs/pseudofs/pseudofs_internal.h
index 8f213ed..d52ae6e 100644
--- a/sys/fs/pseudofs/pseudofs_internal.h
+++ b/sys/fs/pseudofs/pseudofs_internal.h
@@ -64,4 +64,148 @@ void	 pfs_fileno_uninit	(struct pfs_info *);
 void	 pfs_fileno_alloc	(struct pfs_node *);
 void	 pfs_fileno_free	(struct pfs_node *);
 
+/*
+ * Debugging
+ */
+#ifdef PSEUDOFS_TRACE
+extern int pfs_trace;
+
+#define PFS_TRACE(foo) \
+	do { \
+		if (pfs_trace) { \
+			printf("%s(): line %d: ", __func__, __LINE__); \
+			printf foo ; \
+			printf("\n"); \
+		} \
+	} while (0)
+#define PFS_RETURN(err) \
+	do { \
+		if (pfs_trace) { \
+			printf("%s(): line %d: returning %d\n", \
+			    __func__, __LINE__, err); \
+		} \
+		return (err); \
+	} while (0)
+#else
+#define PFS_TRACE(foo) \
+	do { /* nothing */ } while (0)
+#define PFS_RETURN(err) \
+	return (err)
+#endif
+
+/*
+ * Inline helpers for locking
+ */
+static inline void
+pfs_lock(struct pfs_node *pn)
+{
+
+	mtx_lock(&pn->pn_mutex);
+}
+
+static inline void
+pfs_unlock(struct pfs_node *pn)
+{
+
+	mtx_unlock(&pn->pn_mutex);
+}
+
+static inline void
+pfs_assert_owned(struct pfs_node *pn)
+{
+
+	mtx_assert(&pn->pn_mutex, MA_OWNED);
+}
+
+static inline void
+pfs_assert_not_owned(struct pfs_node *pn)
+{
+
+	mtx_assert(&pn->pn_mutex, MA_NOTOWNED);
+}
+
+static inline int
+pn_fill(PFS_FILL_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_fill != NULL, ("%s(): no callback", __func__));
+	if (p != NULL) {
+		PROC_LOCK_ASSERT(p, MA_NOTOWNED);
+		PROC_ASSERT_HELD(p);
+	}
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_fill)(PFS_FILL_ARGNAMES));
+}
+
+static inline int
+pn_attr(PFS_ATTR_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_attr != NULL, ("%s(): no callback", __func__));
+	if (p != NULL)
+		PROC_LOCK_ASSERT(p, MA_OWNED);
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_attr)(PFS_ATTR_ARGNAMES));
+}
+
+static inline int
+pn_vis(PFS_VIS_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_vis != NULL, ("%s(): no callback", __func__));
+	KASSERT(p != NULL, ("%s(): no process", __func__));
+	PROC_LOCK_ASSERT(p, MA_OWNED);
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_vis)(PFS_VIS_ARGNAMES));
+}
+
+static inline int
+pn_ioctl(PFS_IOCTL_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_ioctl != NULL, ("%s(): no callback", __func__));
+	if (p != NULL)
+		PROC_LOCK_ASSERT(p, MA_OWNED);
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_ioctl)(PFS_IOCTL_ARGNAMES));
+}
+
+static inline int
+pn_getextattr(PFS_GETEXTATTR_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_getextattr != NULL, ("%s(): no callback", __func__));
+	if (p != NULL)
+		PROC_LOCK_ASSERT(p, MA_OWNED);
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_getextattr)(PFS_GETEXTATTR_ARGNAMES));
+}
+
+static inline int
+pn_close(PFS_CLOSE_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_close != NULL, ("%s(): no callback", __func__));
+	if (p != NULL)
+		PROC_LOCK_ASSERT(p, MA_OWNED);
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_close)(PFS_CLOSE_ARGNAMES));
+}
+
+static inline int
+pn_destroy(PFS_DESTROY_ARGS)
+{
+
+	PFS_TRACE(("%s", pn->pn_name));
+	KASSERT(pn->pn_destroy != NULL, ("%s(): no callback", __func__));
+	pfs_assert_not_owned(pn);
+	return ((pn->pn_destroy)(PFS_DESTROY_ARGNAMES));
+}
+
 #endif
diff --git a/sys/fs/pseudofs/pseudofs_vncache.c b/sys/fs/pseudofs/pseudofs_vncache.c
index 7544cb1..1310013 100644
--- a/sys/fs/pseudofs/pseudofs_vncache.c
+++ b/sys/fs/pseudofs/pseudofs_vncache.c
@@ -235,7 +235,7 @@ pfs_vncache_free(struct vnode *vp)
 }
 
 /*
- * Purge the cache of dead / disabled entries
+ * Purge the cache of dead entries
  *
  * This is extremely inefficient due to the fact that vgone() not only
  * indirectly modifies the vnode cache, but may also sleep.  We can
@@ -298,26 +298,3 @@ pfs_exit(void *arg, struct proc *p)
 		pfs_purge(NULL);
 	mtx_unlock(&Giant);
 }
-
-/*
- * Disable a pseudofs node, and free all vnodes associated with it
- */
-int
-pfs_disable(struct pfs_node *pn)
-{
-	if (pn->pn_flags & PFS_DISABLED)
-		return (0);
-	pn->pn_flags |= PFS_DISABLED;
-	pfs_purge(pn);
-	return (0);
-}
-
-/*
- * Re-enable a disabled pseudofs node
- */
-int
-pfs_enable(struct pfs_node *pn)
-{
-	pn->pn_flags &= ~PFS_DISABLED;
-	return (0);
-}
diff --git a/sys/fs/pseudofs/pseudofs_vnops.c b/sys/fs/pseudofs/pseudofs_vnops.c
index e36e102..12ddab7 100644
--- a/sys/fs/pseudofs/pseudofs_vnops.c
+++ b/sys/fs/pseudofs/pseudofs_vnops.c
@@ -52,53 +52,44 @@ __FBSDID("$FreeBSD$");
 #include <fs/pseudofs/pseudofs.h>
 #include <fs/pseudofs/pseudofs_internal.h>
 
-#ifdef PSEUDOFS_TRACE
-static int pfs_trace;
-SYSCTL_INT(_vfs_pfs, OID_AUTO, trace, CTLFLAG_RW, &pfs_trace, 0,
-    "enable tracing of pseudofs vnode operations");
-
-#define PFS_TRACE(foo) \
-	do { \
-		if (pfs_trace) { \
-			printf("%s(): line %d: ", __func__, __LINE__); \
-			printf foo ; \
-			printf("\n"); \
-		} \
-	} while (0)
-#define PFS_RETURN(err) \
-	do { \
-		if (pfs_trace) { \
-			printf("%s(): line %d: returning %d\n", \
-			    __func__, __LINE__, err); \
-		} \
-		return (err); \
-	} while (0)
-#else
-#define PFS_TRACE(foo) \
-	do { /* nothing */ } while (0)
-#define PFS_RETURN(err) \
-	return (err)
-#endif
-
 /*
- *
+ * Returns the fileno, adjusted for target pid
  */
 static uint32_t
-pfs_fileno(struct pfs_node *pn, pid_t pid)
+pn_fileno(struct pfs_node *pn, pid_t pid)
 {
-	if (!pn->pn_fileno)
-		pfs_fileno_alloc(pn);
+
+	KASSERT(pn->pn_fileno > 0,
+	    ("%s(): no fileno allocated", __func__));
 	if (pid != NO_PID)
 		return (pn->pn_fileno * NO_PID + pid);
 	return (pn->pn_fileno);
 }
 
 /*
- * Returns non-zero if given file is visible to given process.  If the 'p'
- * parameter is non-NULL, then it will hold a pointer to the process the
- * given file belongs to on return and the process will be locked.
+ * Returns non-zero if given file is visible to given thread.
  */
 static int
+pfs_visible_proc(struct thread *td, struct pfs_node *pn, struct proc *proc)
+{
+	int visible;
+
+	if (proc == NULL)
+		return (0);
+
+	PROC_LOCK_ASSERT(proc, MA_OWNED);
+
+	visible = ((proc->p_flag & P_WEXIT) == 0);
+	if (visible)
+		visible = (p_cansee(td, proc) == 0);
+	if (visible && pn->pn_vis != NULL)
+		visible = pn_vis(td, proc, pn);
+	if (!visible)
+		return (0);
+	return (1);
+}
+
+static int
 pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid, struct proc **p)
 {
 	struct proc *proc;
@@ -106,30 +97,21 @@ pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid, struct proc **p)
 	PFS_TRACE(("%s (pid: %d, req: %d)",
 	    pn->pn_name, pid, td->td_proc->p_pid));
 
-	if (pn->pn_flags & PFS_DISABLED)
+	if (p)
+		*p = NULL;
+	if (pid == NO_PID)
+		PFS_RETURN (1);
+	if ((proc = pfind(pid)) == NULL)
 		PFS_RETURN (0);
-
-	if (pid != NO_PID) {
-		if ((proc = pfind(pid)) == NULL)
-			PFS_RETURN (0);
-		if (proc->p_flag & P_WEXIT) {
-			PROC_UNLOCK(proc);
-			PFS_RETURN (0);
-		}
-		if (p_cansee(td, proc) != 0 ||
-		    (pn->pn_vis != NULL && !(pn->pn_vis)(td, proc, pn))) {
-			PROC_UNLOCK(proc);
-			PFS_RETURN (0);
-		}
-		if (p) {
-			/* We return with the process locked to avoid races. */
+	if (pfs_visible_proc(td, pn, proc)) {
+		if (p)
 			*p = proc;
-		} else
+		else
 			PROC_UNLOCK(proc);
-	} else
-		if (p)
-			*p = NULL;
-	PFS_RETURN (1);
+		PFS_RETURN (1);
+	}
+	PROC_UNLOCK(proc);
+	PFS_RETURN (0);
 }
 
 /*
@@ -139,10 +121,11 @@ static int
 pfs_access(struct vop_access_args *va)
 {
 	struct vnode *vn = va->a_vp;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct vattr vattr;
 	int error;
 
-	PFS_TRACE((((struct pfs_vdata *)vn->v_data)->pvd_pn->pn_name));
+	PFS_TRACE(("%s", pvd->pvd_pn->pn_name));
 
 	error = VOP_GETATTR(vn, &vattr, va->a_cred, va->a_td);
 	if (error)
@@ -159,12 +142,13 @@ static int
 pfs_close(struct vop_close_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct proc *proc;
 	int error;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	/*
 	 * Do nothing unless this is the last close and the node has a
@@ -173,12 +157,13 @@ pfs_close(struct vop_close_args *va)
 	if (vrefcnt(vn) > 1 || pn->pn_close == NULL)
 		PFS_RETURN (0);
 
-	if (pvd->pvd_pid != NO_PID)
+	if (pvd->pvd_pid != NO_PID) {
 		proc = pfind(pvd->pvd_pid);
-	else
+	} else {
 		proc = NULL;
+	}
 
-	error = (pn->pn_close)(va->a_td, proc, pn);
+	error = pn_close(va->a_td, proc, pn);
 
 	if (proc != NULL)
 		PROC_UNLOCK(proc);
@@ -193,20 +178,21 @@ static int
 pfs_getattr(struct vop_getattr_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct vattr *vap = va->a_vap;
 	struct proc *proc;
 	int error = 0;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (ENOENT);
 
 	VATTR_NULL(vap);
 	vap->va_type = vn->v_type;
-	vap->va_fileid = pfs_fileno(pn, pvd->pvd_pid);
+	vap->va_fileid = pn_fileno(pn, pvd->pvd_pid);
 	vap->va_flags = 0;
 	vap->va_blocksize = PAGE_SIZE;
 	vap->va_bytes = vap->va_size = 0;
@@ -219,6 +205,11 @@ pfs_getattr(struct vop_getattr_args *va)
 	case pfstype_procdir:
 	case pfstype_root:
 	case pfstype_dir:
+#if 0
+		pfs_lock(pn);
+		/* compute link count */
+		pfs_unlock(pn);
+#endif
 		vap->va_mode = 0555;
 		break;
 	case pfstype_file:
@@ -235,7 +226,7 @@ pfs_getattr(struct vop_getattr_args *va)
 		vap->va_uid = proc->p_ucred->cr_ruid;
 		vap->va_gid = proc->p_ucred->cr_rgid;
 		if (pn->pn_attr != NULL)
-			error = (pn->pn_attr)(va->a_td, proc, pn, vap);
+			error = pn_attr(va->a_td, proc, pn, vap);
 		PROC_UNLOCK(proc);
 	} else {
 		vap->va_uid = 0;
@@ -252,12 +243,13 @@ static int
 pfs_ioctl(struct vop_ioctl_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct proc *proc;
 	int error;
 
 	PFS_TRACE(("%s: %lx", pn->pn_name, va->a_command));
+	pfs_assert_not_owned(pn);
 
 	if (vn->v_type != VREG)
 		PFS_RETURN (EINVAL);
@@ -272,15 +264,10 @@ pfs_ioctl(struct vop_ioctl_args *va)
 	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
 
-	if (proc != NULL) {
-		_PHOLD(proc);
-		PROC_UNLOCK(proc);
-	}
-
-	error = (pn->pn_ioctl)(curthread, proc, pn, va->a_command, va->a_data);
+	error = pn_ioctl(curthread, proc, pn, va->a_command, va->a_data);
 
 	if (proc != NULL)
-		PRELE(proc);
+		PROC_UNLOCK(proc);
 
 	PFS_RETURN (error);
 }
@@ -292,12 +279,13 @@ static int
 pfs_getextattr(struct vop_getextattr_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct proc *proc;
 	int error;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	/*
 	 * This is necessary because either process' privileges may
@@ -306,23 +294,17 @@ pfs_getextattr(struct vop_getextattr_args *va)
 	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
 
-	if (pn->pn_getextattr == NULL) {
-		if (proc != NULL)
-			PROC_UNLOCK(proc);
-		PFS_RETURN (EOPNOTSUPP);
-	}
-
-	if (proc != NULL) {
-		_PHOLD(proc);
-		PROC_UNLOCK(proc);
-	}
-
-	error = (pn->pn_getextattr)(curthread, proc, pn, va->a_attrnamespace,
-	    va->a_name, va->a_uio, va->a_size, va->a_cred);
+	if (pn->pn_getextattr == NULL)
+		error = EOPNOTSUPP;
+	else
+		error = pn_getextattr(curthread, proc, pn,
+		    va->a_attrnamespace, va->a_name, va->a_uio,
+		    va->a_size, va->a_cred);
 
 	if (proc != NULL)
-		PRELE(proc);
+		PROC_UNLOCK(proc);
 
+	pfs_unlock(pn);
 	PFS_RETURN (error);
 }
 
@@ -335,14 +317,15 @@ pfs_lookup(struct vop_cachedlookup_args *va)
 	struct vnode *vn = va->a_dvp;
 	struct vnode **vpp = va->a_vpp;
 	struct componentname *cnp = va->a_cnp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pd = pvd->pvd_pn;
 	struct pfs_node *pn, *pdn = NULL;
 	pid_t pid = pvd->pvd_pid;
 	char *pname;
-	int error, i, namelen;
+	int error, i, namelen, visible;
 
 	PFS_TRACE(("%.*s", (int)cnp->cn_namelen, cnp->cn_nameptr));
+	pfs_assert_not_owned(pd);
 
 	if (vn->v_type != VDIR)
 		PFS_RETURN (ENOTDIR);
@@ -383,7 +366,8 @@ pfs_lookup(struct vop_cachedlookup_args *va)
 		if (pd->pn_type == pfstype_root)
 			PFS_RETURN (EIO);
 		VOP_UNLOCK(vn, 0, cnp->cn_thread);
-		KASSERT(pd->pn_parent, ("non-root directory has no parent"));
+		KASSERT(pd->pn_parent != NULL,
+		    ("%s(): non-root directory has no parent", __func__));
 		/*
 		 * This one is tricky.  Descendents of procdir nodes
 		 * inherit their parent's process affinity, but
@@ -395,17 +379,23 @@ pfs_lookup(struct vop_cachedlookup_args *va)
 		 */
 		if (pd->pn_type == pfstype_procdir)
 			pid = NO_PID;
+		pfs_lock(pd);
 		pn = pd->pn_parent;
+		pfs_unlock(pd);
 		goto got_pnode;
 	}
 
+	pfs_lock(pd);
+
 	/* named node */
 	for (pn = pd->pn_nodes; pn != NULL; pn = pn->pn_next)
 		if (pn->pn_type == pfstype_procdir)
 			pdn = pn;
 		else if (pn->pn_name[namelen] == '\0' &&
-		    bcmp(pname, pn->pn_name, namelen) == 0)
+		    bcmp(pname, pn->pn_name, namelen) == 0) {
+			pfs_unlock(pd);
 			goto got_pnode;
+		}
 
 	/* process dependent node */
 	if ((pn = pdn) != NULL) {
@@ -413,15 +403,21 @@ pfs_lookup(struct vop_cachedlookup_args *va)
 		for (pid = 0, i = 0; i < namelen && isdigit(pname[i]); ++i)
 			if ((pid = pid * 10 + pname[i] - '0') > PID_MAX)
 				break;
-		if (i == cnp->cn_namelen)
+		if (i == cnp->cn_namelen) {
+			pfs_unlock(pd);
 			goto got_pnode;
+		}
 	}
 
+	pfs_unlock(pd);
+
 	PFS_RETURN (ENOENT);
+
  got_pnode:
-	if (pn != pd->pn_parent && !pn->pn_parent)
-		pn->pn_parent = pd;
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, NULL)) {
+	pfs_assert_not_owned(pd);
+	pfs_assert_not_owned(pn);
+	visible = pfs_visible(curthread, pn, pvd->pvd_pid, NULL);
+	if (!visible) {
 		error = ENOENT;
 		goto failed;
 	}
@@ -448,24 +444,12 @@ static int
 pfs_open(struct vop_open_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	int mode = va->a_mode;
 
 	PFS_TRACE(("%s (mode 0x%x)", pn->pn_name, mode));
-
-	/*
-	 * check if the file is visible to the caller
-	 *
-	 * XXX Not sure if this is necessary, as the VFS system calls
-	 * XXX pfs_lookup() and pfs_access() first, and pfs_lookup()
-	 * XXX calls pfs_visible().  There's a race condition here, but
-	 * XXX calling pfs_visible() from here doesn't really close it,
-	 * XXX and the only consequence of that race is an EIO further
-	 * XXX down the line.
-	 */
-	if (!pfs_visible(va->a_td, pn, pvd->pvd_pid, NULL))
-		PFS_RETURN (ENOENT);
+	pfs_assert_not_owned(pn);
 
 	/* check if the requested mode is permitted */
 	if (((mode & FREAD) && !(mode & PFS_RD)) ||
@@ -486,7 +470,7 @@ static int
 pfs_read(struct vop_read_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct uio *uio = va->a_uio;
 	struct proc *proc;
@@ -494,7 +478,8 @@ pfs_read(struct vop_read_args *va)
 	int error;
 	unsigned int buflen, offset, resid;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	if (vn->v_type != VREG)
 		PFS_RETURN (EINVAL);
@@ -502,7 +487,7 @@ pfs_read(struct vop_read_args *va)
 	if (!(pn->pn_flags & PFS_RD))
 		PFS_RETURN (EBADF);
 
-	if (pn->pn_func == NULL)
+	if (pn->pn_fill == NULL)
 		PFS_RETURN (EIO);
 
 	/*
@@ -511,20 +496,21 @@ pfs_read(struct vop_read_args *va)
 	 */
 	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
-
 	if (proc != NULL) {
 		_PHOLD(proc);
 		PROC_UNLOCK(proc);
 	}
 
 	if (pn->pn_flags & PFS_RAWRD) {
-		error = (pn->pn_func)(curthread, proc, pn, NULL, uio);
+		PFS_TRACE(("%lu resid", (unsigned long)uio->uio_resid));
+		error = pn_fill(curthread, proc, pn, NULL, uio);
+		PFS_TRACE(("%lu resid", (unsigned long)uio->uio_resid));
 		if (proc != NULL)
 			PRELE(proc);
 		PFS_RETURN (error);
 	}
 
-	/* Beaucoup sanity checks so we don't ask for bogus allocation. */
+	/* beaucoup sanity checks so we don't ask for bogus allocation */
 	if (uio->uio_offset < 0 || uio->uio_resid < 0 ||
 	    (offset = uio->uio_offset) != uio->uio_offset ||
 	    (resid = uio->uio_resid) != uio->uio_resid ||
@@ -538,6 +524,7 @@ pfs_read(struct vop_read_args *va)
 			PRELE(proc);
 		PFS_RETURN (EIO);
 	}
+
 	sb = sbuf_new(sb, NULL, buflen, 0);
 	if (sb == NULL) {
 		if (proc != NULL)
@@ -545,7 +532,7 @@ pfs_read(struct vop_read_args *va)
 		PFS_RETURN (EIO);
 	}
 
-	error = (pn->pn_func)(curthread, proc, pn, sb, uio);
+	error = pn_fill(curthread, proc, pn, sb, uio);
 
 	if (proc != NULL)
 		PRELE(proc);
@@ -565,10 +552,13 @@ pfs_read(struct vop_read_args *va)
  * Iterate through directory entries
  */
 static int
-pfs_iterate(struct thread *td, pid_t pid, struct pfs_node *pd,
+pfs_iterate(struct thread *td, struct proc *proc, struct pfs_node *pd,
 	    struct pfs_node **pn, struct proc **p)
 {
+	int visible;
+
 	sx_assert(&allproc_lock, SX_SLOCKED);
+	pfs_assert_owned(pd);
  again:
 	if (*pn == NULL) {
 		/* first node */
@@ -586,12 +576,22 @@ pfs_iterate(struct thread *td, pid_t pid, struct pfs_node *pd,
 		/* out of processes: next node */
 		if (*p == NULL)
 			*pn = (*pn)->pn_next;
+		else
+			PROC_LOCK(*p);
 	}
 
 	if ((*pn) == NULL)
 		return (-1);
 
-	if (!pfs_visible(td, *pn, *p ? (*p)->p_pid : pid, NULL))
+	if (*p != NULL) {
+		visible = pfs_visible_proc(td, *pn, *p);
+		PROC_UNLOCK(*p);
+	} else if (proc != NULL) {
+		visible = pfs_visible_proc(td, *pn, proc);
+	} else {
+		visible = 1;
+	}
+	if (!visible)
 		goto again;
 
 	return (0);
@@ -604,29 +604,26 @@ static int
 pfs_readdir(struct vop_readdir_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pd = pvd->pvd_pn;
 	pid_t pid = pvd->pvd_pid;
+	struct proc *p, *proc;
 	struct pfs_node *pn;
 	struct dirent *entry;
 	struct uio *uio;
-	struct proc *p;
 	off_t offset;
 	int error, i, resid;
 	char *buf, *ent;
 
 	KASSERT(pd->pn_info == vn->v_mount->mnt_data,
-	    ("directory's pn_info does not match mountpoint's mnt_data"));
-	PFS_TRACE((pd->pn_name));
+	    ("%s(): pn_info does not match mountpoint", __func__));
+	PFS_TRACE(("%s pid %lu", pd->pn_name, (unsigned long)pid));
+	pfs_assert_not_owned(pd);
 
 	if (vn->v_type != VDIR)
 		PFS_RETURN (ENOTDIR);
 	uio = va->a_uio;
 
-	/* check if the directory is visible to the caller */
-	if (!pfs_visible(curthread, pd, pid, NULL))
-		PFS_RETURN (ENOENT);
-
 	/* only allow reading entire entries */
 	offset = uio->uio_offset;
 	resid = uio->uio_resid;
@@ -636,24 +633,41 @@ pfs_readdir(struct vop_readdir_args *va)
 	if (resid == 0)
 		PFS_RETURN (0);
 
-	/* skip unwanted entries */
+	/* can't do this while holding the proc lock... */
+	buf = malloc(resid, M_IOV, M_WAITOK | M_ZERO);
 	sx_slock(&allproc_lock);
-	for (pn = NULL, p = NULL; offset > 0; offset -= PFS_DELEN)
-		if (pfs_iterate(curthread, pid, pd, &pn, &p) == -1) {
+	pfs_lock(pd);
+
+        /* check if the directory is visible to the caller */
+        if (!pfs_visible(curthread, pd, pid, &proc)) {
+		sx_sunlock(&allproc_lock);
+		pfs_unlock(pd);
+		free(buf, M_IOV);
+                PFS_RETURN (ENOENT);
+	}
+	KASSERT(pid == NO_PID || proc != NULL,
+	    ("%s(): no process for pid %lu", __func__, (unsigned long)pid));
+
+	/* skip unwanted entries */
+	for (pn = NULL, p = NULL; offset > 0; offset -= PFS_DELEN) {
+		if (pfs_iterate(curthread, proc, pd, &pn, &p) == -1) {
 			/* nothing left... */
+			if (proc != NULL)
+				PROC_UNLOCK(proc);
+			pfs_unlock(pd);
 			sx_sunlock(&allproc_lock);
+			free(buf, M_IOV);
 			PFS_RETURN (0);
 		}
+	}
 
 	/* fill in entries */
-	ent = buf = malloc(resid, M_IOV, M_WAITOK | M_ZERO);
-	while (pfs_iterate(curthread, pid, pd, &pn, &p) != -1 &&
+	ent = buf;
+	while (pfs_iterate(curthread, proc, pd, &pn, &p) != -1 &&
 	    resid >= PFS_DELEN) {
 		entry = (struct dirent *)ent;
 		entry->d_reclen = PFS_DELEN;
-		if (!pn->pn_parent)
-			pn->pn_parent = pd;
-		entry->d_fileno = pfs_fileno(pn, pid);
+		entry->d_fileno = pn_fileno(pn, pid);
 		/* PFS_DELEN was picked to fit PFS_NAMLEN */
 		for (i = 0; i < PFS_NAMELEN - 1 && pn->pn_name[i] != '\0'; ++i)
 			entry->d_name[i] = pn->pn_name[i];
@@ -681,12 +695,16 @@ pfs_readdir(struct vop_readdir_args *va)
 		default:
 			panic("%s has unexpected node type: %d", pn->pn_name, pn->pn_type);
 		}
-		PFS_TRACE((entry->d_name));
+		PFS_TRACE(("%s", entry->d_name));
 		offset += PFS_DELEN;
 		resid -= PFS_DELEN;
 		ent += PFS_DELEN;
 	}
+	if (proc != NULL)
+		PROC_UNLOCK(proc);
+	pfs_unlock(pd);
 	sx_sunlock(&allproc_lock);
+	PFS_TRACE(("%zd bytes", ent - buf));
 	error = uiomove(buf, ent - buf, uio);
 	free(buf, M_IOV);
 	PFS_RETURN (error);
@@ -699,20 +717,21 @@ static int
 pfs_readlink(struct vop_readlink_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct uio *uio = va->a_uio;
 	struct proc *proc = NULL;
-	char buf[MAXPATHLEN];
+	char buf[PATH_MAX];
 	struct sbuf sb;
 	int error;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	if (vn->v_type != VLNK)
 		PFS_RETURN (EINVAL);
 
-	if (pn->pn_func == NULL)
+	if (pn->pn_fill == NULL)
 		PFS_RETURN (EIO);
 
 	if (pvd->pvd_pid != NO_PID) {
@@ -729,7 +748,7 @@ pfs_readlink(struct vop_readlink_args *va)
 	/* sbuf_new() can't fail with a static buffer */
 	sbuf_new(&sb, buf, sizeof buf, 0);
 
-	error = (pn->pn_func)(curthread, proc, pn, &sb, NULL);
+	error = pn_fill(curthread, proc, pn, &sb, NULL);
 
 	if (proc != NULL)
 		PRELE(proc);
@@ -751,7 +770,12 @@ pfs_readlink(struct vop_readlink_args *va)
 static int
 pfs_reclaim(struct vop_reclaim_args *va)
 {
-	PFS_TRACE((((struct pfs_vdata *)va->a_vp->v_data)->pvd_pn->pn_name));
+	struct vnode *vn = va->a_vp;
+	struct pfs_vdata *pvd = vn->v_data;
+	struct pfs_node *pn = pvd->pvd_pn;
+
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	return (pfs_vncache_free(va->a_vp));
 }
@@ -762,7 +786,12 @@ pfs_reclaim(struct vop_reclaim_args *va)
 static int
 pfs_setattr(struct vop_setattr_args *va)
 {
-	PFS_TRACE((((struct pfs_vdata *)va->a_vp->v_data)->pvd_pn->pn_name));
+	struct vnode *vn = va->a_vp;
+	struct pfs_vdata *pvd = vn->v_data;
+	struct pfs_node *pn = pvd->pvd_pn;
+
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	PFS_RETURN (EOPNOTSUPP);
 }
@@ -774,22 +803,25 @@ static int
 pfs_write(struct vop_write_args *va)
 {
 	struct vnode *vn = va->a_vp;
-	struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
+	struct pfs_vdata *pvd = vn->v_data;
 	struct pfs_node *pn = pvd->pvd_pn;
 	struct uio *uio = va->a_uio;
 	struct proc *proc;
 	struct sbuf sb;
 	int error;
 
-	PFS_TRACE((pn->pn_name));
+	PFS_TRACE(("%s", pn->pn_name));
+	pfs_assert_not_owned(pn);
 
 	if (vn->v_type != VREG)
 		PFS_RETURN (EINVAL);
+	KASSERT(pn->pn_type != pfstype_file,
+	    ("%s(): VREG vnode refers to non-file pfs_node", __func__));
 
 	if (!(pn->pn_flags & PFS_WR))
 		PFS_RETURN (EBADF);
 
-	if (pn->pn_func == NULL)
+	if (pn->pn_fill == NULL)
 		PFS_RETURN (EIO);
 
 	/*
@@ -798,29 +830,32 @@ pfs_write(struct vop_write_args *va)
 	 */
 	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
-
 	if (proc != NULL) {
 		_PHOLD(proc);
 		PROC_UNLOCK(proc);
 	}
 
 	if (pn->pn_flags & PFS_RAWWR) {
-		error = (pn->pn_func)(curthread, proc, pn, NULL, uio);
+		pfs_lock(pn);
+		error = pn_fill(curthread, proc, pn, NULL, uio);
+		pfs_unlock(pn);
 		if (proc != NULL)
 			PRELE(proc);
 		PFS_RETURN (error);
 	}
 
 	sbuf_uionew(&sb, uio, &error);
-	if (error)
+	if (error) {
+		if (proc != NULL)
+			PRELE(proc);
 		PFS_RETURN (error);
+	}
 
-	error = (pn->pn_func)(curthread, proc, pn, &sb, uio);
+	error = pn_fill(curthread, proc, pn, &sb, uio);
 
+	sbuf_delete(&sb);
 	if (proc != NULL)
 		PRELE(proc);
-
-	sbuf_delete(&sb);
 	PFS_RETURN (error);
 }