Hi Teo,
Within the subject: s/programatically/programmatically On Wed, Feb 01, 2023 at 09:53:23AM +0000, Teo Couprie Diaz wrote:
In our debian-based distribution, the two files used in lib/tst_pid.c
Could we switch 'our debian-based' to smth like: 'on some distributions' ?
are not available, but cgroups still imposes a task limit far smaller than the kernel pid_max. If the cgroup sysfs is mounted, we can use it to retrieve the current task limit imposed to the process. Implement the retrieval of this limit.
This can be done by first checking /proc/self/cgroup to get the cgroup the process is in, which will be a path under the cgroup sysfs. To get the path to the cgroup sysfs, check /proc/self/mountinfo. Finally, concatenate those two paths with pid.max to get the full path to the file containing the limit.
This fixes failures for msgstress04.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
Review branch as the changes are quite large: https://git.morello-project.org/Teo-CD/morello-linux-ltp/-/commits/review/te... lib/tst_pid.c | 55 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 11 deletions(-)
diff --git a/lib/tst_pid.c b/lib/tst_pid.c index 21cadef2a..ee3b98771 100644 --- a/lib/tst_pid.c +++ b/lib/tst_pid.c @@ -31,8 +31,6 @@ #include "tst_safe_macros.h" #define PID_MAX_PATH "/proc/sys/kernel/pid_max" -#define CGROUPS_V1_SLICE_FMT "/sys/fs/cgroup/pids/user.slice/user-%d.slice/pids.max" -#define CGROUPS_V2_SLICE_FMT "/sys/fs/cgroup/user.slice/user-%d.slice/pids.max" /* Leave some available processes for the OS */ #define PIDS_RESERVE 50 @@ -96,18 +94,53 @@ static int read_session_pids_limit(const char *path_fmt, int uid, static int get_session_pids_limit(void (*cleanup_fn) (void)) {
- int max_pids, uid;
- char path[PATH_MAX + 1];
- char cgroup_pids[PATH_MAX + 1];
- char catchall;
- int uid, cgroup_ok = 0;
The empty line should be preserved here.
uid = get_session_uid();
- max_pids = read_session_pids_limit(CGROUPS_V2_SLICE_FMT, uid, cleanup_fn);
- if (max_pids < 0)
max_pids = read_session_pids_limit(CGROUPS_V1_SLICE_FMT, uid,cleanup_fn);
- if (max_pids < 0)
return -1;
- /* Check for generic cgroup v1 pid.max */
- cgroup_ok = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d :pids: %s\n", &cgroup_pids);
cgroup_ok and it's use across the function seems bit counterintuitive (at least for me): the name suggest that expected value should be non-zero (true), whereas thie condition below relates succesful parsing the file with !cgroup_ok, so it gest bit misleading. Maybe renaming it to simply result/ret which does not imply (by it's name) what's the expected value ?
The format string does seem to contain an extra whitespace before ":pids:". sscanf does seem to not care about that too much and it does produce the valid result in the end. Assuming I got things right, one whitespace in a format string is supposed to match any combination of those within the input string, where any means none included. So it seems to be fine to have that one I guess.
Also cgroup_pids does not need the '&' (address-of) operator. I'd actually would expect a warning there complaining about type mismatch ... (same applies below)
- /*
* This is a bit of hack of scanf format strings. It only works forward* which means that if a variable has been matched, but not a later* part of the format it still counts as a match. Thus, we need a
Could we potentially rephrase that a bit saying that the search is being terminated once all convertion specifications are matched, disregarding outstanding literal characters ? The 'only works forward' statement is slightly inaccurate in this case.
* catch-all match later in the string that will _not_ be counted if* the static part of the format doesn't match.* This way, the macro will go on the next line rather than returning* on a line matching the %s but not the static part later.** Assume that the root of the mount is /. It can technically be anything,* but it should be / on any normal system.
Minor: Could we put quotes around '/' ?
--- BR B.
*/- if (!cgroup_ok)
cgroup_ok = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo","%*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);- }
- return max_pids;
- /* Check for generic cgroup v2 pid.max */
- cgroup_ok = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
"%*d :: %s\n", &cgroup_pids);- if (!cgroup_ok)
cgroup_ok = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo","%*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);- }
- return -1;
} int tst_get_free_pids_(void (*cleanup_fn) (void)) -- 2.25.1
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