Messages in this thread | | | Date | Wed, 16 Jun 2021 13:26:30 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/6] posix-cpu-timers: Don't start process wide cputime counter if timer is disabled |
| |
On Wed, Jun 16, 2021 at 12:51:16PM +0200, Frederic Weisbecker wrote: > On Wed, Jun 16, 2021 at 10:51:21AM +0200, Peter Zijlstra wrote:
> > The cpu_timer_enqueue() is in arm_timer() and the condition for calling > > that is: > > > > 'new_expires != 0 && val < new_expires' > > > > Which is not the same as the one you add. > > There are two different things here: > > 1) the threadgroup cputime counter, activated by cpu_clock_sample_group(clkid, > p, true) > > 2) the expiration set (+ the callback enqueued) in arm_timer() > > The issue here is that we go through 1) but not through 2)
Correct, but then I would think the cleanup would need the same conditions as 2, and not something slightly different, which is what confused me.
> > I'm thinking the fundamental problem here is the disconnect between > > cpu_timer_enqueue() and pct->timers_active ? > > You're right it's the core issue. But what prevents the whole to be > fundamentally connected is a circular dependency: we need to know the > threadgroup cputime before arming the timer, but we would need to know > if we arm the timer before starting the threadgroup cputime counter > > To sum up, the current sequence is: > > * fetch the threadgroup cputime AND start the whole threadgroup counter > > * arm the timer if it isn't zero and it hasn't yet expired > > While the ideal sequence should be: > > * fetch the threadgroup cputime (without starting the whole threadgroup counter > yet) > > * arm the timer if it isn't zero and it hasn't yet expired > > * iff we armed the timer, start the whole theadgroup counter > > But that means re-iterating the whole threadgroup and update atomically > the group counter with each task's time.
Right, so by the time patch #5 comes around, you seem to be at the point where you can do:
* fetch cputime and start threadgroup counter
* possibly arm timer
* if expired: - fire now - if armed, disarm (which leads to stop)
Which is the other 'obvious' solution to not starting it.
| |