Date: Sat, 17 Jul 2004 19:33:51 +0000 From: Bosko Milekic <bmilekic@FreeBSD.org> To: Brian Fundakowski Feldman <green@freebsd.org> Cc: "M. Warner Losh" <imp@bsdimp.com> Subject: Re: pccbb crashes when detaching (unsafe interrupt handler) Message-ID: <20040717193351.GA83772@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
Brian Feldman wrote:
>>From currentish kern_intr.c:
> if ((ih->ih_flags & IH_DEAD) != 0) {
> mtx_lock(&ithd->it_lock);
> TAILQ_REMOVE(&ithd->it_handlers, ih,
> ih_next);
> wakeup(ih);
> mtx_unlock(&ithd->it_lock);
> goto restart;
> }
>We add a flag IH_PIN:
> if ((ih->ih_flags & (IH_DEAD | IH_PIN)) != 0) {
> if ((ih->ih_flags & IH_DEAD) == 0) {
> wakeup(ih);
> continue;
> }
> mtx_lock(&ithd->it_lock);
> TAILQ_REMOVE(&ithd->it_handlers,
> ih, ih_next);
> wakeup(ih);
> mtx_unlock(&ithd->it_lock);
> goto restart;
> }
Neither -current nor your version should be holding the ithd lock
across the wakeup(). What is being done is that the ithd lock which
is supposed to protect the ithd data structure is being used to
also protect against a temporary (insignificant) race in wakeup().
However, the race is non-fatal and should exist. The race that
results from trying to plug this one is now between the thread
doing the wakeup returning from the wakeup and dropping the lock
before the woken-up thread picks it up. This is (IMO) more wrong
than the non-fatal race this is trying to plug against.
I'll comment more on your proposed solution later but thought I
should get the above off my chest, because it's not the first time
I see it.
-Bosko
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040717193351.GA83772>