| Message ID | 20221025135850.51044-6-anna-maria@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | timer: Move from a push remote at enqueue to a pull at expiry model | expand |
On Tue, Oct 25, 2022 at 03:58:38PM +0200, Anna-Maria Behnsen wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > To improve readability of the code, split base->idle calculation and > expires calculation into separate parts. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de> > --- > kernel/time/timer.c | 29 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/kernel/time/timer.c b/kernel/time/timer.c > index f34bc75ff848..cb4194ecca60 100644 > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -1727,21 +1727,20 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem) > base->clk = nextevt; > } > > - if (time_before_eq(nextevt, basej)) { > - expires = basem; > - base->is_idle = false; > - } else { > - if (base->timers_pending) > - expires = basem + (u64)(nextevt - basej) * TICK_NSEC; > - /* > - * If we expect to sleep more than a tick, mark the base idle. > - * Also the tick is stopped so any added timer must forward > - * the base clk itself to keep granularity small. This idle > - * logic is only maintained for the BASE_STD base, deferrable > - * timers may still see large granularity skew (by design). > - */ > - if ((expires - basem) > TICK_NSEC) > - base->is_idle = true; > + /* > + * Base is idle if the next event is more than a tick away. Also > + * the tick is stopped so any added timer must forward the base clk > + * itself to keep granularity small. This idle logic is only > + * maintained for the BASE_STD base, deferrable timers may still > + * see large granularity skew (by design). > + */ > + base->is_idle = time_after(nextevt, basej + 1); A subtle change, yet welcome, is introduced here. If the next event is just one jiffy ahead, base->is_idle used to be left untouched. So the behaviour was: 1) If the tick was already stopped (so we must be after an idle IRQ), base->is_idle remains true 2) If the tick was not yet stopped, base->is_idle remains false After this patch, 1) becomes: 1) If the tick was already stopped, turn base->is_idle to false As a result, we may spare an IPI if we remotely enqueue a timer to an idle CPU that is going to tick on the next jiffy. Whether it's worth mentioning in the changelog, I leave it to you. > + > + if (base->timers_pending) { > + /* If we missed a tick already, force 0 delta */ > + if (time_before_eq(nextevt, basej)) > + nextevt = basej; Can be time_before() then, right? Reviewed-by: Frederic Weisbecker <frederic@kernel.org> > + expires = basem + (u64)(nextevt - basej) * TICK_NSEC; > } > raw_spin_unlock(&base->lock); > > -- > 2.30.2 >
diff --git a/kernel/time/timer.c b/kernel/time/timer.c index f34bc75ff848..cb4194ecca60 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1727,21 +1727,20 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem) base->clk = nextevt; } - if (time_before_eq(nextevt, basej)) { - expires = basem; - base->is_idle = false; - } else { - if (base->timers_pending) - expires = basem + (u64)(nextevt - basej) * TICK_NSEC; - /* - * If we expect to sleep more than a tick, mark the base idle. - * Also the tick is stopped so any added timer must forward - * the base clk itself to keep granularity small. This idle - * logic is only maintained for the BASE_STD base, deferrable - * timers may still see large granularity skew (by design). - */ - if ((expires - basem) > TICK_NSEC) - base->is_idle = true; + /* + * Base is idle if the next event is more than a tick away. Also + * the tick is stopped so any added timer must forward the base clk + * itself to keep granularity small. This idle logic is only + * maintained for the BASE_STD base, deferrable timers may still + * see large granularity skew (by design). + */ + base->is_idle = time_after(nextevt, basej + 1); + + if (base->timers_pending) { + /* If we missed a tick already, force 0 delta */ + if (time_before_eq(nextevt, basej)) + nextevt = basej; + expires = basem + (u64)(nextevt - basej) * TICK_NSEC; } raw_spin_unlock(&base->lock);