Messages in this thread | | | Subject | Re: [PATCH] timers/nohz: Update nohz load even if tick already stopped | From | Scott Wood <> | Date | Fri, 08 Nov 2019 02:16:03 -0600 |
| |
On Tue, 2019-11-05 at 10:53 +0100, Thomas Gleixner wrote: > On Tue, 5 Nov 2019, Scott Wood wrote: > > On Tue, 2019-11-05 at 00:43 +0100, Thomas Gleixner wrote: > > > As Peter pointed out to me privately we should rather go and analyze > > > the > > > real thing instead of just applying duct tape. > > > > > > /me drops the patch again. > > > > The warning is due to kernel/sched/idle.c not updating curr- > > >se.exec_start. > > > > While debugging I noticed an issue with a particular load pattern. The > > CPU > > goes non-nohz for a brief time at an interval very close to twice > > tick_period. When the tick is started, the timer expiration is more > > than > > tick_period in the past, so hrtimer_forward() tries to catch up by > > adding > > 2*tick_period to the expiration. Then the tick is stopped before that > > new > > expiration, and when the tick is woken up the expiry is again advanced > > by > > 2*tick_period with the timer never actually > > running. sched_tick_remote() > > does fire every second, but there are streaks of several seconds where > > it > > keeps catching the CPU in a non-nohz state, so neither the normal nor > > remote > > ticks are calling calc_load_nohz_remote(). > > > > Is there a reason to not just remove the hrtimer_forward() from > > tick_nohz_restart(), letting the timer fire if it's in the past, which > > will > > take care of doing hrtimer_forward()? > > Well, no. tick_nohz_restart() can be invoked in a situation where the > timer > is armed for something in the far future (or completelt disabled) due to > previously entering an estimated long idle (or user space execution on > NOHZ_FULL) period. > > That means if the timer is not canceled, realigned to the current tick and > forwarded to the next due tick, the tick will not fire on time causing > another sort of trouble.
That might be true of the expiry on entering tick_nohz_restart(), but it shouldn't be true of ts->last_tick which the expiry is set to before calling hrtimer_forward() -- and if it were, hrtimer_forward() is a no-op when the expiry is in the future.
BTW, the name "last_tick" seems misleading as it's actually the saved expiry, not the last time the tick ran.
-Scott
| |