Open mmeeks opened 8 months ago
Thinking on CLOEXEC annotation an lsof +fg -p <pid of 1st coolforkit> shows (but could be different cf. https://github.com/lsof-org/lsof/issues/38):
forkit 166205 michael 0u CHR RW,AP,0x80000 136,1 0t0 4 /dev/pts/1
forkit 166205 michael 1u CHR RW,AP,0x80000 136,1 0t0 4 /dev/pts/1
forkit 166205 michael 2u CHR RW,AP,0x80000 136,1 0t0 4 /dev/pts/1
These are cloexec but surely we want to have stdout/stderr in the child:
forkit 166205 michael 3w REG W,AP,LG 0,50 369505 5447559 /tmp/coolwsd.log
forkit 166205 michael 4w REG W,AP,LG 0,50 369505 5447559 /tmp/coolwsd.log
I'm fairly sure we don't want to inherit those.
forkit 166205 michael 5r FIFO ND,0x80000 0,13 0t0 526843 pipe
forkit 166205 michael 6w FIFO W,ND,0x80000 0,13 0t0 526843 pipe
0x80000 - so cloexec - good.
forkit 166205 michael 7r REG LG 0,57 57714 22415747 /opt/libreoffice/co-24.04/instdir/program/types.rdb
forkit 166205 michael 8r REG LG 0,50 18553257 1409452 /usr/share/mythes/th_en_US_v2.dat
forkit 166205 michael 9r REG LG 0,57 688618 22441438 /opt/libreoffice/co-24.04/instdir/program/types/offapi.rdb
forkit 166205 michael 10r REG LG 0,57 355410 22441485 /opt/libreoffice/co-24.04/instdir/program/types/oovbaapi.rdb
forkit 166205 michael 11r REG LG 0,50 18553257 1409452 /usr/share/mythes/th_en_US_v2.dat
forkit 166205 michael 12r REG LG 0,57 2904916 22394530 /opt/libreoffice/co-24.04/instdir/share/config/images_colibre.zip
Files opened by the core are unhelpfully not CLOEXEC @grandinj we should prolly fix this in the sal code that opens them(?)
forkit 166205 michael 13r FIFO ND,0x80000 0,13 0t0 526849 pipe
forkit 166205 michael 14w FIFO W,ND,0x80000 0,13 0t0 526849 pipe
Good.
forkit 166205 michael 15u unix RW,ND 0xffff9dacc5b94840 0t0 526850 type=STREAM
forkit 166205 michael 17u IPv6 RW,ND 541766 0t0 TCP *:9980 (LISTEN)
forkit 166205 michael 18u unix RW,ND 0xffff9dad56ce61c0 0t0 541767 @coolwsd-2yGdB20L@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ type=STREAM
Hmm - I'll add CLOEXEC to various socket calls in online - makes sense.
core seems to set CLOEXEC on pipes and sockets already.
patch to add CLOEXEC to opening files on core is at https://gerrit.libreoffice.org/c/core/+/162647
Long running profiles of demo servers suggest that really quite a lot of time is spent in forkit and also in kit_spare doing fork / sys_execve - a quick strace -e execve shows that all of these are coolmount commands
around five of these per jail:
[pid 1057514] 1705067637.961776 execve("/bin/sh", ["sh", "-c", "/opt/libreoffice/online/coolmount -b /opt/libreoffice/online/systemplate /opt/libreoffice/online/jails/1056869-c351f329/i9FcuX8Q6hJescJX"], 0x3117890 / 129 vars /) = 0 [pid 1057514] 1705067637.963976 execve("/opt/libreoffice/online/coolmount", ["/opt/libreoffice/online/coolmount", "-b", "/opt/libreoffice/online/systemplate", "/opt/libreoffice/online/jails/1056869-c351f329/i9FcuX8Q6hJescJX"], 0x55bb36ee85e0 / 129 vars /) = 0 [pid 1057514] 1705067637.970834 +++ exited with 0 +++ [pid 1057513] 1705067637.970872 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=1057514, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
Those spawn a shell and then coolmount, which is not ideal. This chews up quite a chunk of time:
Currently I believe the mounting is currently done by synchronously forking coolmount from the forkit process.
The cost here is mostly that the forkit has a very large number of shared libraries mapped and pre-initialized, which requires lots of work managing PTEs etc. as we double fork and exec etc.
Instead we should have a helper process communicated with via a UDS that checks PEERCREDS, and parent, and can only be run by coolforkit - and that has a simple text protocol to pass jail identifiers to mount, and that does those sys-calls without any forking in the client; can do that with a simple, synchronous IPC layer I think replacing the coolmount call.
Hopefully a simple enough re-work that would save lots of kernel-level thrash, and save ~10% of the CPU time we can see in the profile I think =)
Thanks!