git: 177624f2f425 - main - intr: Remove dead code from intr_event_remove_handler()

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=177624f2f425901bb241789d639a16bded2247ae

commit 177624f2f425901bb241789d639a16bded2247ae
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-07-30 14:35:53 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-07-30 14:35:53 +0000

    intr: Remove dead code from intr_event_remove_handler()
    
    We currently destroy the ithread in intr_event_destroy().  In
    preparation for fixing a bug there, remove this dead code and reorganize
    a bit to avoid some code duplication.  No functional change intended.
    
    Reviewed by:    kib, jhb
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D45490
---
 sys/kern/kern_intr.c | 64 ++++++++++++++++------------------------------------
 1 file changed, 19 insertions(+), 45 deletions(-)

diff --git a/sys/kern/kern_intr.c b/sys/kern/kern_intr.c
index 7a57d964acd9..6ccf758ae266 100644
--- a/sys/kern/kern_intr.c
+++ b/sys/kern/kern_intr.c
@@ -851,9 +851,6 @@ intr_event_remove_handler(void *cookie)
 	struct intr_event *ie;
 	struct intr_handler *ih;
 	struct intr_handler **prevptr;
-#ifdef notyet
-	int dead;
-#endif
 
 	if (handler == NULL)
 		return (EINVAL);
@@ -874,53 +871,30 @@ intr_event_remove_handler(void *cookie)
 		    "interrupt event \"%s\"", handler->ih_name, ie->ie_name);
 	}
 
-	/*
-	 * 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) {
+		/*
+		 * 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.
+		 */
 		CK_SLIST_REMOVE_PREVPTR(prevptr, ih, ih_next);
 		intr_event_barrier(ie);
-		intr_event_update(ie);
-		mtx_unlock(&ie->ie_lock);
-		free(handler, M_ITHREAD);
-		return (0);
+	} else {
+		/*
+		 * 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().
+		 */
+		KASSERT((handler->ih_flags & IH_DEAD) == 0,
+		    ("duplicate handle remove"));
+		handler->ih_flags |= IH_DEAD;
+		intr_event_schedule_thread(ie, NULL);
+		while (handler->ih_flags & IH_DEAD)
+			msleep(handler, &ie->ie_lock, 0, "iev_rmh", 0);
 	}
-
-	/*
-	 * 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().
-	 */
-	KASSERT((handler->ih_flags & IH_DEAD) == 0,
-	    ("duplicate handle remove"));
-	handler->ih_flags |= IH_DEAD;
-	intr_event_schedule_thread(ie, NULL);
-	while (handler->ih_flags & IH_DEAD)
-		msleep(handler, &ie->ie_lock, 0, "iev_rmh", 0);
 	intr_event_update(ie);
-
-#ifdef notyet
-	/*
-	 * XXX: This could be bad in the case of ppbus(8).  Also, I think
-	 * this could lead to races of stale data when servicing an
-	 * interrupt.
-	 */
-	dead = 1;
-	CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) {
-		if (ih->ih_handler != NULL) {
-			dead = 0;
-			break;
-		}
-	}
-	if (dead) {
-		ithread_destroy(ie->ie_thread);
-		ie->ie_thread = NULL;
-	}
-#endif
 	mtx_unlock(&ie->ie_lock);
 	free(handler, M_ITHREAD);
 	return (0);