Messages in this thread | | | Date | Wed, 16 Jun 2021 11:16:03 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 3/6] posix-cpu-timers: Force next_expiration recalc after timer deletion |
| |
On Fri, Jun 04, 2021 at 01:31:56PM +0200, Frederic Weisbecker wrote: > A timer deletion only dequeues the timer but it doesn't shutdown > the related costly process wide cputimer counter and the tick dependency. > > The following code snippet keeps this overhead around for one week after > the timer deletion: > > void trigger_process_counter(void) > { > timer_t id; > struct itimerspec val = { }; > > val.it_value.tv_sec = 604800; > timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id); > timer_settime(id, 0, &val, NULL); > timer_delete(id); > } > > Make sure the next target's tick recalculates the nearest expiration and > clears the process wide counter and tick dependency if necessary.
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c > index 132fd56fb1cd..bb1f862c785e 100644 > --- a/kernel/time/posix-cpu-timers.c > +++ b/kernel/time/posix-cpu-timers.c > @@ -405,6 +405,33 @@ static int posix_cpu_timer_create(struct k_itimer *new_timer) > return 0; > } > > +/* > + * Dequeue the timer and reset the base if it was its earliest expiration. > + * It makes sure the next tick recalculates the base next expiration so we > + * don't keep the costly process wide cputime counter around for a random > + * amount of time, along with the tick dependency. > + */ > +static void disarm_timer(struct k_itimer *timer, struct task_struct *p) > +{ > + struct cpu_timer *ctmr = &timer->it.cpu; > + struct posix_cputimer_base *base; > + int clkidx; > + > + if (!cpu_timer_dequeue(ctmr)) > + return; > + > + clkidx = CPUCLOCK_WHICH(timer->it_clock); > + > + if (CPUCLOCK_PERTHREAD(timer->it_clock)) > + base = p->posix_cputimers.bases + clkidx; > + else > + base = p->signal->posix_cputimers.bases + clkidx; > + > + if (cpu_timer_getexpires(ctmr) == base->nextevt) > + base->nextevt = 0; > +}
OK, so check_process_timers() unconditionally recomputes ->nextevt in collect_posix_cputimers() provided ->timers_active. It also clears ->timers_active if it finds none are left. This recompute is before all actual consumers of ->nextevt, with one exception.
This will loose the update of ->nextevt in arm_timer(), if one were to happen between this and check_process_timers(), but afaict that has no ill effect. Still that might warrant a comment somewhere.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
| |