IH-533: Remove usage of forkexecd daemon to execute processes #5581
Conversation
|
Before we are going to merge this I would like to see extended testing. This existing code has been executed millions of times and shown to be robust. We can't afford any instability here. |
|
I think it is worth saying in the commit message that the way in which forkexecd currently operates (double full fork or a large resident memory size) is extremely expensive in XenPV. The changes here remove a layer of RPC (always a good thing), and remove literally millions of hypercalls for pagetable management alone. |
No objections. Can you suggest any tests we have to satisfy this requirement? Current Karma tests are passing. |
|
All tests make heavy use of forkexecd. I would suggest a VM lifecycle stress test like TC-8759 on Linux/NFS. |
Forkexecd was written to avoid some issues with Ocaml and multi-threading. Instead use C code to launch processes and avoid these issues. Interface remains unchanged from Ocaml side but implemntation rely entirely on C code. vfork() is used to avoid performance memory issue. Using fork() in XenPV guests is extremely expensive, currently forkexecd is calling 2 fork()s for each process launched. Reap of the processes are done directly. Code automatically reap child processes to avoid zombies. One small helper is used in case syslog redirection is used. This allows to restart the toolstack and keep launched programs running; note that even with forkexecd daemon one process was used for this purpose. Code tries to keep compability with forkexecd, in particular: - SIGPIPE disposition is set to ignore; - /dev/null is open with O_WRONLY even for stdin; - file descriptors are limited to 1024. We use close_range (if available) to reduce system calls to close file descriptors. Cgroup is set to avoid systemd closing processes on toolstack restart. There's a fuzzer program to check file remapping algorithm; for this reason the algorithm is in a separate file. To turn internal debug on you need to set FORKEXECD_DEBUG_LOGS C preprocessor macro to 1. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
done |
There was a problem hiding this comment.
I would suggest to read https://ocaml.org/manual/5.1/intfc.html and follow all the relevant rules from there.
There are quite a few cases where the code is unsafe (even ignoring the vfork debate for a moment) and will crash at runtime when the garbage collector moves some of the values.
Add more comments to explain some implementations. Remove one optimization making code less readable. Do not use caml_stat_ calls for temporary C memory to potentially avoid Ocaml GC to trigger if runtime changes. Reduce log directory permissions, although disabled by default no reasons to beso wide. Make a macro more readable. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Although error code is never reset back better to stop and report as soon as possible, function could have been overridden. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Address a comment. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Job 3987492 succeeded. |
Forkexecd was written to avoid some issues with Ocaml and multi-threading.
Instead use C code to launch processes and avoid these issues. Interface remains unchanged from Ocaml side but implemntation rely entirely on C code.
vfork() is used to avoid performance memory issue. Reap of the processes are done directly.
Using fork() in XenPV guests is extremely expensive, currently forkexecd is calling 2 fork()s for each process launched.
Code automatically reap child processes to avoid zombies. One small helper is used in case syslog redirection is used. This allows to restart the toolstack and keep launched programs running; note that even with forkexecd daemon one process was used for this purpose.
Code tries to keep compability with forkexecd, in particular:
Cgroup is set to avoid systemd closing processes on toolstack restart. There's a fuzzer program to check file remapping algorithm; for this reason the algorithm is in a separate file.
To turn internal debug on you need to set FORKEXECD_DEBUG_LOGS C preprocessor macro to 1.