git: 7bf11a2f0c9a - main - epoch: Fix epoch_drain_callbacks()

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=7bf11a2f0c9ad7af00996105fd34e61f1040402b

commit 7bf11a2f0c9ad7af00996105fd34e61f1040402b
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-07-08 17:13:01 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-07-09 13:41:27 +0000

    epoch: Fix epoch_drain_callbacks()
    
    This function is supposed to wait until all pending callbacks have been
    executed.  This is useful in some contexts where we tear down some
    context (like a VNET jail and its associated UMA zones) synchronously,
    and we want to make sure that all pending asynchronous callbacks (which
    may free objects to said UMA zones) have run first.
    
    The implementation schedules a callback on each CPU and waits for them
    all to run.  This assumes that, on a given CPU, callbacks are executed
    in the order that they are pushed.  This assumption depends on the
    implementation of epoch_call_task() and ck_epoch_poll_deferred(), and it
    is not true in general.
    
    Callbacks are pushed onto a per-CPU stack in LIFO order.
    ck_epoch_poll_deferred() first pulls out the callbacks from epoch - 2,
    which are always safe to execute, and in so doing reorders them such
    that the oldest callback as at the top of the stack, so in this case,
    epoch_call_task() will execute them in order.  However,
    ck_epoch_poll_deferred() may determine that it is safe to execute
    callbacks from epoch - 1 (or even from the current epoch if there are
    no active readers), and in this case it will push those callbacks onto
    the returned stack.  This means that epoch_call_task() will invoke those
    newer destructors before the older ones, which means that
    epoch_drain_callbacks() may return early.
    
    Fix the correctness problem by simply doing all of this twice: once the
    first callback is invoked, we know that all of the callbacks that were
    pending at the time that epoch_drain_callbacks() was called are
    scheduled to be executed, so when the second callback is executed we
    know that they must be finished.  This is slow, but it is already slow,
    and the slowness is less noticeable after commit dce56594991.
    
    I note that in an ideal world, this function would not exist, and all of
    the teardown would happen asynchronously, rather than the current
    mismash of synchronous and asynchronous cleanup.
    
    PR:             290201
    Reviewed by:    glebius
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D58030
---
 sys/kern/subr_epoch.c | 50 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/sys/kern/subr_epoch.c b/sys/kern/subr_epoch.c
index 51d66d51db8d..266230b04b43 100644
--- a/sys/kern/subr_epoch.c
+++ b/sys/kern/subr_epoch.c
@@ -986,30 +986,40 @@ epoch_drain_callbacks(epoch_t epoch)
 	sched_unbind(td);
 	td->td_pinned = 0;
 
-	CPU_FOREACH(cpu)
-		epoch->e_drain_count++;
-	CPU_FOREACH(cpu) {
-		er = zpcpu_get_cpu(epoch->e_pcpu_record, cpu);
-		sched_bind(td, cpu);
-		epoch_call(epoch, &epoch_drain_cb, &er->er_drain_ctx);
-	}
+	/*
+	 * Schedule a destructor on each CPU and block until all of them have
+	 * run.  Don't assume that destructors are invoked in order: once we're
+	 * finished draining, do the same thing again to ensure that any
+	 * destructors scheduled after the first pass have also run.
+	 */
+	for (int i = 0; i < 2; i++) {
+		CPU_FOREACH(cpu)
+			epoch->e_drain_count++;
+		CPU_FOREACH(cpu) {
+			er = zpcpu_get_cpu(epoch->e_pcpu_record, cpu);
+			sched_bind(td, cpu);
+			epoch_call(epoch, &epoch_drain_cb, &er->er_drain_ctx);
+		}
 
-	/* restore CPU binding, if any */
-	if (was_bound != 0) {
-		sched_bind(td, old_cpu);
-	} else {
-		/* get thread back to initial CPU, if any */
-		if (old_pinned != 0)
+		/* restore CPU binding, if any */
+		if (was_bound != 0) {
 			sched_bind(td, old_cpu);
-		sched_unbind(td);
-	}
-	/* restore pinned after bind */
-	td->td_pinned = old_pinned;
+		} else {
+			/* get thread back to initial CPU, if any */
+			if (old_pinned != 0)
+				sched_bind(td, old_cpu);
+			sched_unbind(td);
+		}
+		/* restore pinned after bind */
+		td->td_pinned = old_pinned;
+		thread_unlock(td);
 
-	thread_unlock(td);
+		while (epoch->e_drain_count != 0)
+			msleep(epoch, &epoch->e_drain_mtx, PZERO, "EDRAIN", 0);
 
-	while (epoch->e_drain_count != 0)
-		msleep(epoch, &epoch->e_drain_mtx, PZERO, "EDRAIN", 0);
+		thread_lock(td);
+	}
+	thread_unlock(td);
 
 	mtx_unlock(&epoch->e_drain_mtx);
 	sx_xunlock(&epoch->e_drain_sx);