On Wed, 6 Apr 2022 12:44:35 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:
>> jcmd uses >> src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java >> to scan temporary directories to find out processes in the container. It >> checks inode to ensure the temp directory is not conflicted. However inode >> maybe same value between the container and others. Thus we should check >> device id for that case. >> >> For example I saw following case on [distroless >> cc-debian11](https://github.com/GoogleContainerTools/distroless/blob/main/cc/README.md) >> container. I started rescue:jdk19 container with sharing PID namespace. >> `/proc/1/root/tmp` is different from `/tmp` on rescue:jdk19, but they are >> same inode value. However we can see the differense in device id. >> >> >> $ podman run -it --rm --entrypoint=sh --pid=container:fa39662f7352 >> rescue:jdk19 >> / # >> / # stat /tmp >> File: /tmp >> Size: 29 Blocks: 0 IO Block: 4096 directory >> Device: efh/239d Inode: 135674931 Links: 1 >> Access: (1777/drwxrwxrwt) Uid: ( 0/ root) Gid: ( 0/ root) >> Access: 2022-04-05 08:51:37.000000000 >> Modify: 2022-04-05 08:51:37.000000000 >> Change: 2022-04-05 08:51:37.000000000 >> >> / # stat /proc/1/root/tmp >> File: /proc/1/root/tmp >> Size: 29 Blocks: 0 IO Block: 4096 directory >> Device: e1h/225d Inode: 135674931 Links: 1 >> Access: (1777/drwxrwxrwt) Uid: ( 0/ root) Gid: ( 0/ root) >> Access: 2022-04-05 08:51:37.000000000 >> Modify: 2022-04-05 08:50:42.000000000 >> Change: 2022-04-05 08:50:42.000000000 > > Yasumasa Suenaga has updated the pull request incrementally with one > additional commit since the last revision: > > Fix comments The code changes look good, but I think the comment should be cleaned up. src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java line 117: > 115: * skip these duplicated directories. > 116: * Host and container devices could have the same inode value, > 117: * so we also need to check the device id. I would suggest rewording the comments from Line 111 to 117 to the following to be more concise: * When cgroups is enabled, the directory /proc/{pid}/root/tmp may * exist even if the given pid is not running inside a container. In * this case, this directory is usually the same as /tmp and should * be skipped, or else we would get duplicated hsperfdata files. * This case can be detected if the inode and device id of * /proc/{pid}/root/tmp are the same as /tmp. ------------- Changes requested by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8103
