On 01/02/2023 09:53, Teo Couprie Diaz wrote:
For a cgroup resource limitation, where a numerical value is expected "max" is a valid limitation. It means that the cgroup will not be limited _more_ than it already is. This can mean that it will use the kernel limit for the resource, if it exists, or the limit of a parent cgroup.
This patch reworks "read_session_pids_limit" to go up the cgroup hierarchy if it encounters a "max" value rather than a numerical one, using the kernel limit in the event where it doesn't find any. If it doesn't find either "max" or a numerical value, or errors, it will return -1.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
lib/tst_pid.c | 73 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index ee3b98771..baceb328f 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -64,29 +64,52 @@ static unsigned int get_session_uid(void) return getuid(); } -static int read_session_pids_limit(const char *path_fmt, int uid,
void (*cleanup_fn) (void))+static int read_session_pids_limit(const char *cgroup_mount,
const char *cgroup_path, void (*cleanup_fn) (void))
I feel that the name is still accurate, maybe even more so, but I could change it if needed. Would some comment explaining what it does be useful here ?
{
- int max_pids, ret;
- int ret, max_pids = -1; char max_pid_value[100];
- char path[PATH_MAX];
- char path[PATH_MAX + 1], file_path[PATH_MAX + 1];
- struct stat stat;
- ino_t cgroup_root_inode, current_dir_inode;
- ret = snprintf(path, sizeof(path), path_fmt, uid);
- ret = snprintf(path, sizeof(path), "%s%s", cgroup_mount, cgroup_path); if (ret < 0 || (size_t)ret >= sizeof(path)) return -1;
- if (access(path, R_OK) != 0) {
tst_resm(TINFO, "Cannot read session user limits from '%s'", path);return -1;- }
- SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value);
- if (!strcmp(max_pid_value, "max")) {
SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &max_pids);tst_resm(TINFO, "Found limit of processes %d (from %s=max)", max_pids, path);- } else {
max_pids = SAFE_STRTOL(max_pid_value, 0, INT_MAX);tst_resm(TINFO, "Found limit of processes %d (from %s)", max_pids, path);
- ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);
if (ret < 0 || (size_t)ret >= sizeof(path))return -1;- SAFE_STAT(cgroup_mount, &stat);
- cgroup_root_inode = stat.st_ino;
- SAFE_STAT(file_path, &stat);
- current_dir_inode = stat.st_ino;
Not really happy with the amount of duplicated code here, but it's necessary to handle the case where the pid mount is the root of the cgroup hierarchy, to prevent going up a level. Even though I guess I could make use of a do_while ? Very open to any suggestions on this as I don't like the structure very much.
- while (cgroup_root_inode != current_dir_inode) {
ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);if (ret < 0 || (size_t)ret >= sizeof(path))return -1;strncat(path, "/..", PATH_MAX);SAFE_STAT(path, &stat);current_dir_inode = stat.st_ino;if (access(file_path, R_OK) != 0) {tst_resm(TINFO, "Cannot read session user limits from '%s', going up the cgroup hierarchy hoping to find one.",
Regarding this and the one long message later, the checkpatch complains that it's too long, but it complains when I split the string literal on multiple lines. What would be the expected fix there ?
file_path);continue;}SAFE_FILE_SCANF(cleanup_fn, file_path, "%s", max_pid_value);if (strcmp(max_pid_value, "max")) {max_pids = SAFE_STRTOL(max_pid_value, 0, INT_MAX);tst_resm(TINFO, "Found limit of processes %d (from %s)", max_pids, file_path);return max_pids;} else if (max_pids < 0) {SAFE_FILE_SCANF(cleanup_fn, PID_MAX_PATH, "%d", &max_pids);tst_resm(TINFO, "Using kernel processes limit of %d (from %s=max) unless we find a value while going back up the cgroup hierarchy",max_pids, file_path);}return max_pids; @@ -120,11 +143,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void)) "%*s %*s %*s %*s %s %*s %*s - %*s cgroup %*[rw],pid%c", &path, &catchall);
- if (!cgroup_ok) {
strncat(path, cgroup_pids, PATH_MAX);strncat(path, "/pids.max", PATH_MAX);return read_session_pids_limit(path, uid, cleanup_fn);- }
- if (!cgroup_ok)
return read_session_pids_limit(path, cgroup_pids, cleanup_fn);/* Check for generic cgroup v2 pid.max */ cgroup_ok = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup", @@ -134,11 +154,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void)) "%*s %*s %*s %*s %s %*s %*s - cgroup2 %c", &path, &catchall);
- if (!cgroup_ok) {
strncat(path, cgroup_pids, PATH_MAX);strncat(path, "/pids.max", PATH_MAX);return read_session_pids_limit(path, uid, cleanup_fn);- }
- if (!cgroup_ok)
return read_session_pids_limit(path, cgroup_pids, cleanup_fn);return -1; }