On 02/10/21(Sat) 21:09, Mark Kettenis wrote:
> > Date: Sat, 2 Oct 2021 20:35:41 +0200
> > From: Martin Pieuchot <m...@openbsd.org>
> > [...]
> > There's no sleeping point but a call to wakeup(). This wakeup() is
> > supposed to wake a btrace(8) process. But if the curproc, which just
> > added itself to the global sleep queue, ends up in the same bucket as
> > the btrace process, the KASSERT() line 565 of kern/kern_synch.c will
> > trigger:
> >
> > /*
> > * If the rwlock passed to rwsleep() is contended, the
> > * CPU will end up calling wakeup() between sleep_setup()
> > * and sleep_finish().
> > */
> > if (p == curproc) {
> > KASSERT(p->p_stat == SONPROC);
> > continue;
> > }
>
> Ah, right. But that means the comment isn't accurate. At least there
> are other cases that make us hit that codepath.
>
> How useful is that KASSERT in catching actual bugs?
I added the KASSERT() to limit the scope of the check. If the test is
true `curproc' is obviously on the CPU. Its usefulness is questionable.
So a simpler fix would be to remove the assert, diff below does that and
update the comment, ok?
Index: kern/kern_synch.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_synch.c,v
retrieving revision 1.179
diff -u -p -r1.179 kern_synch.c
--- kern/kern_synch.c 9 Sep 2021 18:41:39 -0000 1.179
+++ kern/kern_synch.c 3 Oct 2021 08:48:28 -0000
@@ -558,14 +558,11 @@ wakeup_n(const volatile void *ident, int
for (p = TAILQ_FIRST(qp); p != NULL && n != 0; p = pnext) {
pnext = TAILQ_NEXT(p, p_runq);
/*
- * If the rwlock passed to rwsleep() is contended, the
- * CPU will end up calling wakeup() between sleep_setup()
- * and sleep_finish().
+ * This happens if wakeup(9) is called after enqueuing
+ * itself on the sleep queue and both `ident' collide.
*/
- if (p == curproc) {
- KASSERT(p->p_stat == SONPROC);
+ if (p == curproc)
continue;
- }
#ifdef DIAGNOSTIC
if (p->p_stat != SSLEEP && p->p_stat != SSTOP)
panic("wakeup: p_stat is %d", (int)p->p_stat);
