2017-03-10 09:17:46

by Vlad Zakharov

[permalink] [raw]
Subject: update timer frequencies

Hello,

I am trying to implement a cpufreq driver for ARC CPUs. 
The point is that ARC timers (including those are used for timekeeping) are driven by the same clock as ARC CPU core(s).

So if cpufreq driver changes CPU frequency timers frequency also updates. 
I added notification handler to ARC timer driver were I attempted to update clocksource frequency with
"__clocksource_update_freq_hz()" but I found that actually the frequency didn't update. 

During my further investigation I mentioned that timekeeping framework doesn't allow frequency changes for more than
11%. This is quiet frustratingly: ARC cores supports such adjustments themselves but current framework API doesn't give
us a chance to use this functionality and in fact it even forbids CPU frequency changes in runtime at all.

Maybe I am mistaken and there is some way to get this going? Or maybe we can add some minor changes to timekeeping
framework to resolve this issue (special flag that allows such changes or something like this?).

Thanks.

--
Best regards,
Vlad Zakharov <[email protected]>


2017-03-10 10:28:17

by Thomas Gleixner

[permalink] [raw]
Subject: Re: update timer frequencies

Vlad,

On Fri, 10 Mar 2017, Vlad Zakharov wrote:
>
> I am trying to implement a cpufreq driver for ARC CPUs. The point is
> that ARC timers (including those are used for timekeeping) are driven by
> the same clock as ARC CPU core(s).

To be honest: That's broken by design and you really should go and tell
your hardware folks to fix that. Proper timekeeping is essential for any
Operating System (not only Linux).

It's well known for more than TWO decades that changing the frequency of
the timekeeper clocksource is a complete disaster, but obviously every
hardware vendor has to learn that the hard way instead of simply learning
from history.

> So if cpufreq driver changes CPU frequency timers frequency also
> updates. I added notification handler to ARC timer driver were I
> attempted to update clocksource frequency with
> "__clocksource_update_freq_hz()" but I found that actually the frequency
> didn't update.
>
> During my further investigation I mentioned that timekeeping framework
> doesn't allow frequency changes for more than 11%. This is quiet
> frustratingly: ARC cores supports such adjustments themselves but current
> framework API doesn't give us a chance to use this functionality and in
> fact it even forbids CPU frequency changes in runtime at all.

It does not forbid CPU frequency changes. It forbids timekeeper clocksource
frequency jumps.

For a very good reason. Having frequency jumps disturbs timekeeping in
several ways (monotonic behaviour, accuracy).

> Maybe I am mistaken and there is some way to get this going? Or maybe we
> can add some minor changes to timekeeping framework to resolve this issue
> (special flag that allows such changes or something like this?).

No, we won't add a special flag because the availability will just
proliferate completely braindead hardware designs.

We had a gazillion of horrible hacks which tried to make this work in the
past, but none of them ever worked reliably under all circumstances. We
won't bring them back.

Your hardware design is broken by making the clocksource clock depend on
the CPU frequency. Tell your hardware people to fix that or just use a
seperate independent clocksource.

Thanks,

tglx

2017-08-04 06:59:08

by Vineet Gupta

[permalink] [raw]
Subject: Re: update timer frequencies

Hi Thomas,

On 03/10/2017 03:58 PM, Thomas Gleixner wrote:
> Vlad,
>
> On Fri, 10 Mar 2017, Vlad Zakharov wrote:
>>
>> I am trying to implement a cpufreq driver for ARC CPUs. The point is
>> that ARC timers (including those are used for timekeeping) are driven by
>> the same clock as ARC CPU core(s).
>
> To be honest: That's broken by design and you really should go and tell
> your hardware folks to fix that. Proper timekeeping is essential for any
> Operating System (not only Linux).
>
> It's well known for more than TWO decades that changing the frequency of
> the timekeeper clocksource is a complete disaster, but obviously every
> hardware vendor has to learn that the hard way instead of simply learning
> from history.
>
...
>
> Your hardware design is broken by making the clocksource clock depend on
> the CPU frequency. Tell your hardware people to fix that or just use a
> seperate independent clocksource.

The hardware is being changed and I had a couple of questions to help do it right:

1. The clocksource timers TIMER1, GFRC, RTC etc will now be clocked independent of
core using a fixed clk. Is there some magic freq value which best works so we can
recommend that to implementors. AFAIKR ARM has 24 MHz.

2. I'm not sure if the timer generating interrupts (periodic or oneshot) needs to
be fed invariant fixed clk or dynamic core clk. Naively it should follow the core
- but what happens to scheduled timers (say TCP timeouts): if this clk changes -
they need to be canceled/updated. If it doesn't then the notion of timing is
broken ? I'm likely not thinking this through correctly.

Thx,
-Vineet

2017-08-04 09:38:15

by Peter Zijlstra

[permalink] [raw]
Subject: Re: update timer frequencies

On Fri, Aug 04, 2017 at 12:28:43PM +0530, Vineet Gupta wrote:

> The hardware is being changed and I had a couple of questions to help do it right:

Awesome ;-)

> 2. I'm not sure if the timer generating interrupts (periodic or oneshot)
> needs to be fed invariant fixed clk or dynamic core clk. Naively it should
> follow the core - but what happens to scheduled timers (say TCP timeouts):
> if this clk changes - they need to be canceled/updated. If it doesn't then
> the notion of timing is broken ? I'm likely not thinking this through
> correctly.

Please keep the timers on the very same clock as your clocksource.
clockevent and clocksource having different (and possibly) drifting
timelines is painful.

Having them on the same clock makes everything so much easier, since you
_know_ what time it is when they fire and won't have to recompute and
possibly rearm the timer.