tick_nohz_idle_got_tick() is called by cpuidle_reflect() within the idle
loop with interrupts enabled. This function modifies the struct
tick_sched's bitfield "got_idle_tick". However this bitfield is stored
within the same mask as other bitfields that can be modified from
interrupts.
Fortunately so far it looks like the only race that can happen is while
writing ->got_idle_tick to 0, an interrupt fires and writes the
->idle_active field to 0. It's then possible that the interrupted write
to ->got_idle_tick writes back the old value of ->idle_active back to 1.
However if that happens, the worst possible outcome is that the time
spent between that interrupt and the upcoming call to
tick_nohz_idle_exit() is accounted as idle, which is negligible quantity.
Still all the bitfield writes within this struct tick_sched's shadow
mask should be IRQ-safe. Therefore move this bitfield out to its own
storage to avoid further suprises.
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/time/tick-sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
index 35808bbb8a47..3b555e0fa937 100644
--- a/kernel/time/tick-sched.h
+++ b/kernel/time/tick-sched.h
@@ -61,7 +61,6 @@ struct tick_sched {
unsigned int tick_stopped : 1;
unsigned int idle_active : 1;
unsigned int do_timer_last : 1;
- unsigned int got_idle_tick : 1;
/* Tick handling: jiffies stall check */
unsigned int stalled_jiffies;
@@ -73,6 +72,7 @@ struct tick_sched {
ktime_t next_tick;
unsigned long idle_jiffies;
ktime_t idle_waketime;
+ unsigned int got_idle_tick;
/* Idle entry */
seqcount_t idle_sleeptime_seq;
--
2.43.0
On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
> tick_nohz_idle_got_tick() is called by cpuidle_reflect() within the idle
> loop with interrupts enabled. This function modifies the struct
> tick_sched's bitfield "got_idle_tick". However this bitfield is stored
> within the same mask as other bitfields that can be modified from
> interrupts.
>
> Fortunately so far it looks like the only race that can happen is while
> writing ->got_idle_tick to 0, an interrupt fires and writes the
> ->idle_active field to 0. It's then possible that the interrupted write
> to ->got_idle_tick writes back the old value of ->idle_active back to 1.
>
> However if that happens, the worst possible outcome is that the time
> spent between that interrupt and the upcoming call to
> tick_nohz_idle_exit() is accounted as idle, which is negligible quantity.
>
> Still all the bitfield writes within this struct tick_sched's shadow
> mask should be IRQ-safe. Therefore move this bitfield out to its own
> storage to avoid further suprises.
>
> Signed-off-by: Frederic Weisbecker <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>