Linux: /proc/<pid>/status refactor no2
#1222
Conversation
BenBE
added
enhancement
|
Can we just detect container status once per process instead of per thread? According to |
|
@cgzones What's missing from this branch to be ready? |
cgzones
force-pushed
the
linux_pcaps
branch
2 times, most recently
from
5234a8d to
d5b796d
Compare
cgzones
force-pushed
the
linux_pcaps
branch
2 times, most recently
from
1bb3ee6 to
00e42ea
Compare
linux/LinuxProcessTable.c
Outdated
| #include <unistd.h> | ||
| #include <linux/capability.h> |
There was a problem hiding this comment.
Shouldn't this be behind a check for when capabilities are actually needed?
Also, this header is covered by an IWYU rule asking to import sys/capability.h instead. Or is there explicit reason to use linux/capability.h instead here?
There was a problem hiding this comment.
Fixed the include path.
There is currently no option to disable username coloring for elevated non-root processes, see #1379.
There was a problem hiding this comment.
Reverted the path change, since <sys/capability.h> is provided by libcap-dev, which we don't require, and <linux/capability.h> is by linux-libc-dev.
There was a problem hiding this comment.
AFAICS libcap-dev is checked for in configure.ac if --enable-capabilities is given. And that check looks for libcap(-dev) …
|
@cgzones Thank you very much for these Patches. We would love to try these in production. This PR only applies to current upstream and not latest 3.3.0 release. Since we are unfortunately not as firm with C we can only do so much as to rebuild the debian package for 3.3.0 with a patch applied. This fails when trying this PR for obvious reasons since it has been based on latest upstream master, which apparently includes some significant changes compared to 3.3.0. So may I ask you in case this is not a lot of effort, could you provide with a patch which includes your improvements and cleanly applies to release 3.3.0? Thanks so much in advance! |
|
@kartoffelheinz The code in linux/LinuxProcessTable.c changed quite a lot with conflicting changes, so at the moment I am unable to supply rebased patches for 3.3.0. |
htop-dev#1211 has shown reading /proc/<pid>/status might have a significant performance impact. It was started to be read by default to gather the permitted capabilities of the process. Gather permitted capabilities via the syscall capget(2) instead. cap_get_proc(3) is not used to avoid linking with -lcap.
Document for each block gathering information about a task whether the information is shared for the whole process or specific to a single userland thread. Also avoid system calls for process-shared information and reuse the information from the main task.
Improve maintainability by reordering calls in LinuxProcessList_recurseProcTree() to group them by side-effect or interdependency.
htop-dev#1211 showed reading /proc/<pid>/status might have a significant performance impact. Do not read by default only if actually needed, since the permitted capabilities are no longer gather from it. Improves: 8ea144d ("Linux: Refactor /proc/<pid>/status parsing") Improves: htop-dev#1211
Container engines like docker and podman rely on Linux namespaces. If available check if the target process is inside a different PID namespace than htop. This check will mark sandbox'ed applications, e.g. under bubblewrap, but not light-wight containers, like distrobox.
Might be useful for some users and for debugging the hideRunningInContainer detection.
#1211 showed reading
/proc/<pid>/statusmight have a significantperformance impact. Do not read by default only if actually needed,
since the permitted capabilities are no longer gather from it.
Improves: 8ea144d ("Linux: Refactor /proc//status parsing")
Improves: #1211