kernel/timeout: Serialize handler callbacks on SMP
On multiprocessor systems, it's routine to enter sys_clock_announce() in parallel (the driver will generally announce zero ticks on all but one cpu). When that happens, each call will independently enter the loop over the timeout list. The access is correctly synchronized, so the list handling is correct. But the lock is RELEASED around the invocation of the callback, which means that the individual callbacks may interleave between cpus. That means that individual application-provided callbacks may be executed in parallel, which to the app is indistinguishable from "out of order". That's surprising and error-prone. Don't do it. Place a secondary outer spinlock around the announce loop (but not the timeslicing handling) to correctly serialize the timeout handling on a single cpu. (It should be noted that this was discovered not because of a timeout callback race, but because the resulting simultaneous calls to sys_clock_set_timeout from separate cores seems to cause extremely high latency excursions on intel_adsp hardware using the cavs_timer driver. That hardware issue is still poorly understood, but this fix is desirable regardless.) Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
parent
656f87a89f
commit
b1182bf83b
1 changed files with 19 additions and 0 deletions
|
@ -18,6 +18,17 @@ static sys_dlist_t timeout_list = SYS_DLIST_STATIC_INIT(&timeout_list);
|
|||
|
||||
static struct k_spinlock timeout_lock;
|
||||
|
||||
/* On multiprocessor setups, it's possible to have multiple
|
||||
* sys_clock_announce() calls arrive in parallel (the latest to exit
|
||||
* the driver will generally be announcing zero ticks). But we want
|
||||
* the list of timeouts to be executed serially, so as not to confuse
|
||||
* application code. This lock wraps the announce loop, external to
|
||||
* the nested timeout_lock.
|
||||
*/
|
||||
#ifdef CONFIG_SMP
|
||||
static struct k_spinlock ann_lock;
|
||||
#endif
|
||||
|
||||
#define MAX_WAIT (IS_ENABLED(CONFIG_SYSTEM_CLOCK_SLOPPY_IDLE) \
|
||||
? K_TICKS_FOREVER : INT_MAX)
|
||||
|
||||
|
@ -242,6 +253,10 @@ void sys_clock_announce(int32_t ticks)
|
|||
z_time_slice(ticks);
|
||||
#endif
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
k_spinlock_key_t ann_key = k_spin_lock(&ann_lock);
|
||||
#endif
|
||||
|
||||
k_spinlock_key_t key = k_spin_lock(&timeout_lock);
|
||||
|
||||
announce_remaining = ticks;
|
||||
|
@ -270,6 +285,10 @@ void sys_clock_announce(int32_t ticks)
|
|||
sys_clock_set_timeout(next_timeout(), false);
|
||||
|
||||
k_spin_unlock(&timeout_lock, key);
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
k_spin_unlock(&ann_lock, ann_key);
|
||||
#endif
|
||||
}
|
||||
|
||||
int64_t sys_clock_tick_get(void)
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue