[Xen-devel] Re: [PATCH] CPUIDLE: revise tsc-save/restore to avoid big tsc skew between cpus

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

On 05/12/2008 06:22, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:

> Originally, the sequence for each cpu is [tsc-save, entry deepC, break-evt,
> exit deepC, tsc-restore], the system error is quite easy to be accumulated.
> Once the workloads between cpus are not balanced, the tsc skew between cpus
> will eventually become bigger & begger - more than 10 seconds can be observed.
> 
> Now, we just keep a initial stamp via cstate_init_stamp during the booting/s3
> resuming, which is based on the platform stime. All cpus need only to do
> tsc-restore relative to the initial stamp after exit deepC. The base is fixed,
> and is the same for all cpus, so it can avoid accumulated tsc-skew.
> 
> Signed-off-by: Wei Gang <gang.wei@xxxxxxxxx>

Synchronising to a start-of-day timestamp and extrapolating with cpu_khz is
not a good idea because of the inherent accumulating inaccuracy in cpu_khz
(it only has a fixed number of bits of precision). So, for example, if
certain CPUs have not deep-C-slept for a long while, they will wander from
your 'true' TSC values and then you will see TSC mismatches when the CPU
does eventually C-sleep, or compared with other CPUs when they do so.

More significantly, cpu_khz is no good as a fixed static estimate of CPU
clock speed when we start playing with P states.

I think your new code structure is correct. That is, work out wakeup TSC
value from read_platform_stime() rather than some saved TSC value. However,
you should extrapolate from that CPU's own t->stime_local_stamp,
t->tsc_scale, and t->local_tsc_stamp. It's probably a pretty simple change
to your new cstate_restore_tsc().

You see what I mean?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel