Module Name: src
Committed By: thorpej
Date: Wed Apr 29 01:52:26 UTC 2020
Modified Files:
src/sys/kern: kern_lwp.c kern_proc.c
src/sys/sys: lwp.h proc.h
Log Message:
- proc_find() retains traditional semantics of requiring the canonical
PID to look up a proc. Add a separate proc_find_lwpid() to look up a
proc by the ID of any of its LWPs.
- Add proc_find_lwp_acquire_proc(), which enables looking up the LWP
*and* a proc given the ID of any LWP. Returns with the proc::p_lock
held.
- Rewrite lwp_find2() in terms of proc_find_lwp_acquire_proc(), and add
allow the proc to be wildcarded, rather than just curproc or specific
proc.
- lwp_find2() now subsumes the original intent of lwp_getref_lwpid(), but
in a much nicer way, so garbage-collect the remnants of that recently
added mechanism.
To generate a diff of this commit:
cvs rdiff -u -r1.236 -r1.237 src/sys/kern/kern_lwp.c
cvs rdiff -u -r1.250 -r1.251 src/sys/kern/kern_proc.c
cvs rdiff -u -r1.208 -r1.209 src/sys/sys/lwp.h
cvs rdiff -u -r1.363 -r1.364 src/sys/sys/proc.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_lwp.c
diff -u src/sys/kern/kern_lwp.c:1.236 src/sys/kern/kern_lwp.c:1.237
--- src/sys/kern/kern_lwp.c:1.236 Sun Apr 26 18:53:33 2020
+++ src/sys/kern/kern_lwp.c Wed Apr 29 01:52:26 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_lwp.c,v 1.236 2020/04/26 18:53:33 thorpej Exp $ */
+/* $NetBSD: kern_lwp.c,v 1.237 2020/04/29 01:52:26 thorpej Exp $ */
/*-
* Copyright (c) 2001, 2006, 2007, 2008, 2009, 2019, 2020
@@ -223,7 +223,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v 1.236 2020/04/26 18:53:33 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v 1.237 2020/04/29 01:52:26 thorpej Exp $");
#include "opt_ddb.h"
#include "opt_lockdebug.h"
@@ -266,32 +266,6 @@ __KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v
static pool_cache_t lwp_cache __read_mostly;
struct lwplist alllwp __cacheline_aligned;
-/*
- * Lookups by global thread ID operate outside of the normal LWP
- * locking protocol.
- *
- * => When we look up an LWP in the table, we take lwp_threadid_lock as
- * a READER. While still holding the lock, we add a reference to
- * the LWP (using atomics). After adding the reference, we drop the
- * lwp_threadid_lock. We now take p_lock and check the state of the
- * LWP. If the LWP is draining its references, we drop the reference
- * we took and return NULL. Otherwise, the lookup has succeeded and
- * the LWP is returned with a reference count that the caller is
- * responsible for dropping.
- *
- * => When a LWP is exiting, it also drains off any references being
- * held by others. However, the reference in the lookup path is taken
- * outside the normal locking protocol. There needs to be additional
- * serialization so that EITHER lwp_drainrefs() sees the incremented
- * reference count so that it knows to wait, OR lwp_getref_lwpid() sees
- * that the LWP is waiting to drain and thus drops the reference
- * immediately. This is achieved by taking lwp_threadid_lock as a
- * WRITER when setting LPR_DRAINING. Note the locking order:
- *
- * p_lock -> lwp_threadid_lock
- */
-static krwlock_t lwp_threadid_lock __cacheline_aligned;
-
static void lwp_dtor(void *, void *);
/* DTrace proc provider probes */
@@ -325,7 +299,6 @@ struct lwp lwp0 __aligned(MIN_LWP_ALIGNM
.l_fd = &filedesc0,
};
-static void lwp_threadid_init(void);
static int sysctl_kern_maxlwp(SYSCTLFN_PROTO);
/*
@@ -378,7 +351,6 @@ lwpinit(void)
maxlwp = cpu_maxlwp();
sysctl_kern_lwp_setup();
- lwp_threadid_init();
}
void
@@ -1405,10 +1377,20 @@ lwp_migrate(lwp_t *l, struct cpu_info *t
lwp_unlock(l);
}
+#define lwp_find_exclude(l) \
+ ((l)->l_stat == LSIDL || (l)->l_stat == LSZOMB)
+
/*
* Find the LWP in the process. Arguments may be zero, in such case,
* the calling process and first LWP in the list will be used.
* On success - returns proc locked.
+ *
+ * => pid == 0 -> look in curproc.
+ * => pid == -1 -> match any proc.
+ * => otherwise look up the proc.
+ *
+ * => lid == 0 -> first LWP in the proc
+ * => otherwise specific LWP
*/
struct lwp *
lwp_find2(pid_t pid, lwpid_t lid)
@@ -1416,27 +1398,62 @@ lwp_find2(pid_t pid, lwpid_t lid)
proc_t *p;
lwp_t *l;
- /* Find the process. */
- if (pid != 0) {
- mutex_enter(proc_lock);
- p = proc_find(pid);
- if (p == NULL) {
- mutex_exit(proc_lock);
+ /* First LWP of specified proc. */
+ if (lid == 0) {
+ switch (pid) {
+ case -1:
+ /* No lookup keys. */
return NULL;
+ case 0:
+ p = curproc;
+ mutex_enter(p->p_lock);
+ break;
+ default:
+ mutex_enter(proc_lock);
+ p = proc_find(pid);
+ if (__predict_false(p == NULL)) {
+ mutex_exit(proc_lock);
+ return NULL;
+ }
+ mutex_enter(p->p_lock);
+ mutex_exit(proc_lock);
+ break;
}
- mutex_enter(p->p_lock);
- mutex_exit(proc_lock);
- } else {
- p = curlwp->l_proc;
- mutex_enter(p->p_lock);
+ LIST_FOREACH(l, &p->p_lwps, l_sibling) {
+ if (__predict_true(!lwp_find_exclude(l)))
+ break;
+ }
+ goto out;
}
- /* Find the thread. */
- if (lid != 0) {
- l = lwp_find(p, lid);
- } else {
- l = LIST_FIRST(&p->p_lwps);
+
+ l = proc_find_lwp_acquire_proc(lid, &p);
+ if (l == NULL)
+ return NULL;
+ KASSERT(p != NULL);
+ KASSERT(mutex_owned(p->p_lock));
+
+ if (__predict_false(lwp_find_exclude(l))) {
+ l = NULL;
+ goto out;
+ }
+
+ /* Apply proc filter, if applicable. */
+ switch (pid) {
+ case -1:
+ /* Match anything. */
+ break;
+ case 0:
+ if (p != curproc)
+ l = NULL;
+ break;
+ default:
+ if (p->p_pid != pid)
+ l = NULL;
+ break;
}
- if (l == NULL) {
+
+ out:
+ if (__predict_false(l == NULL)) {
mutex_exit(p->p_lock);
}
return l;
@@ -1462,7 +1479,7 @@ lwp_find(struct proc *p, lwpid_t id)
* No need to lock - all of these conditions will
* be visible with the process level mutex held.
*/
- if (l != NULL && (l->l_stat == LSIDL || l->l_stat == LSZOMB))
+ if (__predict_false(l != NULL && lwp_find_exclude(l)))
l = NULL;
return l;
@@ -1664,21 +1681,6 @@ lwp_need_userret(struct lwp *l)
}
/*
- * Add one reference to an LWP. Interlocked against lwp_drainrefs()
- * either by holding the proc's lock or by holding lwp_threadid_lock.
- * If callers don't hold the proc's lock, then they must check for a
- * larva after acquiring the reference. References can't be added to
- * zombies because references have already been drained off before the
- * state changes to LSZOMB.
- */
-static void
-lwp_addref2(struct lwp *l)
-{
- KASSERT(l->l_stat != LSZOMB);
- atomic_inc_uint(&l->l_refcnt);
-}
-
-/*
* Add one reference to an LWP. This will prevent the LWP from
* exiting, thus keep the lwp structure and PCB around to inspect.
*/
@@ -1686,7 +1688,8 @@ void
lwp_addref(struct lwp *l)
{
KASSERT(mutex_owned(l->l_proc->p_lock));
- lwp_addref2(l);
+ KASSERT(l->l_stat != LSZOMB);
+ l->l_refcnt++;
}
/*
@@ -1715,9 +1718,9 @@ lwp_delref2(struct lwp *l)
KASSERT(mutex_owned(p->p_lock));
KASSERT(l->l_stat != LSZOMB);
- KASSERT(atomic_load_relaxed(&l->l_refcnt) > 0);
+ KASSERT(l->l_refcnt > 0);
- if (atomic_dec_uint_nv(&l->l_refcnt) == 0)
+ if (--l->l_refcnt == 0)
cv_broadcast(&p->p_lwpcv);
}
@@ -1733,18 +1736,9 @@ lwp_drainrefs(struct lwp *l)
KASSERT(mutex_owned(p->p_lock));
- /*
- * Lookups by thread ID hold lwp_threadid_lock as a reader,
- * increase l_refcnt, release it, and then acquire p_lock to
- * check for LPR_DRAINING. By taking lwp_threadid_lock as a
- * writer here we ensure that either we see the increase in
- * l_refcnt or that they see LPR_DRAINING.
- */
- rw_enter(&lwp_threadid_lock, RW_WRITER);
l->l_prflag |= LPR_DRAINING;
- rw_exit(&lwp_threadid_lock);
- while (atomic_load_relaxed(&l->l_refcnt) > 0) {
+ while (l->l_refcnt > 0) {
rv = true;
cv_wait(&p->p_lwpcv, p->p_lock);
}
@@ -2031,58 +2025,6 @@ lwp_setprivate(struct lwp *l, void *ptr)
return error;
}
-static void
-lwp_threadid_init(void)
-{
- rw_init(&lwp_threadid_lock);
-}
-
-/*
- * Lookup an LWP by global thread ID. Care must be taken because
- * callers of this are operating outside the normal locking protocol.
- * We return the LWP with an additional reference that must be dropped
- * with lwp_delref().
- */
-struct lwp *
-lwp_getref_lwpid(lwpid_t tid)
-{
- struct lwp *l;
-
- /*
- * We rely on lwp_thread_cleanup() to hide LWP IDs from us
- * to ensure that we cannot add a reference do an exiting
- * LWP.
- */
- rw_enter(&lwp_threadid_lock, RW_READER);
- l = proc_seek_lwpid(tid);
- if (__predict_false(l == NULL)) {
- rw_exit(&lwp_threadid_lock);
- return NULL;
- }
-
- /*
- * Acquire a reference on the lwp. It is safe to do this unlocked
- * here because lwp_drainrefs() serializes with us by taking the
- * lwp_threadid_lock as a writer.
- */
- lwp_addref2(l);
- rw_exit(&lwp_threadid_lock);
-
- /*
- * Now verify that our reference is valid.
- */
- struct proc *p = l->l_proc;
- mutex_enter(p->p_lock);
- if (__predict_false(l->l_stat == LSLARVAL ||
- (l->l_prflag & LPR_DRAINING) != 0)) {
- lwp_delref2(l);
- l = NULL;
- }
- mutex_exit(p->p_lock);
-
- return l;
-}
-
/*
* Perform any thread-related cleanup on LWP exit.
* N.B. l->l_proc->p_lock must be HELD on entry but will
@@ -2097,13 +2039,6 @@ lwp_thread_cleanup(struct lwp *l)
KASSERT((tid & FUTEX_TID_MASK) == tid);
KASSERT(mutex_owned(l->l_proc->p_lock));
- /*
- * Hide this LWP from seekers (namely lwp_getref_lwpid())
- * to prevent them from attempting to acquire a reference
- * on a zombie.
- */
- proc_hide_lwpid(tid);
-
mutex_exit(l->l_proc->p_lock);
/*
Index: src/sys/kern/kern_proc.c
diff -u src/sys/kern/kern_proc.c:1.250 src/sys/kern/kern_proc.c:1.251
--- src/sys/kern/kern_proc.c:1.250 Sun Apr 26 18:53:33 2020
+++ src/sys/kern/kern_proc.c Wed Apr 29 01:52:26 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_proc.c,v 1.250 2020/04/26 18:53:33 thorpej Exp $ */
+/* $NetBSD: kern_proc.c,v 1.251 2020/04/29 01:52:26 thorpej Exp $ */
/*-
* Copyright (c) 1999, 2006, 2007, 2008, 2020 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_proc.c,v 1.250 2020/04/26 18:53:33 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_proc.c,v 1.251 2020/04/29 01:52:26 thorpej Exp $");
#ifdef _KERNEL_OPT
#include "opt_kstack.h"
@@ -147,17 +147,14 @@ struct pid_table {
#define PT_F_FREE ((uintptr_t)__BIT(0))
#define PT_F_LWP 0 /* pseudo-flag */
#define PT_F_PROC ((uintptr_t)__BIT(1))
-#define PT_F_HIDDEN ((uintptr_t)__BIT(2))
#define PT_F_TYPEBITS (PT_F_FREE|PT_F_PROC)
-#define PT_F_ALLBITS (PT_F_FREE|PT_F_PROC|PT_F_HIDDEN)
+#define PT_F_ALLBITS (PT_F_FREE|PT_F_PROC)
#define PT_VALID(s) (((s) & PT_F_FREE) == 0)
#define PT_RESERVED(s) ((s) == 0)
-#define PT_HIDDEN(s) ((s) & PT_F_HIDDEN)
#define PT_NEXT(s) ((u_int)(s) >> 1)
#define PT_SET_FREE(pid) (((pid) << 1) | PT_F_FREE)
-#define PT_SET_HIDDEN(s) ((s) | PT_F_HIDDEN)
#define PT_SET_LWP(l) ((uintptr_t)(l))
#define PT_SET_PROC(p) (((uintptr_t)(p)) | PT_F_PROC)
#define PT_SET_RESERVED 0
@@ -708,63 +705,63 @@ proc_find_lwp(proc_t *p, pid_t pid)
}
/*
- * proc_seek_lwpid: locate an lwp by only the ID.
+ * proc_find_lwp_acquire_proc: locate an lwp and acquire a lock
+ * on its containing proc.
*
- * => This is a specialized interface used for looking up an LWP
- * without holding a lock on its owner process.
- * => Callers of this interface MUST provide a separate synchronization
- * mechanism to ensure the validity of the returned LWP. LARVAL LWPs
- * are found there, so callers must check for them!
- * => Only returns LWPs whose ID has not been hidden from us.
+ * => Similar to proc_find_lwp(), but does not require you to have
+ * the proc a priori.
+ * => Also returns proc * to caller, with p::p_lock held.
+ * => Same caveats apply.
*/
struct lwp *
-proc_seek_lwpid(pid_t pid)
+proc_find_lwp_acquire_proc(pid_t pid, struct proc **pp)
{
struct pid_table *pt;
+ struct proc *p = NULL;
struct lwp *l = NULL;
uintptr_t slot;
+ KASSERT(pp != NULL);
+ mutex_enter(proc_lock);
rw_enter(&pid_table_lock, RW_READER);
pt = &pid_table[pid & pid_tbl_mask];
slot = pt->pt_slot;
- if (__predict_true(PT_IS_LWP(slot) && pt->pt_pid == pid &&
- !PT_HIDDEN(slot))) {
+ if (__predict_true(PT_IS_LWP(slot) && pt->pt_pid == pid)) {
+ /*
+ * Locking order is p::p_lock -> pid_table_lock, but
+ * we're already holding pid_table_lock; we need to
+ * release it before acquiring p::p_lock. This is
+ * safe because p will be stable by virtue of holding
+ * proc_lock.
+ */
l = PT_GET_LWP(slot);
+ p = l->l_proc;
+ rw_exit(&pid_table_lock);
+ mutex_enter(p->p_lock);
+ if (__predict_false(l->l_stat == LSLARVAL)) {
+ mutex_exit(p->p_lock);
+ l = NULL;
+ p = NULL;
+ }
+ } else {
+ rw_exit(&pid_table_lock);
}
- rw_exit(&pid_table_lock);
+ mutex_exit(proc_lock);
+ KASSERT(p == NULL || mutex_owned(p->p_lock));
+ *pp = p;
return l;
}
/*
- * proc_hide_lwpid: hide an lwp ID from seekers.
- */
-void
-proc_hide_lwpid(pid_t pid)
-{
- struct pid_table *pt;
- uintptr_t slot;
-
- rw_enter(&pid_table_lock, RW_WRITER);
- pt = &pid_table[pid & pid_tbl_mask];
-
- slot = pt->pt_slot;
- KASSERT(PT_IS_LWP(slot));
- KASSERT(pt->pt_pid == pid);
- pt->pt_slot = PT_SET_HIDDEN(slot);
-
- rw_exit(&pid_table_lock);
-}
-
-/*
* proc_find_raw_pid_table_locked: locate a process by the ID.
*
* => Must be called with proc_lock held and the pid_table_lock
* at least held for reading.
*/
static proc_t *
-proc_find_raw_pid_table_locked(pid_t pid)
+proc_find_raw_pid_table_locked(pid_t pid, bool any_lwpid)
{
struct pid_table *pt;
proc_t *p = NULL;
@@ -784,7 +781,7 @@ proc_find_raw_pid_table_locked(pid_t pid
* valid here.
*/
p = PT_GET_LWP(slot)->l_proc;
- if (__predict_false(p->p_pid != pid))
+ if (__predict_false(p->p_pid != pid && !any_lwpid))
p = NULL;
} else if (PT_IS_PROC(slot) && pt->pt_pid == pid) {
p = PT_GET_PROC(slot);
@@ -797,19 +794,19 @@ proc_find_raw(pid_t pid)
{
KASSERT(mutex_owned(proc_lock));
rw_enter(&pid_table_lock, RW_READER);
- proc_t *p = proc_find_raw_pid_table_locked(pid);
+ proc_t *p = proc_find_raw_pid_table_locked(pid, false);
rw_exit(&pid_table_lock);
return p;
}
static proc_t *
-proc_find_pid_table_locked(pid_t pid)
+proc_find_pid_table_locked(pid_t pid, bool any_lwpid)
{
proc_t *p;
KASSERT(mutex_owned(proc_lock));
- p = proc_find_raw_pid_table_locked(pid);
+ p = proc_find_raw_pid_table_locked(pid, any_lwpid);
if (__predict_false(p == NULL)) {
return NULL;
}
@@ -824,16 +821,28 @@ proc_find_pid_table_locked(pid_t pid)
return NULL;
}
-proc_t *
-proc_find(pid_t pid)
+static proc_t *
+proc_find_internal(pid_t pid, bool any_lwpid)
{
KASSERT(mutex_owned(proc_lock));
rw_enter(&pid_table_lock, RW_READER);
- proc_t *p = proc_find_pid_table_locked(pid);
+ proc_t *p = proc_find_pid_table_locked(pid, any_lwpid);
rw_exit(&pid_table_lock);
return p;
}
+proc_t *
+proc_find(pid_t pid)
+{
+ return proc_find_internal(pid, false);
+}
+
+proc_t *
+proc_find_lwpid(pid_t pid)
+{
+ return proc_find_internal(pid, true);
+}
+
/*
* pgrp_find_pid_table_locked: locate a process group by the ID.
*
@@ -1227,7 +1236,7 @@ proc_enterpgrp(struct proc *curp, pid_t
/* Can only set another process under restricted circumstances. */
if (pid != curp->p_pid) {
/* Must exist and be one of our children... */
- p = proc_find_pid_table_locked(pid);
+ p = proc_find_pid_table_locked(pid, false);
if (p == NULL || !p_inferior(p, curp)) {
rval = ESRCH;
goto done;
Index: src/sys/sys/lwp.h
diff -u src/sys/sys/lwp.h:1.208 src/sys/sys/lwp.h:1.209
--- src/sys/sys/lwp.h:1.208 Sun Apr 26 18:53:33 2020
+++ src/sys/sys/lwp.h Wed Apr 29 01:52:26 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: lwp.h,v 1.208 2020/04/26 18:53:33 thorpej Exp $ */
+/* $NetBSD: lwp.h,v 1.209 2020/04/29 01:52:26 thorpej Exp $ */
/*
* Copyright (c) 2001, 2006, 2007, 2008, 2009, 2010, 2019, 2020
@@ -349,7 +349,6 @@ kmutex_t *lwp_setlock(lwp_t *, kmutex_t
void lwp_unlock_to(lwp_t *, kmutex_t *);
int lwp_trylock(lwp_t *);
void lwp_addref(lwp_t *);
-lwp_t * lwp_getref_lwpid(lwpid_t);
void lwp_delref(lwp_t *);
void lwp_delref2(lwp_t *);
bool lwp_drainrefs(lwp_t *);
Index: src/sys/sys/proc.h
diff -u src/sys/sys/proc.h:1.363 src/sys/sys/proc.h:1.364
--- src/sys/sys/proc.h:1.363 Fri Apr 24 03:22:06 2020
+++ src/sys/sys/proc.h Wed Apr 29 01:52:26 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: proc.h,v 1.363 2020/04/24 03:22:06 thorpej Exp $ */
+/* $NetBSD: proc.h,v 1.364 2020/04/29 01:52:26 thorpej Exp $ */
/*-
* Copyright (c) 2006, 2007, 2008, 2020 The NetBSD Foundation, Inc.
@@ -497,12 +497,12 @@ extern struct pool ptimer_pool; /* Memor
int proc_find_locked(struct lwp *, struct proc **, pid_t);
proc_t * proc_find_raw(pid_t);
proc_t * proc_find(pid_t); /* Find process by ID */
+proc_t * proc_find_lwpid(pid_t); /* Find process by LWP ID */
struct lwp * proc_find_lwp(proc_t *, pid_t); /* Find LWP in proc by ID */
+ /* Find LWP, acquire proc */
+struct lwp * proc_find_lwp_acquire_proc(pid_t, proc_t **);
struct pgrp * pgrp_find(pid_t); /* Find process group by ID */
-struct lwp * proc_seek_lwpid(pid_t); /* Find LWP by ID only */
-void proc_hide_lwpid(pid_t); /* Hide LWP ID from seekers */
-
void procinit(void);
void procinit_sysctl(void);
int proc_enterpgrp(struct proc *, pid_t, pid_t, bool);
