svn commit: r357744 - head/sys/kern

[Hans Petter Selasky]

Author: hselasky
Date: Mon Feb 10 20:23:08 2020
New Revision: 357744
URL: https://svnweb.freebsd.org/changeset/base/357744

Log:
  Fix for unbalanced EPOCH(9) usage in the generic kernel interrupt
  handler.
  
  Interrupt handlers are removed via intr_event_execute_handlers() when
  IH_DEAD is set. The thread removing the interrupt is woken up, and
  calls intr_event_update(). When this happens, the ie_hflags are
  cleared and re-built from all the remaining handlers sharing the
  event. When the last IH_NET handler is removed, the IH_NET flag will
  be cleared from ih_hflags (or ie_hflags may still be being rebuilt in
  a different context), and the ithread_execute_handlers() may return
  with ie_hflags missing IH_NET. This can lead to a scenario where
  IH_NET was present before calling ithread_execute_handlers, and is not
  present at its return, meaning the need for epoch must be cached
  locally.
  
  This can happen when loading and unloading network drivers. Also make
  sure the ie_hflags is not cleared before being updated.
  
  This is a regression issue after r357004.
  
  Backtrace:
  panic()
  # trying to access epoch tracker on stack of dead thread
  _epoch_enter_preempt()
  ifunit_ref()
  ifioctl()
  fo_ioctl()
  kern_ioctl()
  sys_ioctl()
  syscallenter()
  amd64_syscall()
  
  Differential Revision:	https://reviews.freebsd.org/D23483
  Reviewed by:	glebius@, gallatin@, mav@, jeff@ and kib@
  Sponsored by:	Mellanox Technologies

Modified:
  head/sys/kern/kern_intr.c

Modified: head/sys/kern/kern_intr.c
==============================================================================
--- head/sys/kern/kern_intr.c	Mon Feb 10 18:28:02 2020	(r357743)
+++ head/sys/kern/kern_intr.c	Mon Feb 10 20:23:08 2020	(r357744)
@@ -189,12 +189,12 @@ intr_event_update(struct intr_event *ie)
 {
 	struct intr_handler *ih;
 	char *last;
-	int missed, space;
+	int missed, space, flags;
 
 	/* Start off with no entropy and just the name of the event. */
 	mtx_assert(&ie->ie_lock, MA_OWNED);
 	strlcpy(ie->ie_fullname, ie->ie_name, sizeof(ie->ie_fullname));
-	ie->ie_hflags = 0;
+	flags = 0;
 	missed = 0;
 	space = 1;
 
@@ -207,8 +207,9 @@ intr_event_update(struct intr_event *ie)
 			space = 0;
 		} else
 			missed++;
-		ie->ie_hflags |= ih->ih_flags;
+		flags |= ih->ih_flags;
 	}
+	ie->ie_hflags = flags;
 
 	/*
 	 * If there is only one handler and its name is too long, just copy in
@@ -1208,6 +1209,7 @@ ithread_loop(void *arg)
 	struct thread *td;
 	struct proc *p;
 	int wake, epoch_count;
+	bool needs_epoch;
 
 	td = curthread;
 	p = td->td_proc;
@@ -1242,20 +1244,22 @@ ithread_loop(void *arg)
 		 * that the load of ih_need in ithread_execute_handlers()
 		 * is ordered after the load of it_need here.
 		 */
-		if (ie->ie_hflags & IH_NET) {
+		needs_epoch =
+		    (atomic_load_int(&ie->ie_hflags) & IH_NET) != 0;
+		if (needs_epoch) {
 			epoch_count = 0;
 			NET_EPOCH_ENTER(et);
 		}
 		while (atomic_cmpset_acq_int(&ithd->it_need, 1, 0) != 0) {
 			ithread_execute_handlers(p, ie);
-			if ((ie->ie_hflags & IH_NET) &&
+			if (needs_epoch &&
 			    ++epoch_count >= intr_epoch_batch) {
 				NET_EPOCH_EXIT(et);
 				epoch_count = 0;
 				NET_EPOCH_ENTER(et);
 			}
 		}
-		if (ie->ie_hflags & IH_NET)
+		if (needs_epoch)
 			NET_EPOCH_EXIT(et);
 		WITNESS_WARN(WARN_PANIC, NULL, "suspending ithread");
 		mtx_assert(&Giant, MA_NOTOWNED);