RFR: 8268228: TSC is not used for CPUTimeStampCounter on AMD processor [v2]

[David Holmes]

On 7/06/2021 10:45 am, Yasumasa Suenaga wrote:
> On Fri, 4 Jun 2021 05:24:15 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:
> 
>>> I ran JVM on Ryzen 3300X, and I got following `jdk.CPUTimeStampCounter` event.
>>>
>>>
>>> jdk.CPUTimeStampCounter {
>>>    startTime = 10:41:14.993
>>>    fastTimeEnabled = false
>>>    fastTimeAutoEnabled = true
>>>    osFrequency = 1000000000
>>>    fastTimeFrequency = 1000000000
>>> }
>>>
>>>
>>> I confirmed 3300X supports Invariant TSC (so `fastTimeAutoEnabled` is set to `true`), however it does not seem to be used (`fastTimeEnabled` is `false`).
>>>
>>> Frequency is come from brand string from CPUID (e.g. "Intel(R) Core(TM) i3-8145U CPU @ 2.10GHz"). However AMD processor (Ryzen at least) does not have it ("AMD Ryzen 3 3300X 4-Core Processor").
>>> Fortunately rdtsc_x86.cpp can calculate the frequency like bogomips. We should fallback to it if we cannot get the frequency even if invariant TSC is supported.
>>>
>>> After this change, I got following `jdk.CPUTimeStampCounter` event. Base clock of Ryzen 3 3300X is 3.8GHz, so `fastTimeFrequency` looks good.
>>>
>>>
>>> jdk.CPUTimeStampCounter {
>>>    startTime = 10:33:52.884
>>>    fastTimeEnabled = true
>>>    fastTimeAutoEnabled = true
>>>    osFrequency = 10000000 Hz
>>>    fastTimeFrequency = 3792929124 Hz
>>> }
>>>
>>>
>>> This problem is not only for JFR. I confirmed `Rdtsc` class is used in ticks.cpp , and it relates to GC code at least.
>>
>> Yasumasa Suenaga has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    Fix comments
> 
> I can't see discussion in JDK-8211240 (it seems to be security issue), but I think this PR is not disruptive.

Sorry I hadn't noticed the issue was non-public.

Any PR that suddenly changes how a large set of systems will behave is 
disruptive in my opinion. The ramifications of this PR are not clear to me.

> According to [CPUID specification from AMD](https://www.amd.com/system/files/TechDocs/25481.pdf), invariant TSC flag means we can use TSC as a clocksource - it is same with intel processors.
> 
> Did you mean we can't believe invariant TSC flag other than intel processors?

We don't trust the invariant TSC flag in general. We are aware of Intel 
machines that claim to have invariant TSC and yet the Linux kernel does 
not consider the TSC to be a reliable clocksource. We generally prefer 
to trust the OS in these areas rather than having to deal with a wide 
range of hardware idiosyncracies in the VM - it is neither scalable nor 
maintainable.

The rdtsc usage arose because JFR (at one point in time) needed lower 
latency timers for events than were available via the OS, and then we 
had to change other logging code (ie GC counters) to be consistent with 
the JFR code. JDK-8211240 is a RFE to investigate current latencies and 
if they are adequate then revert to only using OS timers/counters again.

> Indeed Linux kernel hasn't treat it in case of AMD processors because [a patch for Linux kernel](https://patchwork.kernel.org/project/linux-pm/patch/f1643565587e70dd2fe2714e9afa566689211a9a.1433050960.git.len.brown@intel.com/) was focused in intel processors.

A great example of how broken the TSC can be on machines that claim it 
is fine!

Cheers,
David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/4350
>