RE: [PATCH] x86: Support always running TSC on Intel CPUs

From: Pallipadi, Venkatesh
Date: Tue Nov 18 2008 - 11:47:26 EST




>-----Original Message-----
>From: Ingo Molnar [mailto:mingo@xxxxxxx]
>Sent: Tuesday, November 18, 2008 8:06 AM
>To: Joe Korty
>Cc: Pallipadi, Venkatesh; H Peter Anvin; Thomas Gleixner; linux-kernel
>Subject: Re: [PATCH] x86: Support always running TSC on Intel CPUs
>
>
>* Joe Korty <joe.korty@xxxxxxxx> wrote:
>
>> On Tue, Nov 18, 2008 at 09:09:52AM +0100, Ingo Molnar wrote:
>> >
>> > * Venki Pallipadi <venkatesh.pallipadi@xxxxxxxxx> wrote:
>> >
>> > > + if (c->x86_power & (1 << 8)) {
>> > > set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>> > > + set_cpu_cap(c, X86_FEATURE_NOSTOP_TSC);
>> > > + }
>> >
>> > hm, the naming is a bit confusing. We now have 3 variants:
>> >
>> > X86_FEATURE_TSC
>> > X86_FEATURE_CONSTANT_TSC
>> > X86_FEATURE_NOSTOP_TSC
>> >
>> > NOSTOP_TSC is basically what CONSTANT_TSC should have been
>to begin
>> > with ;-)
>> >
>> > i'd suggest to rename it to this:
>> >
>> > X86_FEATURE_TSC
>> > X86_FEATURE_CONSTANT_FREQ_TSC
>> > X86_FEATURE_STABLE_TSC
>> >
>> > ... with CONSTANT_FREQ_TSC not having any real role in the
>long run.
>> > (it's similarly problematic to a completely unstable TSC)
>> >
>> > does this sound ok?
>>
>>
>> To me, the new naming has the same head-scratching potential
>> as the old....
>>
>> How about:
>>
>> X86_FEATURE_TSC
>> X86_FEATURE_STABLE_TSC_OBSOLETE
>> X86_FEATURE_STABLE_TSC
>
>the _honest_ naming would be:
>
> X86_FEATURE_TSC
> X86_FEATURE_STABLE_TSC_BUT_NOT_ALWAYS
> X86_FEATURE_STABLE_TSC
>
>;-)
>
>what's head-scratching about X86_FEATURE_CONSTANT_FREQ_TSC? It's a
>limited TSC variant: it follows a reference frequency that does not
>change with cpufreq changes, but it can stop at a whim in C states. So
>it's not "stable" nor really "constant" in the everyday sense.
>

I don't like STABLE_TSC_OBSOLETE as it does not say anything
descriptive about what it means and people have to look at the code
to find out.

My original intention was to split this into two features
- P/T state invariant TSC which counts at constant rate
- C-state invariant TSC that never stops
Some CPUs will have only first feature. Others may have both.

But, I agree it is confusing.

How about?
X86_FEATURE_FIXEDFREQ_MAYSTOP_TSC
X86_FEATURE_ALWAYS_RUNNING_TSC

Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/