From 629bf20f59f98a735ca22018ad00c93580aff5f3 Mon Sep 17 00:00:00 2001 From: Casper Norrbin Date: Mon, 15 Dec 2025 10:23:31 +0000 Subject: [PATCH] 8371408: [Linux] VM.info output for container information is confusing Reviewed-by: sgehwolf, dholmes --- .../os/linux/cgroupV1Subsystem_linux.cpp | 6 +- .../os/linux/cgroupV2Subsystem_linux.cpp | 4 +- src/hotspot/os/linux/osContainer_linux.cpp | 34 +++++++++-- src/hotspot/os/linux/osContainer_linux.hpp | 2 + src/hotspot/os/linux/os_linux.cpp | 59 ++++++++----------- .../containers/docker/TestContainerInfo.java | 16 +---- .../containers/docker/TestLimitsUpdating.java | 12 ++-- .../docker/TestMemoryAwareness.java | 6 +- .../jtreg/containers/docker/TestMisc.java | 22 +++---- .../containers/docker/DockerTestUtils.java | 11 ++++ 10 files changed, 95 insertions(+), 77 deletions(-) diff --git a/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp b/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp index ddcb0db2161..2df604083d2 100644 --- a/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp +++ b/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp @@ -467,9 +467,9 @@ void CgroupV1MemoryController::print_version_specific_info(outputStream* st, phy kmem_max_usage.set_value(temp); } - OSContainer::print_container_helper(st, kmem_limit, "kernel_memory_limit_in_bytes"); - OSContainer::print_container_helper(st, kmem_usage, "kernel_memory_usage_in_bytes"); - OSContainer::print_container_helper(st, kmem_max_usage, "kernel_memory_max_usage_in_bytes"); + OSContainer::print_container_helper(st, kmem_limit, "kernel_memory_limit"); + OSContainer::print_container_helper(st, kmem_usage, "kernel_memory_usage"); + OSContainer::print_container_helper(st, kmem_max_usage, "kernel_memory_max_usage"); } char* CgroupV1Subsystem::cpu_cpuset_cpus() { diff --git a/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp b/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp index f435e53c02c..c61d30e9236 100644 --- a/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp +++ b/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp @@ -378,8 +378,8 @@ void CgroupV2MemoryController::print_version_specific_info(outputStream* st, phy if (memory_swap_limit_value(reader(), swap_limit_val)) { swap_limit.set_value(swap_limit_val); } - OSContainer::print_container_helper(st, swap_current, "memory_swap_current_in_bytes"); - OSContainer::print_container_helper(st, swap_limit, "memory_swap_max_limit_in_bytes"); + OSContainer::print_container_helper(st, swap_current, "memory_swap_current"); + OSContainer::print_container_helper(st, swap_limit, "memory_swap_max_limit"); } char* CgroupV2Controller::construct_path(char* mount_path, const char* cgroup_path) { diff --git a/src/hotspot/os/linux/osContainer_linux.cpp b/src/hotspot/os/linux/osContainer_linux.cpp index d86bbf7428a..15a6403d07f 100644 --- a/src/hotspot/os/linux/osContainer_linux.cpp +++ b/src/hotspot/os/linux/osContainer_linux.cpp @@ -287,20 +287,44 @@ bool OSContainer::pids_current(uint64_t& value) { return cgroup_subsystem->pids_current(value); } +template struct metric_fmt; +template<> struct metric_fmt { static constexpr const char* fmt = "%llu"; }; +template<> struct metric_fmt { static constexpr const char* fmt = "%lu"; }; +template<> struct metric_fmt { static constexpr const char* fmt = "%d"; }; +template<> struct metric_fmt { static constexpr const char* fmt = "%s"; }; + +template void OSContainer::print_container_metric(outputStream*, const char*, unsigned long long int, const char*); +template void OSContainer::print_container_metric(outputStream*, const char*, unsigned long int, const char*); +template void OSContainer::print_container_metric(outputStream*, const char*, int, const char*); +template void OSContainer::print_container_metric(outputStream*, const char*, const char*, const char*); + +template +void OSContainer::print_container_metric(outputStream* st, const char* metrics, T value, const char* unit) { + constexpr int max_length = 38; // Longest "metric: value" string ("maximum number of tasks: not supported") + constexpr int longest_value = max_length - 11; // Max length - shortest "metric: " string ("cpu_quota: ") + char value_str[longest_value + 1] = {}; + os::snprintf_checked(value_str, longest_value, metric_fmt::fmt, value); + st->print("%s: %*s", metrics, max_length - static_cast(strlen(metrics)) - 2, value_str); // -2 for the ": " + if (unit[0] != '\0') { + st->print_cr(" %s", unit); + } else { + st->print_cr(""); + } +} + void OSContainer::print_container_helper(outputStream* st, MetricResult& res, const char* metrics) { - st->print("%s: ", metrics); if (res.success()) { if (res.value() != value_unlimited) { if (res.value() >= 1024) { - st->print_cr(PHYS_MEM_TYPE_FORMAT " k", (physical_memory_size_type)(res.value() / K)); + print_container_metric(st, metrics, res.value() / K, "kB"); } else { - st->print_cr(PHYS_MEM_TYPE_FORMAT, res.value()); + print_container_metric(st, metrics, res.value(), "B"); } } else { - st->print_cr("%s", "unlimited"); + print_container_metric(st, metrics, "unlimited"); } } else { // Not supported - st->print_cr("%s", "unavailable"); + print_container_metric(st, metrics, "unavailable"); } } diff --git a/src/hotspot/os/linux/osContainer_linux.hpp b/src/hotspot/os/linux/osContainer_linux.hpp index 895c99ba167..11c3e086feb 100644 --- a/src/hotspot/os/linux/osContainer_linux.hpp +++ b/src/hotspot/os/linux/osContainer_linux.hpp @@ -65,6 +65,8 @@ class OSContainer: AllStatic { static void init(); static void print_version_specific_info(outputStream* st); static void print_container_helper(outputStream* st, MetricResult& res, const char* metrics); + template + static void print_container_metric(outputStream* st, const char* metrics, T value, const char* unit = ""); static inline bool is_containerized(); static const char * container_type(); diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp index 8af89bb3a71..7f6b4851013 100644 --- a/src/hotspot/os/linux/os_linux.cpp +++ b/src/hotspot/os/linux/os_linux.cpp @@ -2432,62 +2432,57 @@ bool os::Linux::print_container_info(outputStream* st) { st->print_cr("container (cgroup) information:"); const char *p_ct = OSContainer::container_type(); - st->print_cr("container_type: %s", p_ct != nullptr ? p_ct : "not supported"); + OSContainer::print_container_metric(st, "container_type", p_ct != nullptr ? p_ct : "not supported"); char *p = OSContainer::cpu_cpuset_cpus(); - st->print_cr("cpu_cpuset_cpus: %s", p != nullptr ? p : "not supported"); + OSContainer::print_container_metric(st, "cpu_cpuset_cpus", p != nullptr ? p : "not supported"); free(p); p = OSContainer::cpu_cpuset_memory_nodes(); - st->print_cr("cpu_memory_nodes: %s", p != nullptr ? p : "not supported"); + OSContainer::print_container_metric(st, "cpu_memory_nodes", p != nullptr ? p : "not supported"); free(p); int i = -1; bool supported = OSContainer::active_processor_count(i); - st->print("active_processor_count: "); if (supported) { assert(i > 0, "must be"); if (ActiveProcessorCount > 0) { - st->print_cr("%d, but overridden by -XX:ActiveProcessorCount %d", i, ActiveProcessorCount); + OSContainer::print_container_metric(st, "active_processor_count", ActiveProcessorCount, "(from -XX:ActiveProcessorCount)"); } else { - st->print_cr("%d", i); + OSContainer::print_container_metric(st, "active_processor_count", i); } } else { - st->print_cr("not supported"); + OSContainer::print_container_metric(st, "active_processor_count", "not supported"); } supported = OSContainer::cpu_quota(i); - st->print("cpu_quota: "); if (supported && i > 0) { - st->print_cr("%d", i); + OSContainer::print_container_metric(st, "cpu_quota", i); } else { - st->print_cr("%s", !supported ? "not supported" : "no quota"); + OSContainer::print_container_metric(st, "cpu_quota", !supported ? "not supported" : "no quota"); } supported = OSContainer::cpu_period(i); - st->print("cpu_period: "); if (supported && i > 0) { - st->print_cr("%d", i); + OSContainer::print_container_metric(st, "cpu_period", i); } else { - st->print_cr("%s", !supported ? "not supported" : "no period"); + OSContainer::print_container_metric(st, "cpu_period", !supported ? "not supported" : "no period"); } supported = OSContainer::cpu_shares(i); - st->print("cpu_shares: "); if (supported && i > 0) { - st->print_cr("%d", i); + OSContainer::print_container_metric(st, "cpu_shares", i); } else { - st->print_cr("%s", !supported ? "not supported" : "no shares"); + OSContainer::print_container_metric(st, "cpu_shares", !supported ? "not supported" : "no shares"); } uint64_t j = 0; supported = OSContainer::cpu_usage_in_micros(j); - st->print("cpu_usage_in_micros: "); if (supported && j > 0) { - st->print_cr(UINT64_FORMAT, j); + OSContainer::print_container_metric(st, "cpu_usage", j, "us"); } else { - st->print_cr("%s", !supported ? "not supported" : "no usage"); + OSContainer::print_container_metric(st, "cpu_usage", !supported ? "not supported" : "no usage"); } MetricResult memory_limit; @@ -2530,31 +2525,29 @@ bool os::Linux::print_container_info(outputStream* st) { if (OSContainer::cache_usage_in_bytes(val)) { cache_usage.set_value(val); } - OSContainer::print_container_helper(st, memory_limit, "memory_limit_in_bytes"); - OSContainer::print_container_helper(st, mem_swap_limit, "memory_and_swap_limit_in_bytes"); - OSContainer::print_container_helper(st, mem_soft_limit, "memory_soft_limit_in_bytes"); - OSContainer::print_container_helper(st, mem_throttle_limit, "memory_throttle_limit_in_bytes"); - OSContainer::print_container_helper(st, mem_usage, "memory_usage_in_bytes"); - OSContainer::print_container_helper(st, mem_max_usage, "memory_max_usage_in_bytes"); - OSContainer::print_container_helper(st, rss_usage, "rss_usage_in_bytes"); - OSContainer::print_container_helper(st, cache_usage, "cache_usage_in_bytes"); + OSContainer::print_container_helper(st, memory_limit, "memory_limit"); + OSContainer::print_container_helper(st, mem_swap_limit, "memory_and_swap_limit"); + OSContainer::print_container_helper(st, mem_soft_limit, "memory_soft_limit"); + OSContainer::print_container_helper(st, mem_throttle_limit, "memory_throttle_limit"); + OSContainer::print_container_helper(st, mem_usage, "memory_usage"); + OSContainer::print_container_helper(st, mem_max_usage, "memory_max_usage"); + OSContainer::print_container_helper(st, rss_usage, "rss_usage"); + OSContainer::print_container_helper(st, cache_usage, "cache_usage"); OSContainer::print_version_specific_info(st); supported = OSContainer::pids_max(j); - st->print("maximum number of tasks: "); if (supported && j != value_unlimited) { - st->print_cr(UINT64_FORMAT, j); + OSContainer::print_container_metric(st, "maximum number of tasks", j); } else { - st->print_cr("%s", !supported ? "not supported" : "unlimited"); + OSContainer::print_container_metric(st, "maximum number of tasks", !supported ? "not supported" : "unlimited"); } supported = OSContainer::pids_current(j); - st->print("current number of tasks: "); if (supported && j > 0) { - st->print_cr(UINT64_FORMAT, j); + OSContainer::print_container_metric(st, "current number of tasks", j); } else { - st->print_cr("%s", !supported ? "not supported" : "no current tasks"); + OSContainer::print_container_metric(st, "current number of tasks", !supported ? "not supported" : "no tasks"); } return true; diff --git a/test/hotspot/jtreg/containers/docker/TestContainerInfo.java b/test/hotspot/jtreg/containers/docker/TestContainerInfo.java index a5579aa9528..5e7a0000cf5 100644 --- a/test/hotspot/jtreg/containers/docker/TestContainerInfo.java +++ b/test/hotspot/jtreg/containers/docker/TestContainerInfo.java @@ -73,23 +73,11 @@ public class TestContainerInfo { checkContainerInfo(out); } - private static void shouldMatchWithValue(OutputAnalyzer output, String match, String value) { - output.shouldContain(match); - String str = output.getOutput(); - for (String s : str.split(System.lineSeparator())) { - if (s.contains(match)) { - if (!s.contains(value)) { - throw new RuntimeException("memory_swap_current_in_bytes NOT " + value + "! Line was : " + s); - } - } - } - } - private static void checkContainerInfo(OutputAnalyzer out) throws Exception { String str = out.getOutput(); if (str.contains("cgroupv2")) { - shouldMatchWithValue(out, "memory_swap_max_limit_in_bytes", "0"); - shouldMatchWithValue(out, "memory_swap_current_in_bytes", "0"); + DockerTestUtils.shouldMatchWithValue(out, "memory_swap_max_limit", "0"); + DockerTestUtils.shouldMatchWithValue(out, "memory_swap_current", "0"); } else { throw new SkippedException("This test is cgroups v2 specific, skipped on cgroups v1"); } diff --git a/test/hotspot/jtreg/containers/docker/TestLimitsUpdating.java b/test/hotspot/jtreg/containers/docker/TestLimitsUpdating.java index df8ba5b6161..db8f4fd1826 100644 --- a/test/hotspot/jtreg/containers/docker/TestLimitsUpdating.java +++ b/test/hotspot/jtreg/containers/docker/TestLimitsUpdating.java @@ -128,12 +128,12 @@ public class TestLimitsUpdating { // Do assertions based on the output in target container OutputAnalyzer targetOut = out[0]; - targetOut.shouldContain("active_processor_count: 2"); // initial value - targetOut.shouldContain("active_processor_count: 3"); // updated value - targetOut.shouldContain("memory_limit_in_bytes: 512000 k"); // initial value - targetOut.shouldContain("memory_and_swap_limit_in_bytes: 512000 k"); // initial value - targetOut.shouldContain("memory_limit_in_bytes: 307200 k"); // updated value - targetOut.shouldContain("memory_and_swap_limit_in_bytes: 307200 k"); // updated value + DockerTestUtils.shouldMatchWithValue(targetOut, "active_processor_count", "2"); // initial value + DockerTestUtils.shouldMatchWithValue(targetOut, "active_processor_count", "3"); // updated value + DockerTestUtils.shouldMatchWithValue(targetOut, "memory_limit", "512000 kB"); // initial value + DockerTestUtils.shouldMatchWithValue(targetOut, "memory_and_swap_limit", "512000 kB"); // initial value + DockerTestUtils.shouldMatchWithValue(targetOut, "memory_limit", "307200 kB"); // updated value + DockerTestUtils.shouldMatchWithValue(targetOut, "memory_and_swap_limit", "307200 kB"); // updated value } private static List getContainerUpdate(int cpuQuota, int cpuPeriod, String memory) { diff --git a/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java b/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java index 4cf895156fe..6034aefcd6e 100644 --- a/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java +++ b/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java @@ -169,11 +169,11 @@ public class TestMemoryAwareness { opts.addDockerOpts("--memory-swap=" + swapToSet); Common.run(opts) - .shouldMatch("memory_limit_in_bytes:.*" + expectedMem) - .shouldNotMatch("memory_and_swap_limit_in_bytes:.*not supported") + .shouldMatch("memory_limit:.*" + expectedMem) + .shouldNotMatch("memory_and_swap_limit:.*not supported") // On systems with swapaccount=0 this returns the memory limit. // On systems with swapaccount=1 this returns the set memory+swap value. - .shouldMatch("memory_and_swap_limit_in_bytes:.*(" + expectedMem + "|" + expectedSwap + ")"); + .shouldMatch("memory_and_swap_limit:.*(" + expectedMem + "|" + expectedSwap + ")"); } /* diff --git a/test/hotspot/jtreg/containers/docker/TestMisc.java b/test/hotspot/jtreg/containers/docker/TestMisc.java index fca3cef5513..794bbc7b355 100644 --- a/test/hotspot/jtreg/containers/docker/TestMisc.java +++ b/test/hotspot/jtreg/containers/docker/TestMisc.java @@ -127,10 +127,10 @@ public class TestMisc { // mapping function. if (numberMatch) { int valueExpected = isCgroupV2 ? expected : cpuShares; - out.shouldContain("cpu_shares: " + valueExpected); + DockerTestUtils.shouldMatchWithValue(out, "cpu_shares", String.valueOf(valueExpected)); } else { // must not print "no shares" - out.shouldNotContain("cpu_shares: no shares"); + DockerTestUtils.shouldNotMatchWithValue(out, "cpu_shares", "no shares"); } } @@ -141,7 +141,7 @@ public class TestMisc { Common.addWhiteBoxOpts(opts); OutputAnalyzer out = Common.run(opts); - out.shouldContain("but overridden by -XX:ActiveProcessorCount 2"); + DockerTestUtils.shouldMatchWithValue(out, "active_processor_count", "2 (from -XX:ActiveProcessorCount)"); } private static void checkContainerInfo(OutputAnalyzer out) throws Exception { @@ -158,11 +158,11 @@ public class TestMisc { "Memory Throttle Limit", "Memory Usage", "Maximum Memory Usage", - "memory_max_usage_in_bytes", + "memory_max_usage", "maximum number of tasks", "current number of tasks", - "rss_usage_in_bytes", - "cache_usage_in_bytes" + "rss_usage", + "cache_usage" }; for (String s : expectedToContain) { @@ -170,13 +170,13 @@ public class TestMisc { } String str = out.getOutput(); if (str.contains("cgroupv1")) { - out.shouldContain("kernel_memory_usage_in_bytes"); - out.shouldContain("kernel_memory_max_usage_in_bytes"); - out.shouldContain("kernel_memory_limit_in_bytes"); + out.shouldContain("kernel_memory_usage"); + out.shouldContain("kernel_memory_max_usage"); + out.shouldContain("kernel_memory_limit"); } else { if (str.contains("cgroupv2")) { - out.shouldContain("memory_swap_current_in_bytes"); - out.shouldContain("memory_swap_max_limit_in_bytes"); + out.shouldContain("memory_swap_current"); + out.shouldContain("memory_swap_max_limit"); } else { throw new RuntimeException("Output has to contain information about cgroupv1 or cgroupv2"); } diff --git a/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java b/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java index ec3e6d773b1..22a8572af46 100644 --- a/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java +++ b/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; import java.util.List; +import java.util.regex.Pattern; import jdk.internal.platform.Metrics; import jdk.test.lib.Container; import jdk.test.lib.Utils; @@ -341,6 +342,16 @@ public class DockerTestUtils { return output; } + public static void shouldMatchWithValue(OutputAnalyzer output, String metric, String value) throws Exception { + String pattern = "^" + Pattern.quote(metric) + ":\\s*" + Pattern.quote(value) + ".*$"; + output.shouldMatch(pattern); + } + + public static void shouldNotMatchWithValue(OutputAnalyzer output, String metric, String value) throws Exception { + String pattern = "^" + Pattern.quote(metric) + ":\\s*" + Pattern.quote(value) + ".*$"; + output.shouldNotMatch(pattern); + } + private static void writeOutputToFile(String output, String fileName) throws Exception { try (FileWriter fw = new FileWriter(fileName)) {