smp: avoid spinlock starvation when schedulers are serialized

[pguyot]

This is especially useful with valgrind and on CI

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[petermm]

AMP: pick and choose:

PR Review — smp: avoid spinlock starvation when schedulers are serialized

Commit: ad01041562df53f370e2ee3db44effbdb2bb242b
Author: Paul Guyot
File touched: src/libAtomVM/smp.h (+19/-2)

Summary

The change rewrites the atomics-based smp_spinlock_lock() busy-wait so that, after
a configurable number of failed CAS attempts, the thread calls sched_yield(). This
relieves spinlock starvation when more schedulers are runnable than CPUs can serve
(e.g. under Valgrind or oversubscribed CI), letting the lock holder make progress.

Verdict: Approach is correct and worth merging. Two non-blocking improvements
recommended around portability of the sched_yield() detection and the magic
threshold.

What's good

• Semantically equivalent loop. Resetting current = 0 each iteration is still
  required (weak CAS overwrites current on failure), and spurious failures are
  handled. Uncontended acquisition is unchanged.
• Yield only on contention. The yield happens only after a failed acquisition and
  only periodically, so it adds no cost on the fast path.
• Compiles out cleanly on platforms where SMP_SPIN_YIELD is undefined.

Findings

1. (Minor) sched_yield() availability is inferred from OS macros

#if !defined(SMP_SPIN_YIELD) && (defined(__unix__) || defined(__APPLE__) || defined(ESP_PLATFORM))
#include <sched.h>
#define SMP_SPIN_YIELD() sched_yield()
#endif

__unix__ does not guarantee <sched.h> exists or that sched_yield() is declared
under the active feature-test macros; conversely some systems provide sched_yield()
without these predefined macros. AtomVM already has CMake-based function detection
(define_if_function_exists, see src/libAtomVM/CMakeLists.txt:243), so prefer a
real HAVE_SCHED_YIELD check over OS sniffing.

2. (Minor) Magic, power-of-two-only threshold

if ((spins & 0x3F) == 0) {
    SMP_SPIN_YIELD();
}

The 0x3F (= 64) interval is undocumented, not tunable, and the bitmask form forces
the interval to be a power of two. Name it and use a counter reset so it can be tuned
(e.g. raised to 256/1024) without code changes if a contention-heavy workload regresses.

3. (Nit) Ignored return value

sched_yield()'s return value is unused; cast to void to avoid ignored-result
warnings on some libc/compiler combinations.

Suggested fix (diff)

diff --git a/src/libAtomVM/CMakeLists.txt b/src/libAtomVM/CMakeLists.txt
--- a/src/libAtomVM/CMakeLists.txt
+++ b/src/libAtomVM/CMakeLists.txt
@@ include(DefineIfExists)
+define_if_function_exists(libAtomVM sched_yield "sched.h" PUBLIC HAVE_SCHED_YIELD)
 define_if_function_exists(libAtomVM open "fcntl.h" PUBLIC HAVE_OPEN)
 define_if_function_exists(libAtomVM opendir "dirent.h" PUBLIC HAVE_OPENDIR)

diff --git a/src/libAtomVM/smp.h b/src/libAtomVM/smp.h
--- a/src/libAtomVM/smp.h
+++ b/src/libAtomVM/smp.h
@@ static inline void smp_spinlock_init(SpinLock *lock)
     lock->lock = 0;
 }
 
-#if !defined(SMP_SPIN_YIELD) && (defined(__unix__) || defined(__APPLE__) || defined(ESP_PLATFORM))
+#if !defined(SMP_SPIN_YIELD) && defined(HAVE_SCHED_YIELD)
 #include <sched.h>
-#define SMP_SPIN_YIELD() sched_yield()
+#define SMP_SPIN_YIELD() ((void) sched_yield())
 #endif
 
+#if defined(SMP_SPIN_YIELD) && !defined(SMP_SPIN_YIELD_INTERVAL)
+/*
+ * Low enough to let serialized scheduler threads make progress under Valgrind/CI,
+ * while avoiding a syscall on very short transient contention.
+ */
+#define SMP_SPIN_YIELD_INTERVAL 64U
+#endif
+
 static inline void smp_spinlock_lock(SpinLock *lock)
 {
-#ifdef SMP_SPIN_YIELD
+#if defined(SMP_SPIN_YIELD) && (SMP_SPIN_YIELD_INTERVAL > 0)
     unsigned int spins = 0;
 #endif
     int current;
     while (true) {
         current = 0;
         if (ATOMIC_COMPARE_EXCHANGE_WEAK_INT(&lock->lock, &current, 1)) {
             return;
         }
-#ifdef SMP_SPIN_YIELD
-        spins++;
-        if ((spins & 0x3F) == 0) {
+#if defined(SMP_SPIN_YIELD) && (SMP_SPIN_YIELD_INTERVAL > 0)
+        if (++spins >= SMP_SPIN_YIELD_INTERVAL) {
+            spins = 0;
             SMP_SPIN_YIELD();
         }
 #endif
     }
 }

Notes / caveats

• This does not make the spinlock formally starvation-free — a thread can still
  lose the CAS race repeatedly. It improves practical liveness under
  oversubscription, which matches the stated goal.
• sched_yield() on Linux/CFS is not a precise fairness primitive and can hurt
  throughput under heavy contention; acceptable here since it only fires periodically
  and only under contention.
• Ensure SMP_SPIN_YIELD() is safe in every context AtomVM spinlocks are taken
  (it would be invalid from interrupt context). Current usage appears to be normal
  runtime/scheduler locks, so this is fine.

[petermm]

suppose we want changelog entry with SMP_SPIN_YIELD_INTERVAL mention? - your call.