[crac] RFR: PID adjustment on checkpoint [v3]

[Anton Kozlov]

On Wed, 21 Jun 2023 13:50:18 GMT, Roman Marchenko <rmarchenko at openjdk.org> wrote:

>> src/java.base/share/native/launcher/main.c line 339:
>> 
>>> 337:                     spin_last_pid(crac_min_pid);
>>> 338:                 }
>>> 339:             }
>> 
>> I think we may drop get_last_pid completely. E.g. it should be enough to 
>> 
>> if (!set_last_pid(crac_min_pid - 1)) { // set_last_pid reports status
>>   spin_last_pid(...)
>> }
>
> The last PID may be greater than the current PID, so there might be the last PID satisfying CRaC requirements. If so, there is no need to set the last PID at all in such case.

But to understand that our next pid is fine, we need an extra read before write, and that read alone is of the same cost as the unconditional write.

>> src/java.base/share/native/launcher/main.c line 344:
>> 
>>> 342:             // by creating the main process waiting for children before exit.
>>> 343:             g_child_pid = fork();
>>> 344:             if (0 < g_child_pid) {
>> 
>> Does it make sense to check if `g_child_pid < crac_min_pid`? As the ns_last_pid is a subject for races, and instead of guessing what the next pid will be, we'll have the ability to explicity check if the child has the right pid. The child will have to compare its pid with the expected crac_min_pid.
>
> I'm not sure I understand your thoughts. Could you explain what do you mean, please?
> 
> `if (0 < g_child_pid)` here is just a detector for child/parent code to execute, nothing more. At this line, there is no need for child to check its PID.

I mean to check child pid (that is `g_check_pid), does it satisfy our min pid requirements. 

Right now, we have to check last_ns_pid and _assume_ what will be the child pid. Instead, we may create the child and based on the pid it gets, decide if the requirement is satifised, or we need another fork. Althought this will require one extra fork, if last_ns_pid is writeable.

This actually relates to https://github.com/openjdk/crac/pull/86#discussion_r1236956565.
OK, please disregard this.

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

PR Review Comment: https://git.openjdk.org/crac/pull/86#discussion_r1237338111
PR Review Comment: https://git.openjdk.org/crac/pull/86#discussion_r1237339338