Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH v8 08/25] timer: Rework idle logic | Date | Tue, 10 Oct 2023 00:15:09 +0200 |
| |
On Wed, Oct 04 2023 at 14:34, Anna-Maria Behnsen wrote: > > - 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);
This is wrongly ordered. base->is_idle must be updated _after_ evaluating base->timers_pending because the below can change nextevt, no?
> + if (base->timers_pending) { > + /* If we missed a tick already, force 0 delta */ > + if (time_before(nextevt, basej)) > + nextevt = basej; > + expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
Thanks,
tglx
| |