[crac] RFR: Fix CPUFeatures crash on new->old CPU
Jan Kratochvil
jkratochvil at openjdk.org
Fri Oct 27 13:33:04 UTC 2023
On Fri, 27 Oct 2023 12:45:53 GMT, Radim Vansa <rvansa at openjdk.org> wrote:
>> - reproducible on i7-1165G7 "qemu-kvm -cpu host" for a checkpoint and "qemu-kvm -cpu SandyBridge" for its restore
>
> src/hotspot/share/runtime/crac_structs.hpp line 225:
>
>> 223: assert(id > 0, "id is expected to be a PID and therefore > 0");
>> 224: char *d = _path;
>> 225: const char prefix[] = "/tmp/cracshm.";
>
> Have you investigated any other option for transferring the data? While both `/tmp/` and `/dev/shm` are temp files, using `shm_open` looks like the go-to approach for sharing data. I dislike hardcoding the `/tmp` path, though it might be viable since perfMemory also has it hardcoded? In any case, this code is not linux-specific (this should be implemented in `crac_posix.cpp`, I suppose).
`shm_open` calls some string functions. So with the current (mis)design of CPUFeatures it cannot be used. One could call directly the syscall but that is too ugly. And then I do not see why to use SHM with the current design. One process creates the object, writes its data, closes the object. Second process opens the object, deletes the object, reads the data, closes the object. That is a normal usage of a tmp file. Shared memory has its use for some simultaneous data exchange but this is not the case so shared memory is overengineered. There is also limited amount of shared memory available so it is wasteful. Linuxes nowadays even have /tmp as tmpfs by default so even no disk operations are involved.
I agree "/tmp" should not be in this platform-independent file, I will fix that, thanks.
There are more portable functions for temporary files (`mktemp()` et al.) but these all use string functions.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/134#discussion_r1374589002
More information about the crac-dev
mailing list