Re: intr_event_destroy(9)

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 = 0
> (0xffffffff806f9560) locked _at_
> /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 = 0x280c1aff, rsp =
> 0xffffd44c, rbp = 0xffffdc98 ---
> 
> Looking at intr_event_destroy, I see this snippet from r157728:
> 
> 
> #ifndef notyet
> 	if (ie->ie_thread != NULL) {
> 		ithread_destroy(ie->ie_thread);
> 		ie->ie_thread = NULL;
> 	}
> #endif
> 
> There is an msleep(9) in ithread_destroy(9).  And everywhere else that
> uses notyet has #ifdef, not #ifndef.  So... is this a typo?

No, it's actually on purpose I think as the other bits under notyet destroy 
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)
{
	struct thread *td;

	CTR2(KTR_INTR, "%s: killing %s", __func__, ithread->it_event->ie_name);
	td = ithread->it_thread;
	thread_lock(td);
	ithread->it_flags |= IT_DEAD;
	if (TD_AWAITING_INTR(td)) {
		TD_CLR_IWAIT(td);
		sched_add(td, SRQ_INTR);
	}
	thread_unlock(td);
}

Maybe you have a local change?  If so, you can probably unlock the global 
event_list lock before calling ithread_destroy() (but after the 
TAILQ_REMOVE()) in intr_event_destroy().

-- 
John Baldwin