Hi,
from a customer's system I got this panic:
kernel diagnostic assertion "(kn->kn_status & KN_QUEUED) == 0" failed: file "..
/../../../kern/kern_event.c", line 1071
panic() at panic+0xfe
__assert() at __assert+0x25
knote_enqueue() at knote_enqueue+0x8c
knote() at knote+0x47
selwakeup() at selwakeup+0x1b
logwakeup() at logwakeup+0x20
log() at log+0xfc
...
softclock() at softclock+0x315
softintr_dispatch() at softintr_dispatch+0x7f
When looking at the condition in KNOTE_ACTIVATE()
if ((kn->kn_status & (KN_QUEUED | KN_DISABLED)) == 0)
knote_enqueue(kn);
and the assertion in knote_enqueue()
KASSERT((kn->kn_status & KN_QUEUED) == 0);
it is quite obvious that interrupts must be blocked between those.
So put the splhigh() around KNOTE_ACTIVATE() and use a splassert()
within knote_enqueue(). This is more or less where FreeBSD puts
its KQ_LOCK().
ok?
bluhm
Index: kern/kern_event.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.71
diff -u -p -r1.71 kern_event.c
--- kern/kern_event.c 6 Jan 2016 17:58:46 -0000 1.71
+++ kern/kern_event.c 29 Mar 2016 19:15:40 -0000
@@ -338,9 +338,12 @@ void
filt_timerexpire(void *knx)
{
struct knote *kn = knx;
+ int s;
kn->kn_data++;
+ s = splhigh();
KNOTE_ACTIVATE(kn);
+ splx(s);
if ((kn->kn_flags & EV_ONESHOT) == 0)
filt_timer_timeout_add(kn);
@@ -954,7 +957,11 @@ kqueue_wakeup(struct kqueue *kq)
void
knote_activate(struct knote *kn)
{
+ int s;
+
+ s = splhigh();
KNOTE_ACTIVATE(kn);
+ splx(s);
}
/*
@@ -964,10 +971,14 @@ void
knote(struct klist *list, long hint)
{
struct knote *kn, *kn0;
+ int s;
- SLIST_FOREACH_SAFE(kn, list, kn_selnext, kn0)
+ SLIST_FOREACH_SAFE(kn, list, kn_selnext, kn0) {
+ s = splhigh();
if (kn->kn_fop->f_event(kn, hint))
KNOTE_ACTIVATE(kn);
+ splx(s);
+ }
}
/*
@@ -1073,14 +1084,13 @@ void
knote_enqueue(struct knote *kn)
{
struct kqueue *kq = kn->kn_kq;
- int s = splhigh();
+ splassert(IPL_HIGH);
KASSERT((kn->kn_status & KN_QUEUED) == 0);
TAILQ_INSERT_TAIL(&kq->kq_head, kn, kn_tqe);
kn->kn_status |= KN_QUEUED;
kq->kq_count++;
- splx(s);
kqueue_wakeup(kq);
}
