Re: [PATCHSET] Announce: High-res timers, tickless/dyntick and dynamic HZ

Ingo Molnar
2006-06-19T12:31:28Z

* Con Kolivas <kernel@kolivas.org> wrote:

> Nice work Thomas and Ingo.
>
> The approach to previous dynticks that I was working on had some nasty
> issues with scalability that were not addressable without a complete
> rewrite which is why I abandoned the previous implementation. Your
> approach for using the hires timer events is ultimately a better
> solution and the code base is cleaner so I'm very pleased to see it.

thanks!

> A couple of comments.
>
> One of the problems we enountered with dynticks was that using the
> higher resolution timers such as TSC and HPET to adjust for timer
> ticks over longer periods when skipping ticks made the overall clock
> drift when run for many days and only the PM Timer was not prone to
> this happening. ie the timers were very accurate for short periods but
> over days it would drift. It could well have been a design flaw in the
> dynticks I was maintaining rather than the timers themselves but have
> you checked that this isn't a problem?

not yet. If it's a real problem we could introduce a 'make clock events
more reliable' framework by doing something like always programming
clock event sources into periodic mode and reading their current time
offset [if possible] when the event is processesed (thus compensating
for most of the drift caused by irq processing latency). But if it's not
needed it would be nice to avoid that complexity. I'm also wondering why
the PM timer was the most accurate in that regard - it's almost as slow
to program as the PIT, so i'd have expected it to to show the biggest
drift.

(another technique to reduce drift: we could increase the APIC-priority
of the lapic timer, making it less suspect to drift when there are lots
of other IRQs going on.)

can you think of any other similar 'weird cases' that you saw happen
with dynticks? For example there's the 'APIC stops timer irqs when
entering C3 mode' bug - any similar weirdness we should be careful
about? [right now the patch doesnt handle the C3 mode bug, but it should
be relatively straightforward to blacklist lapic events in that case]

i'm looking at dynticks-060227.patch right now, and there seem to be a
fair amount of dyntick specific changes to ACPI's processor_idle.c code.
Do you remember what those changes were about and should we pick them up
in one way or another?

> The other thing I note is that there is a reasonable amount of
> indirection in fairly hot paths. It looks like there is scope for more
> local variable storage of these indirect calls. [...]

which function(s) were you looking at when coming to this conclusion?
clockevents_init_next_event() perhaps? [we could certainly put
'sources->nextevent' into a local variable there]

> [...] Also if set_next_event is separated from struct clock_event, the
> whole struct looks like a suitable candidate for __read_mostly.

You mean ->event_handler()? We can make all clockevent instantiations
__read_mostly right now - all of the fields of clock_event are static,
even ->event_handler() will change at most once per bootup [when we
switch from low-res into high-res mode].

Ingo