From de271f01c2a457480541270df87a76676e472b8f Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Thu, 22 Feb 2001 02:18:32 +0000 Subject: [PATCH] Work around a race condition where an interrupt handler can be removed from an interrupt thread while the interrupt thread is blocked on Giant waiting to execute the interrupt handler being removed. The result was that the intrhand structure would be free'd, and we would call 0xdeadc0de. The work around is to check to see if the interrupt thread is idle when removing a handler. If not, then we mark the interrupt handler as being dead using the new IH_DEAD flag and don't remove it from the interrupt threads' list of handlers. When the interrupt thread resumes, it will see a dead handler while traversing the list of handlers and will remove the handler then. --- sys/kern/kern_intr.c | 37 ++++++++++++++++++++++++++++++++++--- sys/sys/interrupt.h | 1 + 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/sys/kern/kern_intr.c b/sys/kern/kern_intr.c index 1cd558ed0315..1c37284961dd 100644 --- a/sys/kern/kern_intr.c +++ b/sys/kern/kern_intr.c @@ -301,11 +301,29 @@ ithread_remove_handler(void *cookie) ih->ih_name, ithread->it_name); ok: #endif - TAILQ_REMOVE(&ithread->it_handlers, handler, ih_next); - ithread_update(ithread); + /* + * If the interrupt thread is already running, then just mark this + * handler as being dead and let the ithread do the actual removal. + */ + mtx_lock_spin(&sched_lock); + if (ithread->it_proc->p_stat != SWAIT) { + 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. + */ + ithread->it_need = 1; + } else { + TAILQ_REMOVE(&ithread->it_handlers, handler, ih_next); + ithread_update(ithread); + } + mtx_unlock_spin(&sched_lock); mtx_unlock_spin(&ithread_list_lock); - free(handler, M_ITHREAD); + if ((handler->ih_flags & IH_DEAD) == 0) + free(handler, M_ITHREAD); return (0); } @@ -468,6 +486,7 @@ ithread_loop(void *arg) * another pass. */ atomic_store_rel_int(&ithd->it_need, 0); +restart: TAILQ_FOREACH(ih, &ithd->it_handlers, ih_next) { if (ithd->it_flags & IT_SOFT && !ih->ih_need) continue; @@ -480,6 +499,18 @@ ithread_loop(void *arg) if ((ih->ih_flags & IH_MPSAFE) == 0) mtx_lock(&Giant); + if ((ih->ih_flags & IH_DEAD) != 0) { + mtx_lock_spin(&ithread_list_lock); + TAILQ_REMOVE(&ithd->it_handlers, ih, + ih_next); + ithread_update(ithd); + mtx_unlock_spin(&ithread_list_lock); + if (!mtx_owned(&Giant)) + mtx_lock(&Giant); + free(ih, M_ITHREAD); + mtx_unlock(&Giant); + goto restart; + } ih->ih_handler(ih->ih_argument); if ((ih->ih_flags & IH_MPSAFE) == 0) mtx_unlock(&Giant); diff --git a/sys/sys/interrupt.h b/sys/sys/interrupt.h index 1bc48d6dbd6a..9c44d81a619a 100644 --- a/sys/sys/interrupt.h +++ b/sys/sys/interrupt.h @@ -50,6 +50,7 @@ struct intrhand { #define IH_FAST 0x00000001 /* Fast interrupt. */ #define IH_EXCLUSIVE 0x00000002 /* Exclusive interrupt. */ #define IH_ENTROPY 0x00000004 /* Device is a good entropy source. */ +#define IH_DEAD 0x00000008 /* Handler should be removed. */ #define IH_MPSAFE 0x80000000 /* Handler does not need Giant. */ /*