On Wed, Feb 01, 2023 at 09:58:43AM +0000, Teo Couprie Diaz wrote:
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 ?
The name still reflects the purpose so that can stay, but indeed a short comment would be useful.
{
- 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.
See my reply to original message to this patch.
- 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 ?
User-visible strings should not be broken into multiple lines (as an exception to the 80 columns rule), as that makes it difficult to grep the sources for such strings. If it can be avoided than preferably the string literals should be kept to bare minimum.
---BR B.
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);}@@ -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);@@ -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);
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org