From owner-freebsd-current@FreeBSD.ORG Tue Oct 26 16:56:57 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 51539106566B; Tue, 26 Oct 2010 16:56:57 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-pw0-f54.google.com (mail-pw0-f54.google.com [209.85.160.54]) by mx1.freebsd.org (Postfix) with ESMTP id 1EAD98FC16; Tue, 26 Oct 2010 16:56:56 +0000 (UTC) Received: by pwi2 with SMTP id 2so342145pwi.13 for ; Tue, 26 Oct 2010 09:56:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=ByHoVyifibpwfpDGOKh4pmqt6hckwqAlWeE7Tu5pBkg=; b=f1F+TdsdkF9oLFFM66gyEQhdyUSWtQkT9intbUT21HiU56IDD3GPxvlu0acm7bnTPD JWHH/wp05VlDXTy17Gvdd7LWmW4M+deeNoHz9xt76Ydj0W5QRRERa0C97Go/4v1H6YKv q84WgoAvgLcJT6EqS18L4i7jas5GEX//S7ERc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=Gxk6kudom69f15+JfGjvvEGSAGyCsSsfunfQUhNzf3XREUv3OcgJqvFKLs+bT0nsWd A3rytOhoWHEzD0MF7SnCAK1FX89kejiGFrhgnJMbfasQ6lHJLCLmiQp3OkCtsHH9j6Nb W/8Rmng/qZTlzYq4hafeLkK0vGhG1j7PhqEcU= MIME-Version: 1.0 Received: by 10.42.229.6 with SMTP id jg6mr815665icb.121.1288112216304; Tue, 26 Oct 2010 09:56:56 -0700 (PDT) Received: by 10.231.159.198 with HTTP; Tue, 26 Oct 2010 09:56:56 -0700 (PDT) In-Reply-To: <201010261246.42238.jhb@freebsd.org> References: <201010261246.42238.jhb@freebsd.org> Date: Tue, 26 Oct 2010 09:56:56 -0700 Message-ID: From: Matthew Fleming To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org Subject: Re: intr_event_destroy(9) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2010 16:56:57 -0000 On Tue, Oct 26, 2010 at 9:46 AM, John Baldwin 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