Date: Mon, 3 Jul 2006 11:14:08 -0700 From: John-Mark Gurney <gurney_j@resnet.uoregon.edu> To: Fredrik Lindberg <fli+freebsd-current@shapeshifter.se> Cc: freebsd-current@freebsd.org, "Christian S. J. Peron" <csjp@freebsd.org> Subject: Re: panic: knlist locked, but should not be Message-ID: <20060703181408.GB734@funkthat.com> In-Reply-To: <44A927AC.7080807@shapeshifter.se> References: <44A927AC.7080807@shapeshifter.se>
next in thread | previous in thread | raw e-mail | index | archive | help
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."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060703181408.GB734>