Author: mjg
Date: Wed Nov 21 20:15:56 2018
New Revision: 340744
URL: https://svnweb.freebsd.org/changeset/base/340744
Log:
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
Modified:
head/sys/fs/nfsclient/nfs_clport.c
head/sys/fs/pseudofs/pseudofs_vnops.c
head/sys/kern/kern_proc.c
head/sys/sys/proc.h
Modified: head/sys/fs/nfsclient/nfs_clport.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clport.c Wed Nov 21 19:49:21 2018
(r340743)
+++ head/sys/fs/nfsclient/nfs_clport.c Wed Nov 21 20:15:56 2018
(r340744)
@@ -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) {
Modified: head/sys/fs/pseudofs/pseudofs_vnops.c
==============================================================================
--- head/sys/fs/pseudofs/pseudofs_vnops.c Wed Nov 21 19:49:21 2018
(r340743)
+++ head/sys/fs/pseudofs/pseudofs_vnops.c Wed Nov 21 20:15:56 2018
(r340744)
@@ -107,7 +107,7 @@ pfs_visible_proc(struct thread *td, struct pfs_node *p
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, pi
*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);
Modified: head/sys/kern/kern_proc.c
==============================================================================
--- head/sys/kern/kern_proc.c Wed Nov 21 19:49:21 2018 (r340743)
+++ head/sys/kern/kern_proc.c Wed Nov 21 20:15:56 2018 (r340744)
@@ -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,45 +306,43 @@ inferior(struct proc *p)
return (1);
}
-struct proc *
-pfind_locked(pid_t pid)
+/*
+ * 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.
+ */
+static __always_inline struct proc *
+_pfind(pid_t pid, bool zombie)
{
struct proc *p;
- sx_assert(&allproc_lock, SX_LOCKED);
+ p = curproc;
+ if (p->p_pid == pid) {
+ PROC_LOCK(p);
+ return (p);
+ }
+ 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 || p->p_state == PRS_ZOMBIE) {
+ if (p->p_state == PRS_NEW ||
+ (zombie && p->p_state == PRS_ZOMBIE)) {
PROC_UNLOCK(p);
p = NULL;
}
break;
}
}
+ sx_sunlock(PIDHASHLOCK(pid));
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
- * testing for that condition to avoid dereferencing p_ucred, et al.
- */
struct proc *
pfind(pid_t pid)
{
- struct proc *p;
- p = curproc;
- if (p->p_pid == pid) {
- PROC_LOCK(p);
- return (p);
- }
- sx_slock(&allproc_lock);
- p = pfind_locked(pid);
- sx_sunlock(&allproc_lock);
- return (p);
+ return (_pfind(pid, false));
}
/*
@@ -354,24 +351,17 @@ pfind(pid_t pid)
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);
}
Modified: head/sys/sys/proc.h
==============================================================================
--- head/sys/sys/proc.h Wed Nov 21 19:49:21 2018 (r340743)
+++ head/sys/sys/proc.h Wed Nov 21 20:15:56 2018 (r340744)
@@ -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. */
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
