diff --git a/kernel/sched/sched.cpp b/kernel/sched/sched.cpp index d4abf793..02cba24e 100644 --- a/kernel/sched/sched.cpp +++ b/kernel/sched/sched.cpp @@ -349,6 +349,20 @@ __PRIVILEGED_CODE void wake(task* t) { return; } + // Publish this CPU's own deferred off-CPU task before the spin-wait below. + // A switched-out task's on_cpu is cleared lazily at the owning CPU's next + // scheduler trap. Two CPUs that each wake a task still pending-off-CPU on + // the other would otherwise spin forever on each other's on_cpu, neither + // reaching a trap to run finalize_pending_off_cpu(). Publishing here breaks + // that cycle. finalize_pending_off_cpu() mutates per-CPU state non-atomically + // and is otherwise only ever called from the IRQ-masked scheduler trap + // paths; wake() is reachable with interrupts enabled (futex / wait-queue / + // mutex wakeups), so the call must run with interrupts disabled to keep that + // contract and avoid a timer tick corrupting pending_off_cpu_task. + uint64_t irq_flags = cpu::irq_save(); + finalize_pending_off_cpu(); + cpu::irq_restore(irq_flags); + uint32_t task_cpu = __atomic_load_n(&t->exec.cpu, __ATOMIC_RELAXED); if (task_cpu != percpu::current_cpu_id()) { while (__atomic_load_n(&t->exec.on_cpu, __ATOMIC_ACQUIRE)) { diff --git a/kernel/tests/sched/wake_off_cpu_deadlock.test.cpp b/kernel/tests/sched/wake_off_cpu_deadlock.test.cpp new file mode 100644 index 00000000..af0a25e5 --- /dev/null +++ b/kernel/tests/sched/wake_off_cpu_deadlock.test.cpp @@ -0,0 +1,168 @@ +#define STLX_TEST_TIER TIER_SCHED + +#include "stlx_unit_test.h" +#include "helpers.h" +#include "sched/sched.h" +#include "sched/sched_internal.h" +#include "sched/task.h" +#include "smp/smp.h" +#include "percpu/percpu.h" +#include "dynpriv/dynpriv.h" +#include "hw/cpu.h" + +using test_helpers::spin_wait; + +TEST_SUITE(wake_off_cpu_deadlock); + +// Reproduces the two-CPU off-CPU deadlock in sched::wake(). +// +// Background: a task's exec.on_cpu is set to 1 when it is switched in and is +// only cleared lazily at the owning CPU's *next* scheduler trap (the switched +// out task is parked in the per-CPU pending_off_cpu_task and published by +// finalize_pending_off_cpu()). sched::wake() spin-waits on a remote task's +// on_cpu before enqueuing it. +// +// If two CPUs each hold a just-switched-out task in their pending slot (on_cpu +// still 1) and each tries to wake the task parked on the *other* CPU while +// running with interrupts disabled, neither CPU can reach a scheduler trap to +// clear the flag the other is spinning on -> permanent mutual stall. +// +// This test fabricates that exact precondition deterministically: +// - Two controller tasks pin themselves to two distinct non-BSP CPUs with +// interrupts disabled (so their CPUs cannot take a finalizing trap). +// - Each parks a "victim" task in its own CPU's pending slot (on_cpu = 1, +// state = BLOCKED) via sched::defer_off_cpu_finalize(). +// - After a barrier, each controller calls sched::wake() on the *other* +// CPU's victim, entering the cross-CPU on_cpu spin. +// +// On buggy code both controllers spin forever; the BSP detects the stall via a +// bounded watchdog, fails the test, then recovers the wedged CPUs by clearing +// the victims' on_cpu so the suite can continue. On fixed code each wake() +// publishes its own pending task before spinning, so both make progress. + +static constexpr uint32_t CTRL_A = 0; +static constexpr uint32_t CTRL_B = 1; + +static volatile uint32_t g_dl_ready[2]; +static volatile uint32_t g_dl_done[2]; +static volatile uint32_t g_dl_create_failed; +static sched::task* volatile g_dl_victim[2]; + +static void dl_victim_fn(void*) { + sched::exit(0); +} + +static void dl_controller_fn(void* arg) { + uint32_t idx = static_cast(reinterpret_cast(arg)); + uint32_t other = 1u - idx; + + RUN_ELEVATED({ + // Disable interrupts first: this both pins the controller to its CPU + // and prevents any finalizing scheduler trap from clearing the victim's + // on_cpu while the cross-wake is in flight. + uint64_t flags = cpu::irq_save(); + uint32_t cpu = percpu::current_cpu_id(); + + sched::task* victim = sched::create_kernel_task( + dl_victim_fn, nullptr, "dl_victim"); + if (!victim) { + __atomic_store_n(&g_dl_create_failed, 1, __ATOMIC_RELEASE); + __atomic_store_n(&g_dl_ready[idx], 1, __ATOMIC_RELEASE); + __atomic_store_n(&g_dl_done[idx], 1, __ATOMIC_RELEASE); + cpu::irq_restore(flags); + } else { + // Fabricate the "parked off-CPU" state: the victim looks like a task + // that was just switched out on this CPU and is awaiting on_cpu + // publication at the next (never-arriving) trap. + __atomic_store_n(&victim->exec.cpu, cpu, __ATOMIC_RELAXED); + __atomic_store_n(&victim->exec.on_cpu, 1, __ATOMIC_RELEASE); + __atomic_store_n(&victim->state, sched::TASK_STATE_BLOCKED, + __ATOMIC_RELEASE); + __atomic_store_n(&g_dl_victim[idx], victim, __ATOMIC_RELEASE); + + // Install the victim as this CPU's deferred off-CPU task. + sched::defer_off_cpu_finalize(victim); + + // Barrier: both pending slots must be established before any wake. + __atomic_store_n(&g_dl_ready[idx], 1, __ATOMIC_RELEASE); + while (!__atomic_load_n(&g_dl_ready[other], __ATOMIC_ACQUIRE)) { + cpu::relax(); + } + + // Cross-wake the task parked on the other CPU. This is the call that + // deadlocks on buggy code. + sched::task* target = __atomic_load_n(&g_dl_victim[other], + __ATOMIC_ACQUIRE); + if (target) { + sched::wake(target); + } + + __atomic_store_n(&g_dl_done[idx], 1, __ATOMIC_RELEASE); + cpu::irq_restore(flags); + } + }); + + sched::exit(0); +} + +// --- mutual_cross_wake_makes_progress --- +// Proves: two CPUs cross-waking each other's parked-off-CPU task both make +// progress (no permanent spin in sched::wake()). + +TEST(wake_off_cpu_deadlock, mutual_cross_wake_makes_progress) { + uint32_t cpus = smp::cpu_count(); + // Needs two controller CPUs plus the BSP running this test body. + if (cpus < 3) return; + + uint32_t self = percpu::current_cpu_id(); + + // Pick two CPUs distinct from the one running this test. + uint32_t ctrl_cpu[2]; + uint32_t picked = 0; + for (uint32_t c = 0; c < cpus && picked < 2; c++) { + if (c != self) { + ctrl_cpu[picked++] = c; + } + } + ASSERT_EQ(picked, 2u); + + g_dl_ready[0] = 0; + g_dl_ready[1] = 0; + g_dl_done[0] = 0; + g_dl_done[1] = 0; + g_dl_create_failed = 0; + g_dl_victim[0] = nullptr; + g_dl_victim[1] = nullptr; + + RUN_ELEVATED({ + sched::task* a = sched::create_kernel_task( + dl_controller_fn, + reinterpret_cast(static_cast(CTRL_A)), + "dl_ctrl_a"); + sched::task* b = sched::create_kernel_task( + dl_controller_fn, + reinterpret_cast(static_cast(CTRL_B)), + "dl_ctrl_b"); + ASSERT_NOT_NULL(a); + ASSERT_NOT_NULL(b); + sched::enqueue_on(a, ctrl_cpu[0]); + sched::enqueue_on(b, ctrl_cpu[1]); + }); + + bool progressed = spin_wait(&g_dl_done[0]) && spin_wait(&g_dl_done[1]); + + if (!progressed) { + // Deadlock detected: break the wedged controllers out of their on_cpu + // spin so the wedged CPUs recover and the rest of the suite can run. + sched::task* va = __atomic_load_n(&g_dl_victim[0], __ATOMIC_ACQUIRE); + sched::task* vb = __atomic_load_n(&g_dl_victim[1], __ATOMIC_ACQUIRE); + if (va) __atomic_store_n(&va->exec.on_cpu, 0, __ATOMIC_RELEASE); + if (vb) __atomic_store_n(&vb->exec.on_cpu, 0, __ATOMIC_RELEASE); + spin_wait(&g_dl_done[0]); + spin_wait(&g_dl_done[1]); + } + + EXPECT_FALSE(static_cast( + __atomic_load_n(&g_dl_create_failed, __ATOMIC_ACQUIRE))); + EXPECT_TRUE(progressed); +}