proc: convert pfind & friends to use pidhash locks and other cleanup

pfind_locked is retired as it relied on allproc which unnecessarily
restricts locking of the hash.

Sponsored by:	The FreeBSD Foundation

sys/fs/nfsclient/nfs_clport.c

@@ -1157,7 +1157,7 @@ nfscl_procdoesntexist(u_int8_t *own)
 	tl.cval[2] = *own++;
 	tl.cval[3] = *own++;
 	pid = tl.lval;
-	p = pfind_locked(pid);
+	p = pfind(pid);
 	if (p == NULL)
 		return (1);
 	if (p->p_stats == NULL) {

sys/fs/pseudofs/pseudofs_vnops.c

@@ -107,7 +107,7 @@ pfs_visible_proc(struct thread *td, struct pfs_node *pn, struct proc *proc)
 
 static int
 pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid,
-    bool allproc_locked, struct proc **p)
+    struct proc **p)
 {
 	struct proc *proc;
 
@@ -118,7 +118,7 @@ pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid,
 		*p = NULL;
 	if (pid == NO_PID)
 		PFS_RETURN (1);
-	proc = allproc_locked ? pfind_locked(pid) : pfind(pid);
+	proc = pfind(pid);
 	if (proc == NULL)
 		PFS_RETURN (0);
 	if (pfs_visible_proc(td, pn, proc)) {
@@ -206,7 +206,7 @@ pfs_getattr(struct vop_getattr_args *va)
 	PFS_TRACE(("%s", pn->pn_name));
 	pfs_assert_not_owned(pn);
 
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, false, &proc))
+	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (ENOENT);
 
 	vap->va_type = vn->v_type;
@@ -297,7 +297,7 @@ pfs_ioctl(struct vop_ioctl_args *va)
 	 * This is necessary because process' privileges may
 	 * have changed since the open() call.
 	 */
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, false, &proc)) {
+	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc)) {
 		VOP_UNLOCK(vn, 0);
 		PFS_RETURN (EIO);
 	}
@@ -330,7 +330,7 @@ pfs_getextattr(struct vop_getextattr_args *va)
 	 * This is necessary because either process' privileges may
 	 * have changed since the open() call.
 	 */
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, false, &proc))
+	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
 
 	if (pn->pn_getextattr == NULL)
@@ -466,7 +466,7 @@ pfs_lookup(struct vop_cachedlookup_args *va)
 		PFS_RETURN (ENOENT);
 
 	/* check that parent directory is visible... */
-	if (!pfs_visible(curthread, pd, pvd->pvd_pid, false, NULL))
+	if (!pfs_visible(curthread, pd, pvd->pvd_pid, NULL))
 		PFS_RETURN (ENOENT);
 
 	/* self */
@@ -550,7 +550,7 @@ pfs_lookup(struct vop_cachedlookup_args *va)
  got_pnode:
 	pfs_assert_not_owned(pd);
 	pfs_assert_not_owned(pn);
-	visible = pfs_visible(curthread, pn, pid, false, NULL);
+	visible = pfs_visible(curthread, pn, pid, NULL);
 	if (!visible) {
 		error = ENOENT;
 		goto failed;
@@ -639,7 +639,7 @@ pfs_read(struct vop_read_args *va)
 	 * This is necessary because either process' privileges may
 	 * have changed since the open() call.
 	 */
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, false, &proc))
+	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
 	if (proc != NULL) {
 		_PHOLD(proc);
@@ -795,7 +795,7 @@ pfs_readdir(struct vop_readdir_args *va)
 	pfs_lock(pd);
 
         /* check if the directory is visible to the caller */
-        if (!pfs_visible(curthread, pd, pid, true, &proc)) {
+        if (!pfs_visible(curthread, pd, pid, &proc)) {
 		sx_sunlock(&allproc_lock);
 		pfs_unlock(pd);
                 PFS_RETURN (ENOENT);
@@ -1001,7 +1001,7 @@ pfs_write(struct vop_write_args *va)
 	 * This is necessary because either process' privileges may
 	 * have changed since the open() call.
 	 */
-	if (!pfs_visible(curthread, pn, pvd->pvd_pid, false, &proc))
+	if (!pfs_visible(curthread, pn, pvd->pvd_pid, &proc))
 		PFS_RETURN (EIO);
 	if (proc != NULL) {
 		_PHOLD(proc);

sys/kern/kern_proc.c

@@ -112,7 +112,6 @@ static void proc_dtor(void *mem, int size, void *arg);
 static int proc_init(void *mem, int size, int flags);
 static void proc_fini(void *mem, int size);
 static void pargs_free(struct pargs *pa);
-static struct proc *zpfind_locked(pid_t pid);
 
 /*
  * Other process lists
@@ -307,33 +306,14 @@ inferior(struct proc *p)
 	return (1);
 }
 
-struct proc *
-pfind_locked(pid_t pid)
-{
-	struct proc *p;
-
-	sx_assert(&allproc_lock, SX_LOCKED);
-	LIST_FOREACH(p, PIDHASH(pid), p_hash) {
-		if (p->p_pid == pid) {
-			PROC_LOCK(p);
-			if (p->p_state == PRS_NEW || p->p_state == PRS_ZOMBIE) {
-				PROC_UNLOCK(p);
-				p = NULL;
-			}
-			break;
-		}
-	}
-	return (p);
-}
-
 /*
- * Locate a process by number; return only "live" processes -- i.e., neither
- * zombies nor newly born but incompletely initialized processes.  By not
- * returning processes in the PRS_NEW state, we allow callers to avoid
+ * Locate a process by number.
+ *
+ * By not returning processes in the PRS_NEW state, we allow callers to avoid
  * testing for that condition to avoid dereferencing p_ucred, et al.
  */
-struct proc *
-pfind(pid_t pid)
+static __always_inline struct proc *
+_pfind(pid_t pid, bool zombie)
 {
 	struct proc *p;
 
@@ -342,36 +322,46 @@ pfind(pid_t pid)
 		PROC_LOCK(p);
 		return (p);
 	}
-	sx_slock(&allproc_lock);
-	p = pfind_locked(pid);
-	sx_sunlock(&allproc_lock);
+	sx_slock(PIDHASHLOCK(pid));
+	LIST_FOREACH(p, PIDHASH(pid), p_hash) {
+		if (p->p_pid == pid) {
+			PROC_LOCK(p);
+			if (p->p_state == PRS_NEW ||
+			    (zombie && p->p_state == PRS_ZOMBIE)) {
+				PROC_UNLOCK(p);
+				p = NULL;
+			}
+			break;
+		}
+	}
+	sx_sunlock(PIDHASHLOCK(pid));
 	return (p);
 }
 
+struct proc *
+pfind(pid_t pid)
+{
+
+	return (_pfind(pid, false));
+}
+
 /*
  * Same as pfind but allow zombies.
  */
 struct proc *
 pfind_any(pid_t pid)
 {
-	struct proc *p;
 
-	sx_slock(&allproc_lock);
-	p = pfind_locked(pid);
-	if (p == NULL)
-		p = zpfind_locked(pid);
-	sx_sunlock(&allproc_lock);
-
-	return (p);
+	return (_pfind(pid, true));
 }
 
 static struct proc *
-pfind_tid_locked(pid_t tid)
+pfind_tid(pid_t tid)
 {
 	struct proc *p;
 	struct thread *td;
 
-	sx_assert(&allproc_lock, SX_LOCKED);
+	sx_slock(&allproc_lock);
 	FOREACH_PROC_IN_SYSTEM(p) {
 		PROC_LOCK(p);
 		if (p->p_state == PRS_NEW) {
@@ -385,6 +375,7 @@ pfind_tid_locked(pid_t tid)
 		PROC_UNLOCK(p);
 	}
 found:
+	sx_sunlock(&allproc_lock);
 	return (p);
 }
 
@@ -421,17 +412,15 @@ pget(pid_t pid, int flags, struct proc **pp)
 	if (p->p_pid == pid) {
 		PROC_LOCK(p);
 	} else {
-		sx_slock(&allproc_lock);
+		p = NULL;
 		if (pid <= PID_MAX) {
-			p = pfind_locked(pid);
-			if (p == NULL && (flags & PGET_NOTWEXIT) == 0)
-				p = zpfind_locked(pid);
+			if ((flags & PGET_NOTWEXIT) == 0)
+				p = pfind_any(pid);
+			else
+				p = pfind(pid);
 		} else if ((flags & PGET_NOTID) == 0) {
-			p = pfind_tid_locked(pid);
-		} else {
-			p = NULL;
+			p = pfind_tid(pid);
 		}
-		sx_sunlock(&allproc_lock);
 		if (p == NULL)
 			return (ESRCH);
 		if ((flags & PGET_CANSEE) != 0) {
@@ -1197,21 +1186,6 @@ pstats_free(struct pstats *ps)
 	free(ps, M_SUBPROC);
 }
 
-static struct proc *
-zpfind_locked(pid_t pid)
-{
-	struct proc *p;
-
-	sx_assert(&allproc_lock, SX_LOCKED);
-	LIST_FOREACH(p, &zombproc, p_list) {
-		if (p->p_pid == pid) {
-			PROC_LOCK(p);
-			break;
-		}
-	}
-	return (p);
-}
-
 /*
  * Locate a zombie process by number
  */
@@ -1221,7 +1195,12 @@ zpfind(pid_t pid)
 	struct proc *p;
 
 	sx_slock(&allproc_lock);
-	p = zpfind_locked(pid);
+	LIST_FOREACH(p, &zombproc, p_list) {
+		if (p->p_pid == pid) {
+			PROC_LOCK(p);
+			break;
+		}
+	}
 	sx_sunlock(&allproc_lock);
 	return (p);
 }

sys/sys/proc.h

@@ -981,7 +981,6 @@ extern struct uma_zone *proc_zone;
 
 struct	proc *pfind(pid_t);		/* Find process by id. */
 struct	proc *pfind_any(pid_t);		/* Find (zombie) process by id. */
-struct	proc *pfind_locked(pid_t pid);
 struct	pgrp *pgfind(pid_t);		/* Find process group by id. */
 struct	proc *zpfind(pid_t);		/* Find zombie process by id. */