STM32: Fix race in alarm setting, which impacted scheduling.

Detect potential race condition (should be rare) and return false back
to caller, allowing them to handle the possibility that either the
alarm was never set because it was in the past (old meaning of false),
or that in fact the alarm was set and may have fired within the race
window (new meaning of false). In either case, the caller needs to
make sure the callback got called.
This commit is contained in:
RobertTDowling 2023-12-17 15:11:03 -08:00 committed by Dario Nieuwenhuis
parent c4d9feb36e
commit 44cc1954a9
2 changed files with 20 additions and 3 deletions

View File

@ -474,16 +474,29 @@ impl Driver for RtcDriver {
return false; return false;
} }
let safe_timestamp = timestamp.max(t + 3);
// Write the CCR value regardless of whether we're going to enable it now or not. // Write the CCR value regardless of whether we're going to enable it now or not.
// This way, when we enable it later, the right value is already set. // This way, when we enable it later, the right value is already set.
r.ccr(n + 1).write(|w| w.set_ccr(safe_timestamp as u16)); r.ccr(n + 1).write(|w| w.set_ccr(timestamp as u16));
// Enable it if it'll happen soon. Otherwise, `next_period` will enable it. // Enable it if it'll happen soon. Otherwise, `next_period` will enable it.
let diff = timestamp - t; let diff = timestamp - t;
r.dier().modify(|w| w.set_ccie(n + 1, diff < 0xc000)); r.dier().modify(|w| w.set_ccie(n + 1, diff < 0xc000));
// Reevaluate if the alarm timestamp is still in the future
let t = self.now();
if timestamp <= t {
// If alarm timestamp has passed since we set it, we have a race condition and
// the alarm may or may not have fired.
// Disarm the alarm and return `false` to indicate that.
// It is the caller's responsibility to handle this ambiguity.
r.dier().modify(|w| w.set_ccie(n + 1, false));
alarm.timestamp.set(u64::MAX);
return false;
}
// We're confident the alarm will ring in the future.
true true
}) })
} }

View File

@ -108,6 +108,10 @@ pub trait Driver: Send + Sync + 'static {
/// The `Driver` implementation should guarantee that the alarm callback is never called synchronously from `set_alarm`. /// The `Driver` implementation should guarantee that the alarm callback is never called synchronously from `set_alarm`.
/// Rather - if `timestamp` is already in the past - `false` should be returned and alarm should not be set, /// Rather - if `timestamp` is already in the past - `false` should be returned and alarm should not be set,
/// or alternatively, the driver should return `true` and arrange to call the alarm callback as soon as possible, but not synchronously. /// or alternatively, the driver should return `true` and arrange to call the alarm callback as soon as possible, but not synchronously.
/// There is a rare third possibility that the alarm was barely in the future, and by the time it was enabled, it had slipped into the
/// past. This is can be detected by double-checking that the alarm is still in the future after enabling it; if it isn't, `false`
/// should also be returned to indicate that the callback may have been called already by the alarm, but it is not guaranteed, so the
/// caller should also call the callback, just like in the more common `false` case. (Note: This requires idempotency of the callback.)
/// ///
/// When callback is called, it is guaranteed that now() will return a value greater or equal than timestamp. /// When callback is called, it is guaranteed that now() will return a value greater or equal than timestamp.
/// ///