[crac] RFR: Move CPUFeatures verification to the parent process of JVM [v3]
Radim Vansa
rvansa at openjdk.org
Wed Apr 30 08:26:07 UTC 2025
On Tue, 29 Apr 2025 16:17:50 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:
>> There was originally a mistake:
>> - restoring JVM did restore the image
>> - the restored JVM started checking whether CPU Features of the new host >= CPU Features of the checkpoint host
>>
>> That is difficult as glibc is already configured (IFUNC) in the image for the checkpoint host and calling any such glibc functions in the restored image will crash (as the advanced instructions from misconfigured IFUNC are not available). Some glibc functions had to be reimplemented in a dummy way inside JVM due to this misdesign.
>>
>> This patch changes it to:
>> - restoring JVM checks `cpufeatures` user data in the image against current CPU Features
>> - the restored JVM is started only if the CPU Features are satisfied, restored JVM no longer has to verify anything
>>
>> The patch is a bit of a kitchen sink, there are various improvements of the CPU Features code.
>
> Jan Kratochvil has updated the pull request incrementally with three additional commits since the last revision:
>
> - Update required initialization comment
> - Rename cpufeatures_restore() to cpufeatures_check()
> - Use log error, not assert
src/hotspot/cpu/x86/vm_version_x86.cpp line 2654:
> 2652: _glibc_features = GLIBCFeatures_x64;
> 2653:
> 2654: if (ShowCPUFeatures && !CRaCRestoreFrom)
nitpick: please use parentheses even for single statement
src/hotspot/cpu/x86/vm_version_x86.cpp line 2658:
> 2656:
> 2657: #ifdef LINUX
> 2658: if (!glibc_not_using()) {
If we are ignoring the result I would just move the comment and not use an empty if
test/jdk/jdk/crac/CPUFeatures/SimpleCPUFeatures.sh line 36:
> 34: unset GLIBC_TUNABLES
> 35: $JAVA_HOME/bin/java -XX:CPUFeatures=generic -XX:+ShowCPUFeatures -version 2>&1 | tee /proc/self/fd/2 | grep -q 'openjdk version'
> 36:
Could we add some tests to assert parsing with `glibc.cpu.hwcaps` ?
test/jdk/jdk/crac/fileDescriptors/LoggingVMlogOpenTestNegative.java line 79:
> 77: } finally {
> 78: assertFalse(Files.exists(logPathO));
> 79: if (scenario1) {
The change of behaviour is not clear to me; could you explain this in a comment?
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/227#discussion_r2068038414
PR Review Comment: https://git.openjdk.org/crac/pull/227#discussion_r2068041593
PR Review Comment: https://git.openjdk.org/crac/pull/227#discussion_r2068133735
PR Review Comment: https://git.openjdk.org/crac/pull/227#discussion_r2068147831
More information about the crac-dev
mailing list