panic: knlist locked, but should not be
John-Mark Gurney
gurney_j at resnet.uoregon.edu
Mon Jul 3 18:37:21 UTC 2006
Fredrik Lindberg wrote this message on Mon, Jul 03, 2006 at 16:20 +0200:
> I'm hitting this "knlist locked"-panic when using BPF descriptors
> together with kqueue/kevent.
>
> panic: knlist locked, but should not be
Yeh, csjp broke it in v1.161 by adjusting the locking w/o fixing the
respective issues.
> #10 0xc051a1e3 in knlist_add (knl=0xc3b69460, kn=0xc3ad65d8, islocked=0)
> at /usr/src/sys/kern/kern_event.c:1571
> #11 0xc059c5de in bpfkqfilter (dev=0x12, kn=0xc3ad65d8) at
> /usr/src/sys/net/bpf.c:1194
> #12 0xc050d9e6 in giant_kqfilter (dev=0xc38f9d00, kn=0xc3ad65d8)
> at /usr/src/sys/kern/kern_conf.c:336
> #13 0xc04e9580 in devfs_kqfilter_f (fp=0xc3ad40d8, kn=0xc3ad65d8)
> at /usr/src/sys/fs/devfs/devfs_vnops.c:444
> #14 0xc0517e6d in filt_fileattach (kn=0x12) at file.h:305
> #15 0xc0518eb0 in kqueue_register (kq=0xc3923780, kev=0xe3a67b9c,
> td=0xc3714a20, waitok=1)
> at /usr/src/sys/kern/kern_event.c:902
> #16 0xc05185f4 in kern_kevent (td=0xc3714a20, fd=3, nchanges=1,
> nevents=0, k_ops=0xe3a67c70,
> timeout=0x0) at /usr/src/sys/kern/kern_event.c:640
> #17 0xc0518491 in kevent (td=0xc3714a20, uap=0xe3a67d04) at
> /usr/src/sys/kern/kern_event.c:572
>
> I thinks the problem is as follows, islocked is 0 which triggers
> the macro KNL_ASSERT_LOCK(knl, islocked);
> (It will only bite you if you run with INVARIANTS on)
>
> The knlist is initialized with the same mutex that protects the
> rest of the bpf_d structure.
> bpfopen(), sys/net/bpf.c:383
>
> knlist_init(&d->bd_sel.si_note, &d->bd_mtx, NULL, NULL, NULL);
>
> In bpfkqfilter() and filt_bpfdetach() this mutex is acquired with
> BPFD_LOCK() before calling knlist_add()/knlist_remove()
>
> #define BPFD_LOCK(bd) mtx_lock(&(bd)->bd_mtx)
>
> Hence, the knlist lock IS locked despite being called with islocked = 0.
> If using the same mutex doesn't cause any additional problems,
> (I haven't noticed anything yet, but there could of course be issues
> that I've overlooked), I think knlist_add should be called
> with islocked = 1. A suggested patch is attached.
>
>
> Fredrik Lindberg
> Index: bpf.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/bpf.c,v
> retrieving revision 1.168
> diff -u -r1.168 bpf.c
> --- bpf.c 15 Jun 2006 15:39:12 -0000 1.168
> +++ bpf.c 3 Jul 2006 11:36:30 -0000
> @@ -1152,7 +1152,7 @@
> d->bd_pid = curthread->td_proc->p_pid;
> kn->kn_fop = &bpfread_filtops;
> kn->kn_hook = d;
> - knlist_add(&d->bd_sel.si_note, kn, 0);
> + knlist_add(&d->bd_sel.si_note, kn, 1);
> BPFD_UNLOCK(d);
>
> return (0);
> @@ -1164,7 +1164,7 @@
> struct bpf_d *d = (struct bpf_d *)kn->kn_hook;
>
> BPFD_LOCK(d);
> - knlist_remove(&d->bd_sel.si_note, kn, 0);
> + knlist_remove(&d->bd_sel.si_note, kn, 1);
> BPFD_UNLOCK(d);
> }
Why not drop the lock lines and keep the 0? As you said since it's
the same lock, locking it a bit later won't hurt...
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
More information about the freebsd-current
mailing list