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

[Yasumasa Suenaga]

On Mon, 7 Jun 2021 04:55:47 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix comments
>
> I think JFR is the only VM subsystem that currently uses the "fast" time
> that is based on TSC, with a fallback to OS time facilities if "fast" time
> is not enabled.  (There has been discussion (under JDK-8211240) about
> eliminating that distinction and just always using OS time facilities, but
> it hasn't received much attention.)  GC (and maybe other places?) uses the
> dual time mechanism because we want reliable time but also send JFR events.
> So some some of the major VM clients for time information are currently
> paying some cost for having both implementations.
> 
> Currently the TSC frequency is always obtained from the CPUID brand string,
> with the bogomips style estimate in initialize_frequency never being used.
> Rdtsc::is_supported() is true iff VM_Version_Ext::supports_tscinv_ext().
> And initialize_frequency() uses the brand string if supports_tscinv_ext().
> 
> I think the current implementation of the bogomips calculation can
> intermittently produce catastrophically wrong results.  Descheduling at the
> wrong place(s) in the loop can badly mess things up.  I thought there was a
> bug for this, but can't find one.
> 
> Also, there are things like the Intel erratum referenced here:
> http://lkml.iu.edu/hypermail/linux/kernel/1511.1/01048.html
> that make things even more fun.
> 
> I think that detecting a "good" TSC and it's properties (like frequency) is
> pretty hard, and we should not try to duplicate the OS detection or second
> guess it.  I also think that using the "fast" time when the TSC is not
> "good" is a mistake, but I have so far not convinced the JFR folks.
> 
> So I'm not in favor of this change.  I think we should be moving away from
> direct TSC access rather than trying to use it in more cases.

@kimbarrett Did you mean we cannot detect "good" TSC from invariant TSC flag? If so, I have to withdraw this PR. And also TSC support for intel processor should be removed (it will happen in JDK-8211240?) ASAP.

> Currently the TSC frequency is always obtained from the CPUID brand string,
> with the bogomips style estimate in initialize_frequency never being used.

If TSC support will remain, it should be detected from CPUID with EAX = 16H, however it is not available on AMD processor as I said before, so we need to bogomips style calculation.

-------------

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