Re: pccbb crashes when detaching (unsafe interrupt handler)

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