mirror of
https://github.com/esphome/esphome.git
synced 2026-09-21 20:18:43 +00:00
[scheduler,core] Make NO_ATOMICS writer paths fully memory-model-clean
Switch writer-side plain stores under lock_ to __atomic_store_n with __ATOMIC_RELAXED on NO_ATOMICS. The input value for RMW is read plainly (safe — only writers mutate, serialised by the lock; readers only atomic-load so two reads don't race). Closes the formal C++ memory-model hole where plain-store vs atomic-load was a data race in the standard even though aligned 32-bit STR/LDR on ARMv5TE is atomic in practice. Applies to scheduler.h counter mutators and the under-lock writes to last_millis / millis_major in time_64.cpp's near-rollover branch. Same ARMv5TE codegen (plain STR). ATOMICS / SINGLE paths unchanged.
This commit is contained in:
+22
-14
@@ -550,26 +550,28 @@ class Scheduler {
|
||||
}
|
||||
|
||||
// Increment to_add_count_ (no-op on single-threaded platforms).
|
||||
// On NO_ATOMICS the caller must hold lock_ to serialise RMW against
|
||||
// other writers; reader fast-path uses __atomic_load_n.
|
||||
// On NO_ATOMICS the caller must hold lock_; the atomic store pairs with
|
||||
// the reader's __atomic_load_n in to_add_empty_(). The input-value read is
|
||||
// plain — safe because only writers (serialised by lock_) modify the
|
||||
// counter, and concurrent readers only atomic-load (no conflicting write).
|
||||
void to_add_count_increment_locked_() {
|
||||
#ifdef ESPHOME_THREAD_SINGLE
|
||||
#if defined(ESPHOME_THREAD_SINGLE)
|
||||
// No counter needed — to_add_empty_() checks the vector directly
|
||||
#elif defined(ESPHOME_THREAD_MULTI_ATOMICS)
|
||||
this->to_add_count_.fetch_add(1, std::memory_order_relaxed);
|
||||
#else
|
||||
this->to_add_count_++;
|
||||
__atomic_store_n(&this->to_add_count_, this->to_add_count_ + 1, __ATOMIC_RELAXED);
|
||||
#endif
|
||||
}
|
||||
|
||||
// Reset to_add_count_ (no-op on single-threaded platforms)
|
||||
void to_add_count_clear_locked_() {
|
||||
#ifdef ESPHOME_THREAD_SINGLE
|
||||
#if defined(ESPHOME_THREAD_SINGLE)
|
||||
// No counter needed — to_add_empty_() checks the vector directly
|
||||
#elif defined(ESPHOME_THREAD_MULTI_ATOMICS)
|
||||
this->to_add_count_.store(0, std::memory_order_relaxed);
|
||||
#else
|
||||
this->to_add_count_ = 0;
|
||||
__atomic_store_n(&this->to_add_count_, 0, __ATOMIC_RELAXED);
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -601,7 +603,7 @@ class Scheduler {
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
this->defer_count_.fetch_add(1, std::memory_order_relaxed);
|
||||
#else
|
||||
this->defer_count_++;
|
||||
__atomic_store_n(&this->defer_count_, this->defer_count_ + 1, __ATOMIC_RELAXED);
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -609,7 +611,7 @@ class Scheduler {
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
this->defer_count_.store(0, std::memory_order_relaxed);
|
||||
#else
|
||||
this->defer_count_ = 0;
|
||||
__atomic_store_n(&this->defer_count_, 0, __ATOMIC_RELAXED);
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -636,26 +638,32 @@ class Scheduler {
|
||||
}
|
||||
|
||||
void to_remove_add_locked_(uint32_t count) {
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
#if defined(ESPHOME_THREAD_MULTI_ATOMICS)
|
||||
this->to_remove_.fetch_add(count, std::memory_order_relaxed);
|
||||
#elif defined(ESPHOME_THREAD_MULTI_NO_ATOMICS)
|
||||
__atomic_store_n(&this->to_remove_, this->to_remove_ + count, __ATOMIC_RELAXED);
|
||||
#else
|
||||
this->to_remove_ += count;
|
||||
this->to_remove_ += count;
|
||||
#endif
|
||||
}
|
||||
|
||||
void to_remove_decrement_locked_() {
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
#if defined(ESPHOME_THREAD_MULTI_ATOMICS)
|
||||
this->to_remove_.fetch_sub(1, std::memory_order_relaxed);
|
||||
#elif defined(ESPHOME_THREAD_MULTI_NO_ATOMICS)
|
||||
__atomic_store_n(&this->to_remove_, this->to_remove_ - 1, __ATOMIC_RELAXED);
|
||||
#else
|
||||
this->to_remove_--;
|
||||
this->to_remove_--;
|
||||
#endif
|
||||
}
|
||||
|
||||
void to_remove_clear_locked_() {
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
#if defined(ESPHOME_THREAD_MULTI_ATOMICS)
|
||||
this->to_remove_.store(0, std::memory_order_relaxed);
|
||||
#elif defined(ESPHOME_THREAD_MULTI_NO_ATOMICS)
|
||||
__atomic_store_n(&this->to_remove_, 0, __ATOMIC_RELAXED);
|
||||
#else
|
||||
this->to_remove_ = 0;
|
||||
this->to_remove_ = 0;
|
||||
#endif
|
||||
}
|
||||
|
||||
|
||||
@@ -92,14 +92,14 @@ uint64_t Millis64Impl::compute(uint32_t now) {
|
||||
|
||||
if (now < last && (last - now) > HALF_MAX_UINT32) {
|
||||
// True rollover detected (happens every ~49.7 days)
|
||||
millis_major++;
|
||||
__atomic_store_n(&millis_major, static_cast<uint16_t>(millis_major + 1), __ATOMIC_RELAXED);
|
||||
major++;
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Detected true 32-bit rollover at %" PRIu32 "ms (was %" PRIu32 ")", now, last);
|
||||
#endif /* ESPHOME_DEBUG_SCHEDULER */
|
||||
}
|
||||
// Update last_millis while holding lock
|
||||
last_millis = now;
|
||||
__atomic_store_n(&last_millis, now, __ATOMIC_RELAXED);
|
||||
} else if (now > last) {
|
||||
// Normal case: Not near rollover and time moved forward
|
||||
// Update without lock. While this may cause minor races (microseconds of
|
||||
|
||||
Reference in New Issue
Block a user