Re: [PATCH 11/13] timekeeping: remove xtime_update

[Thomas Gleixner]

On Mon, Oct 12 2020 at 15:37, Arnd Bergmann wrote:
> On Mon, Oct 12, 2020 at 3:16 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>> On Thu, Oct 8, 2020 at 5:48 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> The comment about xtime_update() in arch/ia64/kernel/time.c needs
>> an update.
>
> I think the correct action for ia64 would be to make it a
> proper clockevent driver with oneshot support, and remove
> the rest of this logic.

Correct action would be to remove all of arch/ia64 :)

> I could try to rewrite the comment, but I tried not to touch that
> part since I don't understand the logic behind it. Maybe the
> ia64 maintainers can comment here why it even tries to skip
> a timer tick. Is there a danger of ending up with the timer irq
> permanently disabled if the timer_interrupt() function returns
> with the itm register in the past, or is this simply about not having
> too many interrupts in a row?

There was a comment in the initial ia64 code:

                * There is a race condition here: to be on the "safe"
                * side, we process timer ticks until itm.next is
                * ahead of the itc by at least half the timer
                * interval.  This should give us enough time to set
                * the new itm value without losing a timer tick.

The ITM (Interval Timer Match) register is raising an interrupt when the
ITM value matches the ITC (Interval Timer Counter) register. If the new
counter is already past the match then the timer interrupt will happen
once ITC wrapped around and reaches the match value again. Might take
ages for a 64bit counter to do that. :)

IIRC, PXA had the same problem and HPET definitely has it as well. Seems
Intel patented the concept of broken timers, but at least they listened
when they proposed to implement the TSC deadline timer on x86 in the
exact same way.

See hpet_clkevt_set_next_event() for the gory details how to handle that
correctly.

Thanks,

        tglx