It's rare, but there *are* cases where a thread wants to sleep and
doesn't expect any wakeup() calls at all; e.g., nanosleep() and
sigsuspend(). In these cases, there's no point in requiring a valid
wait channel identifier or adding the thread to the sleep queue.
Diff below explicitly allows tsleep() with ident==NULL, but skips
adding the thread to a sleep queue in that case.
As an implementation detail, we currently treat "p_wchan == NULL" as
an indication that the thread has been woken up. To avoid breaking
that assumption this diff uses "p_wchan == NOSLPQUE" to identify
threads that are still sleeping, but aren't associated with a sleep
queue.
This also lets us tighten the the kernel lock KASSERT in tsleep():
rather than weakly only checking non-timed sleeps, we can precisely
check all sleeps that use a wait channel.
ok?
Index: share/man/man9/tsleep.9
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/share/man/man9/tsleep.9,v
retrieving revision 1.9
diff -u -p -r1.9 tsleep.9
--- share/man/man9/tsleep.9 22 Jan 2014 07:32:47 -0000 1.9
+++ share/man/man9/tsleep.9 8 Jul 2014 16:25:53 -0000
@@ -96,8 +96,9 @@ The same identifier must be used in a ca
.Fn wakeup
to get the process going again.
.Fa ident
-should not be
-.Dv NULL .
+may be
+.Dv NULL
+if the process is not waiting on a wait channel.
.It Fa priority
The process priority to be used when the process is awakened and put on
the queue of runnable processes.
Index: sys/kern/kern_synch.c
===================================================================
RCS file: /home/matthew/cvs-mirror/cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.115
diff -u -p -r1.115 kern_synch.c
--- sys/kern/kern_synch.c 22 Mar 2014 06:05:45 -0000 1.115
+++ sys/kern/kern_synch.c 8 Jul 2014 16:32:50 -0000
@@ -78,6 +78,12 @@ sleep_queue_init(void)
TAILQ_INIT(&slpque[i]);
}
+/*
+ * Dummy wait channel identifier for threads that don't expect a wakeup().
+ * Threads sleeping on this wait channel aren't queued on slpque.
+ */
+#define NOSLPQUE ((volatile void *)(&noslpque))
+static char noslpque;
/*
* During autoconfiguration or after a panic, a sleep will simply
@@ -109,8 +115,17 @@ tsleep(const volatile void *ident, int p
KASSERT((priority & ~(PRIMASK | PCATCH)) == 0);
+ /* Make sure caller gave us at least one reason to stop sleeping. */
+ KASSERT(ident != NULL || (priority & PCATCH) != 0 || timo != 0);
+
#ifdef MULTIPROCESSOR
- KASSERT(timo || __mp_lock_held(&kernel_lock));
+ if (ident != NULL) {
+ /*
+ * Waiting on a condition variable requires a lock, otherwise
+ * we might miss our wakeup notification.
+ */
+ KASSERT(__mp_lock_held(&kernel_lock));
+ }
#endif
if (cold || panicstr) {
@@ -191,12 +206,11 @@ sleep_setup(struct sleep_state *sls, con
{
struct proc *p = curproc;
-#ifdef DIAGNOSTIC
+ KASSERT(ident != NOSLPQUE);
+ KASSERT(p->p_stat == SONPROC);
+
if (ident == NULL)
- panic("tsleep: no ident");
- if (p->p_stat != SONPROC)
- panic("tsleep: not SONPROC");
-#endif
+ ident = NOSLPQUE;
#ifdef KTRACE
if (KTRPOINT(p, KTR_CSW))
@@ -213,7 +227,8 @@ sleep_setup(struct sleep_state *sls, con
p->p_wmesg = wmesg;
p->p_slptime = 0;
p->p_priority = prio & PRIMASK;
- TAILQ_INSERT_TAIL(&slpque[LOOKUP(ident)], p, p_runq);
+ if (ident != NOSLPQUE)
+ TAILQ_INSERT_TAIL(&slpque[LOOKUP(ident)], p, p_runq);
}
void
@@ -350,7 +365,8 @@ void
unsleep(struct proc *p)
{
if (p->p_wchan) {
- TAILQ_REMOVE(&slpque[LOOKUP(p->p_wchan)], p, p_runq);
+ if (p->p_wchan != NOSLPQUE)
+ TAILQ_REMOVE(&slpque[LOOKUP(p->p_wchan)], p, p_runq);
p->p_wchan = NULL;
}
}
@@ -365,6 +381,9 @@ wakeup_n(const volatile void *ident, int
struct proc *p;
struct proc *pnext;
int s;
+
+ KASSERT(ident != NULL);
+ KASSERT(ident != NOSLPQUE);
SCHED_LOCK(s);
qp = &slpque[LOOKUP(ident)];
