All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@tilera.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>, <cpufreq@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Viresh Kumar <viresh.kumar@linaro.org>
Subject: Re: [PATCH 1/2] time: allow changing the timekeeper clock frequency
Date: Thu, 29 Aug 2013 14:40:59 -0400	[thread overview]
Message-ID: <521F95BB.2060600@tilera.com> (raw)
In-Reply-To: <520BF6EC.8070006@tilera.com>

Ping!  I have this work queued up to push as part of the linux-tile tree for the
merge window.  Is that acceptable to the timekeeping/clocksource folks?
Should I hold it back pending further review?  Or does it make sense to
push it as-is and think about further improvements, if any, for a later release?

https://lkml.org/lkml/2013/8/9/497
https://lkml.org/lkml/2013/8/9/499

Thanks in advance!

On 8/14/2013 5:30 PM, Chris Metcalf wrote:
> On 8/14/2013 2:17 PM, John Stultz wrote:
>> So a long while back we had tried to adapt for clock frequency changes
>> on things like the TSC, but it resulting in *terrible* timekeeping as
>> the latency between the frequency change and the handling of the
>> notifications caused lots of clock drift, making it impossible for NTP
>> or other synchronization methods to work properly.
> We've done quite a bit of testing to show that our current implementation
> doesn't have any clock drift over time.  Basically, we take a machine
> running some workload, sync its time via ntpdate, and then run a script
> that changes the CPU speed up or down continually, with a delay of a couple
> seconds in between so we run for some decent amount of time at each speed.
> Every 5 minutes or so, the script runs ntpdate -q to see what the offset
> from real time is.  The skew we see doing that for a couple of days is
> identical to that seen when we _aren't_ changing the CPU frequency.
>
> A key part of making this work, as noted in the comments at the head of
> timekeeping_chfreq_prep(), is the fact that we do the frequency change
> under stop_machine() to make sure that no CPU gets an opportunity to
> sample the clock while it's being changed.
>
> However, I'm wondering whether you're talking about some other sort of
> much more local clock skew or other frequency effect that perhaps we
> haven't tested for.  (For instance, we haven't actually run this code
> on an NTP server.)  Can you give a bit more detail on exactly what sorts
> of bad behavior you saw with the previous implementation, and things one
> might do to detect them?
>
>
>> So early on we made
>> a requirement that all clocksources have a constant frequency and
>> provided a way to disqualify any clocksources that change frequency.
>>
>> So I'd be very hesitant to try to add any such behavior into the
>> timekeeping core. You may want to try to add some logic in the
>> clocksource driver itself to allow for the variable freq clocksource
>> to output what seems to be a fixed freq,
> So, just to be clear, you're suggesting that we claim our clocksource
> runs at some lower virtual speed (say, 1 MHz), and that internally to
> our clocksource drver we divide down the real frequency to the virtual
> one?
>
>
>> and if we get some time on it
>> to prove that it can be made to work well, then we can see about
>> making it more generic.
>>
>> Does that sound ok?
> That seems possible, although it would seem to make the whole process
> a bit less efficient (e.g., our clocksource will have to maintain its
> own multiplier and offset to convert from real ticks to virtual ticks,
> and then the core code will do the same operation again to convert to
> wall-clock time).  Obviously, we're not really anxious to re-test/re-qualify
> a new implementation of this, but if our current version is or might be
> incompatible with other code in the kernel perhaps that's a safer approach.
>
> What sort of eventual more-generic support were you thinking of?
>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com


WARNING: multiple messages have this Message-ID (diff)
From: Chris Metcalf <cmetcalf@tilera.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	cpufreq@vger.kernel.org, Linux PM list <linux-pm@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Viresh Kumar <viresh.kumar@linaro.org>
Subject: Re: [PATCH 1/2] time: allow changing the timekeeper clock frequency
Date: Thu, 29 Aug 2013 14:40:59 -0400	[thread overview]
Message-ID: <521F95BB.2060600@tilera.com> (raw)
In-Reply-To: <520BF6EC.8070006@tilera.com>

Ping!  I have this work queued up to push as part of the linux-tile tree for the
merge window.  Is that acceptable to the timekeeping/clocksource folks?
Should I hold it back pending further review?  Or does it make sense to
push it as-is and think about further improvements, if any, for a later release?

https://lkml.org/lkml/2013/8/9/497
https://lkml.org/lkml/2013/8/9/499

Thanks in advance!

On 8/14/2013 5:30 PM, Chris Metcalf wrote:
> On 8/14/2013 2:17 PM, John Stultz wrote:
>> So a long while back we had tried to adapt for clock frequency changes
>> on things like the TSC, but it resulting in *terrible* timekeeping as
>> the latency between the frequency change and the handling of the
>> notifications caused lots of clock drift, making it impossible for NTP
>> or other synchronization methods to work properly.
> We've done quite a bit of testing to show that our current implementation
> doesn't have any clock drift over time.  Basically, we take a machine
> running some workload, sync its time via ntpdate, and then run a script
> that changes the CPU speed up or down continually, with a delay of a couple
> seconds in between so we run for some decent amount of time at each speed.
> Every 5 minutes or so, the script runs ntpdate -q to see what the offset
> from real time is.  The skew we see doing that for a couple of days is
> identical to that seen when we _aren't_ changing the CPU frequency.
>
> A key part of making this work, as noted in the comments at the head of
> timekeeping_chfreq_prep(), is the fact that we do the frequency change
> under stop_machine() to make sure that no CPU gets an opportunity to
> sample the clock while it's being changed.
>
> However, I'm wondering whether you're talking about some other sort of
> much more local clock skew or other frequency effect that perhaps we
> haven't tested for.  (For instance, we haven't actually run this code
> on an NTP server.)  Can you give a bit more detail on exactly what sorts
> of bad behavior you saw with the previous implementation, and things one
> might do to detect them?
>
>
>> So early on we made
>> a requirement that all clocksources have a constant frequency and
>> provided a way to disqualify any clocksources that change frequency.
>>
>> So I'd be very hesitant to try to add any such behavior into the
>> timekeeping core. You may want to try to add some logic in the
>> clocksource driver itself to allow for the variable freq clocksource
>> to output what seems to be a fixed freq,
> So, just to be clear, you're suggesting that we claim our clocksource
> runs at some lower virtual speed (say, 1 MHz), and that internally to
> our clocksource drver we divide down the real frequency to the virtual
> one?
>
>
>> and if we get some time on it
>> to prove that it can be made to work well, then we can see about
>> making it more generic.
>>
>> Does that sound ok?
> That seems possible, although it would seem to make the whole process
> a bit less efficient (e.g., our clocksource will have to maintain its
> own multiplier and offset to convert from real ticks to virtual ticks,
> and then the core code will do the same operation again to convert to
> wall-clock time).  Obviously, we're not really anxious to re-test/re-qualify
> a new implementation of this, but if our current version is or might be
> incompatible with other code in the kernel perhaps that's a safer approach.
>
> What sort of eventual more-generic support were you thinking of?
>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com


  reply	other threads:[~2013-08-29 18:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-08 19:34 [PATCH 1/2] time: allow changing the timekeeper clock frequency Chris Metcalf
2013-08-08 19:34 ` Chris Metcalf
2013-08-08 19:38 ` [PATCH 2/2] tile: implement dynamic frequency changing Chris Metcalf
2013-08-08 19:38   ` Chris Metcalf
2013-08-09 19:34 ` [PATCH v2 1/2] time: allow changing the timekeeper clock frequency Chris Metcalf
2013-08-09 19:34   ` Chris Metcalf
2013-08-09 19:34   ` [PATCH v2 2/2] tile: implement dynamic frequency changing Chris Metcalf
2013-08-09 19:34     ` Chris Metcalf
2013-08-14 18:17 ` [PATCH 1/2] time: allow changing the timekeeper clock frequency John Stultz
2013-08-14 21:30   ` Chris Metcalf
2013-08-14 21:30     ` Chris Metcalf
2013-08-29 18:40     ` Chris Metcalf [this message]
2013-08-29 18:40       ` Chris Metcalf
2013-08-29 19:30       ` John Stultz
2013-08-30 14:40         ` Chris Metcalf
2013-08-30 14:40           ` Chris Metcalf
2013-08-30 16:04           ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=521F95BB.2060600@tilera.com \
    --to=cmetcalf@tilera.com \
    --cc=cpufreq@vger.kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=tglx@linutronix.de \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.