RFR: 8255799: AArch64: CPU_A53MAC feature may be set incorrectly

[Anton Kozlov]

On Thu, 5 Nov 2020 01:49:51 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

>>> And while we're talking about Linux, can we really not get the info we need
>> without parsing /proc/cpuinfo? And do we need to parse the entire file?
>> 
>> At present reading /proc/cpuinfo that is the only reliable way I know of to identify whether dcpop is  a supported feature (used to force persistence of data to memory). That is needed to support use of NVRAM-backed MappedByteBuffers.
>
>> 
>> At present reading /proc/cpuinfo that is the only reliable way I know of to identify whether dcpop is a supported feature (used to force persistence of data to memory). That is needed to support use of NVRAM-backed MappedByteBuffers.
> 
> I think that comes from `getauxval() & HWCAP_DCPOP`?

> > It's not a feature of single-core CPUs: AFAIK it's a work around for very old arm64 kernels that only reported a single CPU in `/proc/cpuinfo` on multi-core systems where you may have a mix of different CPU types (i.e. mixed A53/A57 where the A57 is reported in cpuinfo).
> > I wonder if we should just remove this workaround altogether? The patch to list all CPUs in `/proc/cpuinfo` was backported to at least the 3.10 series. I really doubt there's anyone running latest OpenJDK on a A53 with such an old kernel.
> 
> Yes, please.

> > @theRealAph BTW, I think Nick is right that this patch is not needed. Are you ok to reject it?
> 
> We should delete lines 184-187 in the current file: this isn't working as intended since the switch to `os::processor_count()` and as discussed the ancient kernel versions where this was necessary should no longer be in use.

In general, I support to abandon this and remove the CPU_A53MAC instead. But the use of the flag is not clear for me https://github.com/openjdk/jdk/blob/f279ddfa06392f8ea14224e478a00bad33b84e7a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp#L447 This code does not check if there is a mix of CPU types, but does for exact type/number-of-cores combination that stands behind the flag. I don't feel comfortable to remove this `nop` now. It will be great if If someone can clarify this. Meanwhile, I'll check for `madd`/`msub`/... and will followup. 

> > At present reading /proc/cpuinfo that is the only reliable way I know of to identify whether dcpop is a supported feature (used to force persistence of data to memory). That is needed to support use of NVRAM-backed MappedByteBuffers.
> 
> I think that comes from `getauxval() & HWCAP_DCPOP`?

Yes, since JDK-8253015 dcpop feature is deteced by hwcap with guarantee about /proc/cpuinfo https://github.com/openjdk/jdk/commit/ec9bee68#diff-7e6fa90a7bcdbe41687eb8d39c6c6232e6518b019937a87aab75284166ef67bdR157-R159. It's only `hwcap => cpuinfo` implication checked, catching silent suboptimal performance. Failing `<=` should be evident from crashing on using dcpop.

> And while we're talking about Linux, can we really not get the info we need without parsing /proc/cpuinfo? And do we need to parse the entire file?

Another reason to read /proc/cpuinfo is to get vendor, model, etc of the CPUs. I would like to avoid addressing this here.

> Tested slowdebug build on Windows+Arm64 with this patch, and smoked tested it with `jtreg:tier1_compiler_1` successfully.
> 
> Change itself looks go to me too (but I'm not a reviewer).

Thanks!

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

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