RFR: JDK-8325139: JFR SwapSpace event - add free swap space information on Linux when running in a container environment [v5]

[Severin Gehwolf]

On Thu, 29 Feb 2024 09:09:06 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> The JFR SwapSpace event (added by [JDK-8324287](https://bugs.openjdk.org/browse/JDK-8324287)) has been added but in a Linux containerized environment we currently just report -1, this should be enhanced. There is already some code in the mbeans (but in Java) that does something similar .
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   adjust CgroupV1Subsystem::memory_and_swap_usage_in_bytes

I had another ponder about this and gave it a spin. In order to keep this robust we should add tests for this (either here or in a follow-up bug):

- Verify `jdk.SwapSpace` event returns the correct values inside a container with `--memory=A --memory-swap=A` and asserting `freeSize = 0`
- Verify `jdk.SwapSpace` event returns the correct values inside a container with `--memory=A --memory-swap=A+B` asserting that `0 < freeSize <= B`.
- Verify in `test/jdk/jdk/jfr/event/os/TestSwapSpaceEvent.java` with `-Xlog:os+container=trace` and default flags that if the `os::free_swap_size` message is present, we never get `-1`.

src/hotspot/os/linux/os_linux.cpp line 326:

> 324:       }
> 325:     }
> 326:     return -1;

After some more pondering and testing it appears we'd want to have something like this diff on top so as to not return `-1` as freely. On a linux host system, this would still return `-1` here in the common case due to https://bugs.openjdk.org/browse/JDK-8261242.


$ git diff
diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp
index e2d80e092b3..684eb846be7 100644
--- a/src/hotspot/os/linux/os_linux.cpp
+++ b/src/hotspot/os/linux/os_linux.cpp
@@ -302,7 +302,17 @@ jlong os::total_swap_space() {
   return  (jlong)(si.totalswap * si.mem_unit);
 }
 
+static jlong host_free_swap() {
+    struct sysinfo si;
+    int ret = sysinfo(&si);
+    if (ret != 0) {
+      return -1;
+    }
+    return (jlong)(si.freeswap * si.mem_unit);
+}
+
 jlong os::free_swap_space() {
+  jlong host_free_swap_val = host_free_swap();
   if (OSContainer::is_containerized()) {
     // see src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java
     // long getFreeSwapSpaceSize()
@@ -323,15 +333,12 @@ jlong os::free_swap_space() {
         }
       }
     }
-    return -1;
-  } else {
-    struct sysinfo si;
-    int ret = sysinfo(&si);
-    if (ret != 0) {
-      return -1;
-    }
-    return (jlong)(si.freeswap * si.mem_unit);
+    // unlimited or not supported. Fall through to return host value
+    log_trace(os,container)("os::free_swap_space: container_swap_limit=" JLONG_FORMAT
+                            " container_mem_limit=" JLONG_FORMAT " returning host value: " JLONG_FORMAT,
+                            mem_swap_limit, mem_limit, host_free_swap_val);
   }
+  return host_free_swap_val;
 }

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

Changes requested by sgehwolf (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17966#pullrequestreview-1908931798
PR Review Comment: https://git.openjdk.org/jdk/pull/17966#discussion_r1507611183