blob: 92533d5303767949aff39f95d8c2b342aafd1c04 [file] [log] [blame]
From a3675a4de4388d22bd5fdc87157ef88d4c56c66e Mon Sep 17 00:00:00 2001
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Date: Thu, 4 May 2023 14:41:55 -0400
Subject: [PATCH] CHROMIUM: tick-sched: Set last_tick correctly so that timer
interrupts happen less
TODO: make this only if the tick_nohz_handler() arrived late, not early.
Some broken hardware may make it arrive earlier.
If there are delays in timer interrupts, or other reasons, we can have
tick_nohz_handler() called in quick succession. This seems useless for
low res, and can momentarily make things appear to be high res.
When the tick is active, a delay in a timer interrupt does not mean the
next tick will also be similarly delayed. This seems counter-intuitive
when we are low res. We want the ticks to be spaced out by at least
TICK_NSEC, not less. So fix that.
Also for stop code, consider the following scenario in low res mode:
1. Assume ts->last_tick is 8.5ms.
2. The CPU exits from idle for some reason, and the tick is restarted.
During this restart, it is determined in tick_nohz_restart() that the
next tick should happen at 9.5ms (HZ=1000). This is programmed to the
clock event (and also recorded into the hrtimer).
3. Just after step 2, the CPU tries to stop the tick while entering idle.
During this, there is a call to tick_nohz_next_event() which sets
ts->timer_expires to 10ms due to rounding to TICK_NSEC.
4. Just after this, tick_nohz_stop_tick() is called which sets
ts->last_tick to 9.5ms (the value recorded in #2 into the hrtimer)
and the clock event device is set to the 10ms due to ts->timer_expires.
5. Now the timer interrupt goes off at 10ms, tick_nohz_restart() is
called again, and it programs the clock event to go off at the
ts->last_tick + TICKNSEC which is 10.5ms.
6. Now the timer interrupt goes off at 10.5ms.
The end result is, we have 2 timer interrupts that went off at a
granularity of less than 1ms which causes timer wheel and hrtimers to
have higher res than they otherwise would.
Fix by setting ts->last_tick in #5 to now, which is really when the last
tick happened. This correct makes tick_nohz_restart() consider the most
recent time that the tick timer fired.
I see a similar issue where tick_nohz_handler() could also program the
next timer event too quickly. For this reason, also set the tick-sched
hrtimer in tick_nohz_handler() to now.
With this, I don't see tick_nohz_handler() firing in quick succession.
( CHROMIUM note:
I am marking it as CHROMIUM for experimentation in Finch and will push
upstream after seeing results. It is possible we will just disable
highres timers if the results are looking good in which case there would
not be a need to push it upstream and we can just disable
CONFIG_HIGH_RES_TIMERS in the builds. However, we may want to exclude
<1K HZ devices such as ARM from that, or still keep the dynamic toggle
option for those -- that's TBD. )
BUG=b:263289152
UPSTREAM-TASK=b:289837863
TEST=cyclictest --smp -p95 -m -i 100 -d 100 with timer_highres=0
Change-Id: I3221e458ec6265d9e4a5f68a3e0b7d1c3558be1b
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Tested-by: Joel Fernandes <joelaf@google.com>
Tested-by: Vineeth Pillai <vineethrp@google.com>
(cherry picked from commit aedb9eb98068331f76f875ac5bfa30d28dfac232)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/4855709
Reviewed-by: Joel Fernandes <joelaf@google.com>
Commit-Queue: Joel Fernandes <joelaf@google.com>
---
kernel/time/tick-sched.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index be77b021e5d63b957465c647bac26714ff7d3825..21f189e8d772375e2572bc21ea81d436f0c3f238 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1403,6 +1403,8 @@ static void tick_nohz_lowres_handler(struct clock_event_device *dev)
tick_sched_do_timer(ts, now);
tick_sched_handle(ts, regs);
+ ts->last_tick = now;
+
/*
* In dynticks mode, tick reprogram is deferred:
* - to the idle task if in dynticks-idle
@@ -1413,6 +1415,7 @@ static void tick_nohz_lowres_handler(struct clock_event_device *dev)
tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
}
+ hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
}
static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
--
2.43.0.rc2.451.g8631bc7472-goog