From 4e5c355aa553f897bbbfe8364720027e893a06cb Mon Sep 17 00:00:00 2001 From: Jonathan Dowland Date: Thu, 19 Mar 2026 18:39:24 +0000 Subject: [PATCH 1/4] 8298730: Refactor subsystem_file_line_contents and add docs and tests Backport-of: 500c3c17379fe0a62d42ba31bdcdb584b1823f60 --- .../src/os/linux/vm/cgroupSubsystem_linux.hpp | 114 ++++++++++-------- .../os/linux/vm/cgroupV1Subsystem_linux.cpp | 13 +- hotspot/src/share/vm/utilities/ostream.cpp | 8 ++ hotspot/src/share/vm/utilities/ostream.hpp | 13 +- 4 files changed, 88 insertions(+), 60 deletions(-) diff --git a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp index 0cc0d5b6287..be944a9e23b 100644 --- a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp +++ b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp @@ -76,18 +76,17 @@ class CgroupController: public CHeapObj { PRAGMA_DIAG_PUSH PRAGMA_FORMAT_NONLITERAL_IGNORED +// Parses a subsystem's file, looking for a matching line. +// If key is null, then the first line will be matched with scan_fmt. +// If key isn't null, then each line will be matched, looking for something that matches "$key $scan_fmt". +// The matching value will be assigned to returnval. +// scan_fmt uses scanf() syntax. +// Return value: 0 on match, OSCONTAINER_ERROR on error. template int subsystem_file_line_contents(CgroupController* c, const char *filename, - const char *matchline, + const char *key, const char *scan_fmt, T returnval) { - FILE *fp = NULL; - char *p; - char file[MAXPATHLEN+1]; - char buf[MAXPATHLEN+1]; - char discard[MAXPATHLEN+1]; - bool found_match = false; - if (c == NULL) { if (PrintContainerInfo) { tty->print_cr("subsystem_file_line_contents: CgroupController* is NULL"); @@ -101,60 +100,73 @@ template int subsystem_file_line_contents(CgroupController* c, return OSCONTAINER_ERROR; } - strncpy(file, c->subsystem_path(), MAXPATHLEN); - file[MAXPATHLEN-1] = '\0'; - int filelen = strlen(file); - if ((filelen + strlen(filename)) > (MAXPATHLEN-1)) { + stringStream file_path; + file_path.print_raw(c->subsystem_path()); + file_path.print_raw(filename); + + if (file_path.size() > (MAXPATHLEN-1)) { if (PrintContainerInfo) { - tty->print_cr("File path too long %s, %s", file, filename); + tty->print_cr("File path too long %s, %s", file_path.base(), filename); } return OSCONTAINER_ERROR; } - strncat(file, filename, MAXPATHLEN-filelen); + const char* absolute_path = file_path.freeze(); if (PrintContainerInfo) { - tty->print_cr("Path to %s is %s", filename, file); + tty->print_cr("Path to %s is %s", filename, absolute_path); } - fp = fopen(file, "r"); - if (fp != NULL) { - int err = 0; - while ((p = fgets(buf, MAXPATHLEN, fp)) != NULL) { - found_match = false; - if (matchline == NULL) { - // single-line file case - int matched = sscanf(p, scan_fmt, returnval); - found_match = (matched == 1); - } else { - // multi-line file case - if (strstr(p, matchline) != NULL) { - // discard matchline string prefix - int matched = sscanf(p, scan_fmt, discard, returnval); - found_match = (matched == 2); - } else { - continue; // substring not found - } - } - if (found_match) { - fclose(fp); - return 0; - } else { - err = 1; - if (PrintContainerInfo) { - tty->print_cr("Type %s not found in file %s", scan_fmt, file); - } - } + + FILE* fp = fopen(absolute_path, "r"); + if (fp == NULL) { + if (PrintContainerInfo) { + tty->print_cr("Open of file %s failed, %s", absolute_path, strerror(errno)); } - if (err == 0) { - if (PrintContainerInfo) { - tty->print_cr("Empty file %s", file); - } + return OSCONTAINER_ERROR; + } + + const int buf_len = MAXPATHLEN+1; + char buf[buf_len]; + char* line = fgets(buf, buf_len, fp); + if (line == NULL) { + if (PrintContainerInfo) { + tty->print_cr("Empty file %s", absolute_path); } + fclose(fp); + return OSCONTAINER_ERROR; + } + + bool found_match = false; + if (key == NULL) { + // File consists of a single line according to caller, with only a value + int matched = sscanf(line, scan_fmt, returnval); + found_match = matched == 1; } else { - if (PrintContainerInfo) { - tty->print_cr("Open of file %s failed, %s", file, strerror(errno)); + // File consists of multiple lines in a "key value" + // fashion, we have to find the key. + const int key_len = strlen(key); + for (; line != NULL; line = fgets(buf, buf_len, fp)) { + char* key_substr = strstr(line, key); + char after_key = line[key_len]; + if (key_substr == line + && isspace(after_key) != 0 + && after_key != '\n') { + // Skip key, skip space + const char* value_substr = line + key_len + 1; + int matched = sscanf(value_substr, scan_fmt, returnval); + found_match = matched == 1; + if (found_match) { + break; + } + } } } - if (fp != NULL) - fclose(fp); + fclose(fp); + if (found_match) { + return 0; + } + if (PrintContainerInfo) { + tty->print_cr("Type %s (key == %s) not found in file %s", scan_fmt, + (key == NULL ? "null" : key), absolute_path); + } return OSCONTAINER_ERROR; } PRAGMA_DIAG_POP diff --git a/hotspot/src/os/linux/vm/cgroupV1Subsystem_linux.cpp b/hotspot/src/os/linux/vm/cgroupV1Subsystem_linux.cpp index b5c14173f6f..e7c23cf1d6b 100644 --- a/hotspot/src/os/linux/vm/cgroupV1Subsystem_linux.cpp +++ b/hotspot/src/os/linux/vm/cgroupV1Subsystem_linux.cpp @@ -111,10 +111,8 @@ jlong CgroupV1Subsystem::read_memory_limit_in_bytes() { } CgroupV1MemoryController* mem_controller = reinterpret_cast(_memory->controller()); if (mem_controller->is_hierarchical()) { - const char* matchline = "hierarchical_memory_limit"; - const char* format = "%s " JULONG_FORMAT; - GET_CONTAINER_INFO_LINE(julong, _memory->controller(), "/memory.stat", matchline, - "Hierarchical Memory Limit is: " JULONG_FORMAT, format, hier_memlimit) + GET_CONTAINER_INFO_LINE(julong, _memory->controller(), "/memory.stat", "hierarchical_memory_limit", + "Hierarchical Memory Limit is: " JULONG_FORMAT, JULONG_FORMAT, hier_memlimit) if (hier_memlimit >= os::Linux::physical_memory()) { if (PrintContainerInfo) { tty->print_cr("Hierarchical Memory Limit is: Unlimited"); @@ -142,15 +140,14 @@ jlong CgroupV1Subsystem::memory_and_swap_limit_in_bytes() { CgroupV1MemoryController* mem_controller = reinterpret_cast(_memory->controller()); if (mem_controller->is_hierarchical()) { const char* matchline = "hierarchical_memsw_limit"; - const char* format = "%s " JULONG_FORMAT; GET_CONTAINER_INFO_LINE(julong, _memory->controller(), "/memory.stat", matchline, - "Hierarchical Memory and Swap Limit is : " JULONG_FORMAT, format, hier_memlimit) - if (hier_memlimit >= host_total_memsw) { + "Hierarchical Memory and Swap Limit is : " JULONG_FORMAT, JULONG_FORMAT, hier_memswlimit) + if (hier_memswlimit >= host_total_memsw) { if (PrintContainerInfo) { tty->print_cr("Hierarchical Memory and Swap Limit is: Unlimited"); } } else { - return (jlong)hier_memlimit; + return (jlong)hier_memswlimit; } } return (jlong)-1; diff --git a/hotspot/src/share/vm/utilities/ostream.cpp b/hotspot/src/share/vm/utilities/ostream.cpp index 8b2f73e78ef..90f2d34fa2d 100644 --- a/hotspot/src/share/vm/utilities/ostream.cpp +++ b/hotspot/src/share/vm/utilities/ostream.cpp @@ -316,6 +316,7 @@ stringStream::stringStream(size_t initial_size) : outputStream() { buffer_pos = 0; buffer_fixed = false; DEBUG_ONLY(rm = Thread::current()->current_resource_mark();) + DEBUG_ONLY(_is_frozen = false); } // useful for output to fixed chunks of memory, such as performance counters @@ -324,11 +325,13 @@ stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) : outpu buffer = fixed_buffer; buffer_pos = 0; buffer_fixed = true; + DEBUG_ONLY(_is_frozen = false); } void stringStream::write(const char* s, size_t len) { size_t write_len = len; // number of non-null bytes to write size_t end = buffer_pos + len + 1; // position after write and final '\0' + assert(_is_frozen == false, "Modification forbidden"); if (end > buffer_length) { if (buffer_fixed) { // if buffer cannot resize, silently truncate @@ -371,6 +374,11 @@ char* stringStream::as_string() { return copy; } +void stringStream::reset() { + assert(_is_frozen == false, "Modification forbidden"); + buffer_pos = 0; _precount = 0; _position = 0; +} + stringStream::~stringStream() {} xmlStream* xtty; diff --git a/hotspot/src/share/vm/utilities/ostream.hpp b/hotspot/src/share/vm/utilities/ostream.hpp index fbcb2994dd9..3201fcb4836 100644 --- a/hotspot/src/share/vm/utilities/ostream.hpp +++ b/hotspot/src/share/vm/utilities/ostream.hpp @@ -27,6 +27,7 @@ #include "memory/allocation.hpp" #include "runtime/timer.hpp" +#include "utilities/macros.hpp" class GCId; DEBUG_ONLY(class ResourceMark;) @@ -177,6 +178,7 @@ class ttyUnlocker: StackObj { // for writing to strings; buffer will expand automatically class stringStream : public outputStream { protected: + DEBUG_ONLY(bool _is_frozen); char* buffer; size_t buffer_pos; size_t buffer_length; @@ -188,8 +190,17 @@ class stringStream : public outputStream { ~stringStream(); virtual void write(const char* c, size_t len); size_t size() { return buffer_pos; } + // Returns internal buffer containing the accumulated string. + // Returned buffer is only guaranteed to be valid as long as stream is not modified const char* base() { return buffer; } - void reset() { buffer_pos = 0; _precount = 0; _position = 0; } + // Freezes stringStream (no further modifications possible) and returns pointer to it. + // No-op if stream is frozen already. + // Returns the internal buffer containing the accumulated string. + const char* freeze() NOT_DEBUG(const) { + DEBUG_ONLY(_is_frozen = true); + return buffer; + }; + void reset(); char* as_string(); }; From cbbff8ce7263dbb66cc757c6701d365b8bfc67da Mon Sep 17 00:00:00 2001 From: Jonathan Dowland Date: Wed, 15 Apr 2026 09:11:17 +0100 Subject: [PATCH 2/4] Remove use of stringStream from cgroupSubsystem_linux Revert the file path handling to the prior method. Signed-off-by: Jonathan Dowland --- .../src/os/linux/vm/cgroupSubsystem_linux.hpp | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp index be944a9e23b..613b33eb6ea 100644 --- a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp +++ b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp @@ -100,25 +100,26 @@ template int subsystem_file_line_contents(CgroupController* c, return OSCONTAINER_ERROR; } - stringStream file_path; - file_path.print_raw(c->subsystem_path()); - file_path.print_raw(filename); + char file[MAXPATHLEN+1]; + strncpy(file, c->subsystem_path(), MAXPATHLEN); + file[MAXPATHLEN-1]='\0'; + int filelen = strlen(file); - if (file_path.size() > (MAXPATHLEN-1)) { + if ((filelen + strlen(filename)) > (MAXPATHLEN-1)) { if (PrintContainerInfo) { - tty->print_cr("File path too long %s, %s", file_path.base(), filename); + tty->print_cr("File path too long %s, %s", file, filename); } return OSCONTAINER_ERROR; } - const char* absolute_path = file_path.freeze(); + strncat(file, filename, MAXPATHLEN-filelen); if (PrintContainerInfo) { - tty->print_cr("Path to %s is %s", filename, absolute_path); + tty->print_cr("Path to %s is %s", filename, file); } - FILE* fp = fopen(absolute_path, "r"); + FILE* fp = fopen(file, "r"); if (fp == NULL) { if (PrintContainerInfo) { - tty->print_cr("Open of file %s failed, %s", absolute_path, strerror(errno)); + tty->print_cr("Open of file %s failed, %s", file, strerror(errno)); } return OSCONTAINER_ERROR; } @@ -128,7 +129,7 @@ template int subsystem_file_line_contents(CgroupController* c, char* line = fgets(buf, buf_len, fp); if (line == NULL) { if (PrintContainerInfo) { - tty->print_cr("Empty file %s", absolute_path); + tty->print_cr("Empty file %s", file); } fclose(fp); return OSCONTAINER_ERROR; @@ -165,7 +166,7 @@ template int subsystem_file_line_contents(CgroupController* c, } if (PrintContainerInfo) { tty->print_cr("Type %s (key == %s) not found in file %s", scan_fmt, - (key == NULL ? "null" : key), absolute_path); + (key == NULL ? "null" : key), file); } return OSCONTAINER_ERROR; } From f6879165922c86af21bd77c18089378c6a5831f0 Mon Sep 17 00:00:00 2001 From: Jonathan Dowland Date: Wed, 15 Apr 2026 09:24:12 +0100 Subject: [PATCH 3/4] Back out changes to ostream.{cpp,hpp} (stringStream) Since we are not calling stringStream from the cgroups code, don't apply the changes to it from 8225225 or 8313083. Signed-off-by: Jonathan Dowland --- hotspot/src/share/vm/utilities/ostream.cpp | 8 -------- hotspot/src/share/vm/utilities/ostream.hpp | 13 +------------ 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/hotspot/src/share/vm/utilities/ostream.cpp b/hotspot/src/share/vm/utilities/ostream.cpp index 90f2d34fa2d..8b2f73e78ef 100644 --- a/hotspot/src/share/vm/utilities/ostream.cpp +++ b/hotspot/src/share/vm/utilities/ostream.cpp @@ -316,7 +316,6 @@ stringStream::stringStream(size_t initial_size) : outputStream() { buffer_pos = 0; buffer_fixed = false; DEBUG_ONLY(rm = Thread::current()->current_resource_mark();) - DEBUG_ONLY(_is_frozen = false); } // useful for output to fixed chunks of memory, such as performance counters @@ -325,13 +324,11 @@ stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) : outpu buffer = fixed_buffer; buffer_pos = 0; buffer_fixed = true; - DEBUG_ONLY(_is_frozen = false); } void stringStream::write(const char* s, size_t len) { size_t write_len = len; // number of non-null bytes to write size_t end = buffer_pos + len + 1; // position after write and final '\0' - assert(_is_frozen == false, "Modification forbidden"); if (end > buffer_length) { if (buffer_fixed) { // if buffer cannot resize, silently truncate @@ -374,11 +371,6 @@ char* stringStream::as_string() { return copy; } -void stringStream::reset() { - assert(_is_frozen == false, "Modification forbidden"); - buffer_pos = 0; _precount = 0; _position = 0; -} - stringStream::~stringStream() {} xmlStream* xtty; diff --git a/hotspot/src/share/vm/utilities/ostream.hpp b/hotspot/src/share/vm/utilities/ostream.hpp index 3201fcb4836..fbcb2994dd9 100644 --- a/hotspot/src/share/vm/utilities/ostream.hpp +++ b/hotspot/src/share/vm/utilities/ostream.hpp @@ -27,7 +27,6 @@ #include "memory/allocation.hpp" #include "runtime/timer.hpp" -#include "utilities/macros.hpp" class GCId; DEBUG_ONLY(class ResourceMark;) @@ -178,7 +177,6 @@ class ttyUnlocker: StackObj { // for writing to strings; buffer will expand automatically class stringStream : public outputStream { protected: - DEBUG_ONLY(bool _is_frozen); char* buffer; size_t buffer_pos; size_t buffer_length; @@ -190,17 +188,8 @@ class stringStream : public outputStream { ~stringStream(); virtual void write(const char* c, size_t len); size_t size() { return buffer_pos; } - // Returns internal buffer containing the accumulated string. - // Returned buffer is only guaranteed to be valid as long as stream is not modified const char* base() { return buffer; } - // Freezes stringStream (no further modifications possible) and returns pointer to it. - // No-op if stream is frozen already. - // Returns the internal buffer containing the accumulated string. - const char* freeze() NOT_DEBUG(const) { - DEBUG_ONLY(_is_frozen = true); - return buffer; - }; - void reset(); + void reset() { buffer_pos = 0; _precount = 0; _position = 0; } char* as_string(); }; From bed4a28e3e4dcb011666dae72cdbe6edfeb5f3e1 Mon Sep 17 00:00:00 2001 From: Jonathan Dowland Date: Wed, 15 Apr 2026 11:14:39 +0100 Subject: [PATCH 4/4] Some style/placement adjustments to reduce diff delta Signed-off-by: Jonathan Dowland --- hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp index 613b33eb6ea..11f901f53dd 100644 --- a/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp +++ b/hotspot/src/os/linux/vm/cgroupSubsystem_linux.hpp @@ -87,6 +87,9 @@ template int subsystem_file_line_contents(CgroupController* c, const char *key, const char *scan_fmt, T returnval) { + FILE *fp = NULL; + char file[MAXPATHLEN+1]; + if (c == NULL) { if (PrintContainerInfo) { tty->print_cr("subsystem_file_line_contents: CgroupController* is NULL"); @@ -100,11 +103,9 @@ template int subsystem_file_line_contents(CgroupController* c, return OSCONTAINER_ERROR; } - char file[MAXPATHLEN+1]; strncpy(file, c->subsystem_path(), MAXPATHLEN); - file[MAXPATHLEN-1]='\0'; + file[MAXPATHLEN-1] = '\0'; int filelen = strlen(file); - if ((filelen + strlen(filename)) > (MAXPATHLEN-1)) { if (PrintContainerInfo) { tty->print_cr("File path too long %s, %s", file, filename); @@ -115,8 +116,7 @@ template int subsystem_file_line_contents(CgroupController* c, if (PrintContainerInfo) { tty->print_cr("Path to %s is %s", filename, file); } - - FILE* fp = fopen(file, "r"); + fp = fopen(file, "r"); if (fp == NULL) { if (PrintContainerInfo) { tty->print_cr("Open of file %s failed, %s", file, strerror(errno));