Index: sys/kern/kern_intr.c =================================================================== --- sys/kern/kern_intr.c +++ sys/kern/kern_intr.c @@ -65,6 +65,11 @@ #include #endif +#define TESTING 1 +#ifdef TESTING +#include +#endif + /* * Describe an interrupt thread. There is one of these per interrupt event. */ @@ -595,9 +600,15 @@ break; } - /* "SLIST_INSERT_PREVPTR". */ + /* + * A replacement for hypothetical SLIST_INSERT_PREVPTR that uses a + * release fence to ensure that the new element is linked into the list + * after its next pointer has been setup. + * This is required for intr_event_handle() to be able to iterate + * ie_handlers in a lock-less manner. + */ SLIST_NEXT(ih, ih_next) = temp_ih; - *prevptr = ih; + atomic_store_rel_ptr((uintptr_t *)prevptr, (uintptr_t)ih); intr_event_update(ie); @@ -685,6 +696,44 @@ return (ie->ie_source); } +/* + * If intr_event_handle() is running in the ISR context at the time of the call, + * then wait for it to complete. + */ +static void +intr_event_barrier(struct intr_event *ie) +{ + int phase; + + mtx_assert(&ie->ie_lock, MA_OWNED); + phase = ie->ie_phase; + + /* + * Switch phase to direct future interrupts to the other active counter. + * Make sure that any preceding stores are visible before the switch. + */ + KASSERT(ie->ie_active[!phase] == 0, ("idle phase has activity")); + atomic_store_rel_int(&ie->ie_phase, !phase); + + /* + * This code cooperates with wait-free iteration of ie_handlers + * in intr_event_handle. + * Make sure that the removal and the phase update are not reordered + * with the active count check. + * Note that no combination of acquire and release fences can provide + * that guarantee as Store->Load sequences can always be reordered. + */ + atomic_thread_fence_seq_cst(); + + /* + * Now wait on the inactive phase. + * The acquire fence is needed so that that all post-barrier accesses + * are after the check. + */ + while (atomic_load_acq_int(&ie->ie_active[phase]) > 0) + cpu_spinwait(); +} + /* * Sleep until an ithread finishes executing an interrupt handler. * @@ -755,44 +804,34 @@ } /* - * If there is no ithread, then just remove the handler and return. - * XXX: Note that an INTR_FAST handler might be running on another - * CPU! + * If there is no ithread, then directly remove the handler. Note that + * intr_event_handle() iterates ie_handlers in a lock-less fashion, so + * care needs to be taken to keep ie_handlers consistent and to free + * the removed handler only when ie_handlers is quiescent. */ if (ie->ie_thread == NULL) { - SLIST_REMOVE_PREVPTR(prevptr, ih, ih_next); + /* + * Replacement for SLIST_REMOVE_PREVPTR that never trashes the + * next pointer in the removed element. + */ + *prevptr = SLIST_NEXT(ih, ih_next); + intr_event_barrier(ie); + intr_event_update(ie); mtx_unlock(&ie->ie_lock); free(handler, M_ITHREAD); return (0); } /* - * If the interrupt thread is already running, then just mark this - * handler as being dead and let the ithread do the actual removal. - * - * During a cold boot while cold is set, msleep() does not sleep, - * so we have to remove the handler here rather than letting the - * thread do it. + * Let the interrupt thread do the job. + * The interrupt source is disabled when the interrupt thread is + * running, so it does not have to worry about interaction with + * intr_event_handle(). */ - thread_lock(ie->ie_thread->it_thread); - if (!TD_AWAITING_INTR(ie->ie_thread->it_thread) && !cold) { - handler->ih_flags |= IH_DEAD; - - /* - * Ensure that the thread will process the handler list - * again and remove this handler if it has already passed - * it on the list. - * - * The release part of the following store ensures - * that the update of ih_flags is ordered before the - * it_need setting. See the comment before - * atomic_cmpset_acq(&ithd->it_need, ...) operation in - * the ithread_execute_handlers(). - */ - atomic_store_rel_int(&ie->ie_thread->it_need, 1); - } else - SLIST_REMOVE_PREVPTR(prevptr, ih, ih_next); - thread_unlock(ie->ie_thread->it_thread); + KASSERT((handler->ih_flags & IH_DEAD) == 0, + ("duplicate handle remove")); + handler->ih_flags |= IH_DEAD; + intr_event_schedule_thread(ie); while (handler->ih_flags & IH_DEAD) msleep(handler, &ie->ie_lock, 0, "iev_rmh", 0); intr_event_update(ie); @@ -975,6 +1014,10 @@ struct intr_handler *ih, **ihp; SLIST_FOREACH_PREVPTR_SAFE(ih, ihp, &ie->ie_handlers, ih_next) { +#ifdef TESTING + KFAIL_POINT_CODE(DEBUG_FP, + intr_event_execute_handlers, /* nothing */); +#endif /* * If this handler is marked for death, remove it from * the list of handlers and wake up the sleeper. @@ -1151,6 +1194,7 @@ struct trapframe *oldframe; struct thread *td; int ret, thread; + int phase; td = curthread; @@ -1175,7 +1219,17 @@ oldframe = td->td_intr_frame; td->td_intr_frame = frame; + /* + * XXX Not sure how to represent memory_order_consume here, + * as a plain load or as an acquire load. + */ + //phase = atomic_load_acq_int(&ie->ie_phase); + phase = ie->ie_phase; + atomic_add_acq_int(&ie->ie_active[phase], 1); SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) { +#ifdef TESTING + KFAIL_POINT_CODE(DEBUG_FP, intr_event_handle, /* nothing */); +#endif if (ih->ih_filter == NULL) { thread = 1; continue; @@ -1212,6 +1266,8 @@ thread = 1; } } + atomic_add_rel_int(&ie->ie_active[phase], -1); + td->td_intr_frame = oldframe; if (thread) { @@ -1227,7 +1283,9 @@ int error __unused; error = intr_event_schedule_thread(ie); +#ifndef TESTING KASSERT(error == 0, ("bad stray interrupt")); +#endif } critical_exit(); td->td_intr_nesting_level--; Index: sys/sys/interrupt.h =================================================================== --- sys/sys/interrupt.h +++ sys/sys/interrupt.h @@ -121,6 +121,8 @@ struct timeval ie_warntm; int ie_irq; /* Physical irq number if !SOFT. */ int ie_cpu; /* CPU this event is bound to. */ + volatile int ie_phase; /* Switched to establish a barrier. */ + volatile int ie_active[2]; /* Filters in ISR context. */ }; /* Interrupt event flags kept in ie_flags. */