git: b83bd6fec075 - main - x86: ipi_bitmap_handler(): Preempt last, reduce critical section

The branch main has been updated by olce:

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

commit b83bd6fec075f4acf614283cd66a691111847881
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2026-10-13 14:32:15 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2026-10-20 16:46:07 +0000

    x86: ipi_bitmap_handler(): Preempt last, reduce critical section
    
    Preemption should happen last so that other IPIs have a chance to be
    executed in due course, in particular hardclockintr().  So, move
    handling of IPI_PREEMPT at the end.
    
    Reduce the critical section to be in force only around hardclockintr(),
    which needs it to execute entirely without preemption (because of code
    it calls that can enter a critical section; interrupts are already
    disabled in this handler, as usual).
    
    The critical_exit() following hardclockintr() may cause preemption if
    due.  If that happens, there is no need to preempt again on IPI_PREEMPT,
    so make sure sched_preempt() is not called in this case.
    
    This fixes inadvertent removal of the optimization that sched_preempt()
    should be called outside of a critical section to perform an immediate
    context switch in the case of IPI_HARDCLOCK and IPI_PREEMPT being
    handled at the same time.
    
    While here, move IPI_AST handling just before IPI_PREEMPT.  We currently
    do not need to do anything for it, but if we were to, it is likely that
    would have to happen after other IPIs but IPI_PREEMPT.
    
    Reviewed by:    kib
    Fixes:          ff326a187907 ("x86: Restore the critical section around ...")
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D60861
---
 sys/x86/x86/mp_x86.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/sys/x86/x86/mp_x86.c b/sys/x86/x86/mp_x86.c
index eff18b1875af..8fdeff859561 100644
--- a/sys/x86/x86/mp_x86.c
+++ b/sys/x86/x86/mp_x86.c
@@ -1335,6 +1335,7 @@ ipi_bitmap_handler(struct trapframe *frame)
 	struct thread *td;
 	int cpu = PCPU_GET(cpuid);
 	u_int ipi_bitmap;
+	bool do_preempt = true;
 
 	kasan_mark(frame, sizeof(*frame), sizeof(*frame), 0);
 	kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
@@ -1343,16 +1344,6 @@ ipi_bitmap_handler(struct trapframe *frame)
 	ipi_bitmap = atomic_readandclear_int(&cpuid_to_pcpu[cpu]->
 	    pc_ipi_bitmap);
 
-	/*
-	 * sched_preempt() must be called to clear the pending preempt
-	 * IPI to enable delivery of further preempts.  However, the
-	 * critical section will cause extra scheduler lock thrashing
-	 * when used unconditionally.  Only critical_enter() if
-	 * hardclock must also run, which requires the section entry.
-	 */
-	if (ipi_bitmap & (1 << IPI_HARDCLOCK))
-		critical_enter();
-
 	td->td_intr_nesting_level++;
 	oldframe = td->td_intr_frame;
 	td->td_intr_frame = frame;
@@ -1360,11 +1351,24 @@ ipi_bitmap_handler(struct trapframe *frame)
 	if (ipi_bitmap & (1 << IPI_TRACE))
 		stack_capture_intr();
 #endif
-	if (ipi_bitmap & (1 << IPI_PREEMPT)) {
+	if (ipi_bitmap & (1 << IPI_HARDCLOCK)) {
 #ifdef COUNT_IPIS
-		(*ipi_preempt_counts[cpu])++;
+		(*ipi_hardclock_counts[cpu])++;
 #endif
-		sched_preempt(td);
+		/*
+		 * Enter a critical section so that hardclockintr() can execute
+		 * entirely without preemption (as it calls code that can enter
+		 * critical sections).
+		 */
+		critical_enter();
+		hardclockintr();
+		/*
+		 * If we are going to preempt now, do not let sched_preempt() do
+		 * an unnecessary extra preemption below.
+		 */
+		if (td->td_owepreempt != 0)
+			do_preempt = false;
+		critical_exit();
 	}
 	if (ipi_bitmap & (1 << IPI_AST)) {
 #ifdef COUNT_IPIS
@@ -1372,16 +1376,21 @@ ipi_bitmap_handler(struct trapframe *frame)
 #endif
 		/* Nothing to do for AST */
 	}
-	if (ipi_bitmap & (1 << IPI_HARDCLOCK)) {
+	/*
+	 * sched_preempt() causes an immediate switch to another thread when not
+	 * executing in a critical section.  In order to ensure that all other
+	 * IPIs are processed in due course (in particular, hardclockintr()), it
+	 * has to be called last.
+	 */
+	if (ipi_bitmap & (1 << IPI_PREEMPT)) {
 #ifdef COUNT_IPIS
-		(*ipi_hardclock_counts[cpu])++;
+		(*ipi_preempt_counts[cpu])++;
 #endif
-		hardclockintr();
+		if (do_preempt)
+			sched_preempt(td);
 	}
 	td->td_intr_frame = oldframe;
 	td->td_intr_nesting_level--;
-	if (ipi_bitmap & (1 << IPI_HARDCLOCK))
-		critical_exit();
 }
 
 /*