Date: Tue, 26 Oct 2010 09:56:56 -0700 From: Matthew Fleming <mdf356@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-current@freebsd.org Subject: Re: intr_event_destroy(9) Message-ID: <AANLkTi=7H-pp8XdD=6KLzCbK%2BiVfcq1Xtm-Dyps==GFG@mail.gmail.com> In-Reply-To: <201010261246.42238.jhb@freebsd.org> References: <AANLkTimOG%2BsVxc3mQw3aYhCpsZznw6Cd53CA-YCgtCgd@mail.gmail.com> <201010261246.42238.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 26, 2010 at 9:46 AM, John Baldwin <jhb@freebsd.org> wrote:
> On Tuesday, October 26, 2010 11:55:10 am Matthew Fleming wrote:
>> It looks like a bug in intr_event_destroy(9): I'm trying to unload a
>> new driver being developed internally for NVRAM, and I get this
>> WITNESS warning and hang:
>>
>>
>> # kldunload rnv
>> Sleeping on "ithdty" with the following non-sleepable locks held:
>> exclusive sleep mutex intr event list (intr event list) r =3D 0
>> (0xffffffff806f9560) locked @
>> /data/sb/BR_BONNEVILLE_HW/src/sys/kern/kern_intr.c:404
>> KDB: stack backtrace:
>> [ffffffff801a544d] db_trace_self_wrapper+0x3d
>> [ffffffff802e7b26] witness_warn+0x2f6
>> [ffffffff802a1a43] _sleep+0xc3
>> [ffffffff8026dad5] intr_event_destroy+0xe5
>> [ffffff87b05ba805] rnv_pci_detach+0xc5
>> [ffffffff802c9414] device_detach+0xb4
>> [ffffffff802c974f] devclass_delete_driver+0xdf
>> [ffffffff802c991d] driver_module_handler+0x11d
>> [ffffffff802843a2] module_unload+0x42
>> [ffffffff80279f4b] linker_file_unload+0x19b
>> [ffffffff8027aa1b] kern_kldunload+0x10b
>> [ffffffff802a2609] isi_syscall+0x99
>> [ffffffff804dee3e] ia32_syscall+0x1ce
>> [ffffffff804a7e50] Xint0x80_syscall+0x60
>> --- syscall (444, FreeBSD ELF32, kldunloadf), rip =3D 0x280c1aff, rsp =
=3D
>> 0xffffd44c, rbp =3D 0xffffdc98 ---
>>
>> Looking at intr_event_destroy, I see this snippet from r157728:
>>
>>
>> #ifndef notyet
>> =A0 =A0 =A0 if (ie->ie_thread !=3D NULL) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 ithread_destroy(ie->ie_thread);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 ie->ie_thread =3D NULL;
>> =A0 =A0 =A0 }
>> #endif
>>
>> There is an msleep(9) in ithread_destroy(9). =A0And everywhere else that
>> uses notyet has #ifdef, not #ifndef. =A0So... is this a typo?
>
> No, it's actually on purpose I think as the other bits under notyet destr=
oy
> the thread when the last handler for it goes away.
>
> However, ithread_destroy() does not block in any of 7.x or later:
>
> static void
> ithread_destroy(struct intr_thread *ithread)
> {
> =A0 =A0 =A0 =A0struct thread *td;
>
> =A0 =A0 =A0 =A0CTR2(KTR_INTR, "%s: killing %s", __func__, ithread->it_eve=
nt->ie_name);
> =A0 =A0 =A0 =A0td =3D ithread->it_thread;
> =A0 =A0 =A0 =A0thread_lock(td);
> =A0 =A0 =A0 =A0ithread->it_flags |=3D IT_DEAD;
> =A0 =A0 =A0 =A0if (TD_AWAITING_INTR(td)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0TD_CLR_IWAIT(td);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sched_add(td, SRQ_INTR);
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0thread_unlock(td);
> }
>
> Maybe you have a local change? =A0If so, you can probably unlock the glob=
al
> event_list lock before calling ithread_destroy() (but after the
> TAILQ_REMOVE()) in intr_event_destroy().
Gah, yes, it looks like a local change we can probably do without.
And as it turns out the driver can pass NULL to siw_add() and skip the
intr_event_destroy() anyways.
Thanks for the help!
Sorry for the noise.
matthew
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=7H-pp8XdD=6KLzCbK%2BiVfcq1Xtm-Dyps==GFG>