[core] Apply __atomic_load_n/store_n pattern to Millis64 NO_ATOMICS path

Same treatment as the scheduler counters (#15947):

  - Unlocked reads of millis_major / last_millis at the top of
    Millis64Impl::compute(): switch from plain reads to
    __atomic_load_n(&..., __ATOMIC_RELAXED).
  - Unlocked write of last_millis in the "normal forward progression"
    branch: switch from plain assignment to __atomic_store_n(...,
    __ATOMIC_RELAXED). This is the one write that happens without the
    lock, so it needs to be formally atomic to pair cleanly with the
    unlocked atomic reader in the C++ memory model.
  - Writes under `lock` stay plain (millis_major++, last_millis = now
    inside the near-rollover branch). The lock serialises them against
    other writers.

On ARMv5TE the builtins compile to plain LDR/STR — same codegen, no
libatomic dependency. Updates the "accepting minor races" comment to
describe the formally-defined version of the race.
This commit is contained in:
J. Nick Koston
2026-04-23 06:11:03 -05:00
parent 202bcf5b10
commit 3c2396ab86
+13 -8
View File
@@ -72,10 +72,14 @@ uint64_t Millis64Impl::compute(uint32_t now) {
// Without atomics, this implementation uses locks more aggressively:
// 1. Always locks when near the rollover boundary (within 10 seconds)
// 2. Always locks when detecting a large backwards jump
// 3. Updates without lock in normal forward progression (accepting minor races)
// This is less efficient but necessary without atomic operations.
uint16_t major = millis_major;
uint32_t last = last_millis;
// 3. Updates without lock in normal forward progression.
// Concurrent reads/writes use __atomic_load_n / __atomic_store_n with
// __ATOMIC_RELAXED so the cross-thread accesses are well-defined in the
// C++ memory model. On ARMv5TE these compile to plain LDR/STR. Writers
// holding `lock` use plain assignments; the lock serialises them against
// other writers.
uint16_t major = __atomic_load_n(&millis_major, __ATOMIC_RELAXED);
uint32_t last = __atomic_load_n(&last_millis, __ATOMIC_RELAXED);
// Define a safe window around the rollover point (10 seconds)
// This covers any reasonable scheduler delays or thread preemption
@@ -101,13 +105,14 @@ uint64_t Millis64Impl::compute(uint32_t now) {
// Update last_millis while holding lock
last_millis = now;
} else if (now > last) {
// Normal case: Not near rollover and time moved forward
// Update without lock. While this may cause minor races (microseconds of
// backwards time movement), they're acceptable because:
// Normal case: Not near rollover and time moved forward. Publish the new
// low word without taking the lock. A concurrent writer under lock may
// overwrite this with a slightly-different value, which can produce a
// few microseconds of backwards time movement — acceptable because:
// 1. The scheduler operates at millisecond resolution, not microsecond
// 2. We've already prevented the critical rollover race condition
// 3. Any backwards movement is orders of magnitude smaller than scheduler delays
last_millis = now;
__atomic_store_n(&last_millis, now, __ATOMIC_RELAXED);
}
// If now <= last and we're not near rollover, don't update
// This minimizes backwards time movement