[LTP] [PATCH 0/3] lib/tst_pid: Find session PID limit more reliably

[LTP] [PATCH 0/3] lib/tst_pid: Find session PID limit more reliably

[Teo Couprie Diaz]

Hello LTP maintainers,

This patch series heavily reworks the logic to find the session PID limit,
which makes it work on a lot more cases than covered by the current paths.

This was motivated by failures I observed with msgstress03 and msgstress04
which weren't able to retrieve the correct PID limit on a minimal debian image
created with debootstrab.
I believe this could help with other failures I found online[0][1][2].


Indeed, the current paths used to find the session pids.max relies on systemd
to set up cgroups for the user sesssions.
This is not always the case: even if systemd is running, there might not be a
systemd user instance. cgroups might be enabled without the use of systemd, or
manually, which wouldn't match with the current paths. The process might be
controlled by another service's cgroup (for example, the sshd PID limits).

This is addressed by the first patch which uses the information in /proc/self/
to find both the cgroup sysfs mount point and the path to the active cgroup.

Further, currently when "max" is encountered in "pids.max" LTP assumes this
means the kernel "pid_max". This is fine if the cgroup is the only one
imposing a pid limit in its hierarchy, but is invalid if there's any parent
cgroup that restricts the PID count, "max" meaning this restricted value.

The second patch addresses this by going up the cgroup hierarchy until a limit
other than "max" is found, falling back to the kernel "pid_max" if it doesn't.


With our system this wasn't enough to fix the msgstress03 failures as it used
more PIDs than the limit it retrieved. Comparing it to msgstress04, msgstress03
is quite agressive in its PID limit and seems to assume tasks will complete
fast enough to not hit the limit.
Indeed, it will fork nprocs times, which is set to the pid limit if the later
is smaller, but then each forked process will fork again.
Fix that by pessimistically assuming that every fork will be alive at the same
time.


This series has been tested with cgroup v1 and v2, on Alpine Linux, Arch Linux
and Debian.

Thanks very much in advance for your time !

CI build:
https://github.com/Teo-CD/ltp/actions/runs/4184905549

[0]: https://lore.kernel.org/ltp/Yep6yovcY2qJkXSR@yuki/T/#t
[1]: https://forums.sifive.com/t/ltp-message-queue-stress-testcase-failure/4601
[2]: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1783881

Teo Couprie Diaz (3):
  lib/tst_pid: Find cgroup pid.max programmatically
  lib/tst_pid: Go to parent cgroups for max value
  ipc/msgstress03: Assume all forks will run concurently

 lib/tst_pid.c                                 | 133 ++++++++++++------
 .../syscalls/ipc/msgstress/msgstress03.c      |   2 +-
 2 files changed, 90 insertions(+), 45 deletions(-)

-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] lib/tst_pid: Find cgroup pid.max programmatically

[Teo Couprie Diaz]

In some distributions, the two files used in lib/tst_pid.c 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 patch changes the way read_session_pids_limit is called, not passing
a format string to be completed anymore, but is still used the same way.
A following patch will update this function.

This fixes failures for msgstress04.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
---
 lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/lib/tst_pid.c b/lib/tst_pid.c
index 5595e79bd..3d0be6dcd 100644
--- a/lib/tst_pid.c
+++ b/lib/tst_pid.c
@@ -32,8 +32,6 @@
 
 #define PID_MAX_PATH "/proc/sys/kernel/pid_max"
 #define THREADS_MAX_PATH "/proc/sys/kernel/threads-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
 
@@ -97,18 +95,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, ret = 0;
 
 	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);
+	/* Check for generic cgroup v1 pid.max */
+	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
+						   "%*d:pids:%s\n", cgroup_pids);
+	/*
+	 * This is a bit of a hack of scanf format strings. Indeed, if all
+	 * conversion specifications have been matched the return of scanf will be
+	 * the same whether any outstanding literal characters match or not.
+	 * As we want to match the literal part, we can add a catchall after it
+	 * so that it won't be counted if the literal part doesn't match.
+	 * This makes the macro go to the next line until the catchall, thus
+	 * the literal parts, is matched.
+	 *
+	 * Assume that the root of the mount is '/'. It can be anything,
+	 * but it should be '/' on any normal system.
+	 */
+	if (!ret)
+		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
+							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
+							   path, &catchall);
+
+	if (!ret) {
+		strncat(path, cgroup_pids, PATH_MAX);
+		strncat(path, "/pids.max", PATH_MAX);
+		return read_session_pids_limit(path, uid, cleanup_fn);
+	}
 
-	if (max_pids < 0)
-		return -1;
+	/* Check for generic cgroup v2 pid.max */
+	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
+						   "%*d::%s\n", cgroup_pids);
+	if (!ret)
+		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
+							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
+							   path, &catchall);
+
+	if (!ret) {
+		strncat(path, cgroup_pids, PATH_MAX);
+		strncat(path, "/pids.max", PATH_MAX);
+		return read_session_pids_limit(path, uid, cleanup_fn);
+	}
 
-	return max_pids;
+	return -1;
 }
 
 int tst_get_free_pids_(void (*cleanup_fn) (void))
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] lib/tst_pid: Go to parent cgroups for max value

[Teo Couprie Diaz]

A cgroup resource limitation can be either a number or "max".
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.

Clean up uid related code as it is not used anymore.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
Co-developed-by: Beata Michalska <beata.michalska@arm.com>
Signed-off-by: Beata Michalska <beata.michalska@arm.com>
---
 lib/tst_pid.c | 96 +++++++++++++++++++++++++++++----------------------
 1 file changed, 54 insertions(+), 42 deletions(-)

diff --git a/lib/tst_pid.c b/lib/tst_pid.c
index 3d0be6dcd..ad1893290 100644
--- a/lib/tst_pid.c
+++ b/lib/tst_pid.c
@@ -44,50 +44,69 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
 	return pid;
 }
 
-/*
- * Get the effective session UID - either one invoking current test via sudo
- * or the real UID.
- */
-static unsigned int get_session_uid(void)
+static int __read_pids_limit(const char *path, void (*cleanup_fn) (void))
 {
-	const char *sudo_uid;
+	char max_pids_value[100];
+	int max_pids;
 
-	sudo_uid = getenv("SUDO_UID");
-	if (sudo_uid) {
-		unsigned int real_uid;
-		int ret;
+	if (access(path, R_OK) != 0) {
+		tst_resm(TINFO, "Cannot read session user limits from '%s'", path);
+		return -1;
+	}
 
-		ret = sscanf(sudo_uid, "%u", &real_uid);
-		if (ret == 1)
-			return real_uid;
+	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pids_value);
+	if (strcmp(max_pids_value, "max")) {
+		max_pids =  SAFE_STRTOL(max_pids_value, 0, INT_MAX);
+		tst_resm(TINFO, "Found limit of processes %d (from %s)",
+				max_pids, path);
+	} else {
+		max_pids = -1;
 	}
 
-	return getuid();
+	return max_pids;
 }
 
-static int read_session_pids_limit(const char *path_fmt, int uid,
-				   void (*cleanup_fn) (void))
+/*
+ * Take the path to the cgroup mount and to the current cgroup pid controller
+ * and try to find the PID limit imposed by cgroup.
+ * Go up the cgroup hierarchy if needed, otherwise use the kernel PID limit.
+ */
+static int read_session_pids_limit(const char *cgroup_mount,
+				   const char *cgroup_path, void (*cleanup_fn) (void))
 {
-	int max_pids, ret;
-	char max_pid_value[100];
-	char path[PATH_MAX];
-
-	ret = snprintf(path, sizeof(path), path_fmt, uid);
+	int ret, cgroup_depth = 0, max_pids = -1;
+	char path[PATH_MAX + 1], file_path[PATH_MAX + 1];
+	const char *sub_path = cgroup_path;
+
+	/* Find the number of groups we can go up. */
+	do {
+		cgroup_depth += 1;
+		sub_path++;
+		sub_path = strchr(sub_path, '/');
+	} while (sub_path);
+
+	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;
+	for (int i = 0 ; i < cgroup_depth ; i++) {
+		/* Create a path to read from. */
+		ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);
+		if (ret < 0 || (size_t)ret >= sizeof(file_path))
+			return -1;
+
+		max_pids = __read_pids_limit(file_path, cleanup_fn);
+		if (max_pids >= 0)
+			return max_pids;
+
+		strncat(path, "/..", PATH_MAX);
 	}
 
-	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value);
-	if (!strcmp(max_pid_value, "max")) {
+	if (max_pids < 0) {
+		/* Read kernel imposed limits */
 		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);
+		tst_resm(TINFO, "Using kernel processes limit of %d",
+			 max_pids);
 	}
 
 	return max_pids;
@@ -98,9 +117,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
 	char path[PATH_MAX + 1];
 	char cgroup_pids[PATH_MAX + 1];
 	char catchall;
-	int uid, ret = 0;
+	int ret = 0;
 
-	uid = get_session_uid();
 	/* Check for generic cgroup v1 pid.max */
 	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
 						   "%*d:pids:%s\n", cgroup_pids);
@@ -121,11 +139,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
 							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
 							   path, &catchall);
 
-	if (!ret) {
-		strncat(path, cgroup_pids, PATH_MAX);
-		strncat(path, "/pids.max", PATH_MAX);
-		return read_session_pids_limit(path, uid, cleanup_fn);
-	}
+	if (!ret)
+		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
 
 	/* Check for generic cgroup v2 pid.max */
 	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
@@ -135,11 +150,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
 							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
 							   path, &catchall);
 
-	if (!ret) {
-		strncat(path, cgroup_pids, PATH_MAX);
-		strncat(path, "/pids.max", PATH_MAX);
-		return read_session_pids_limit(path, uid, cleanup_fn);
-	}
+	if (!ret)
+		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
 
 	return -1;
 }
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] ipc/msgstress03: Assume all forks will run concurently

[Teo Couprie Diaz]

It appears that msgstress03 doesn't account for all PIDs that its children
can use, as it expects the tasks will terminate quickly and not reach
the PID limit.
On some systems, this assumption can be invalid and the PID limit
will be hit.
Change the limit to account for all possible children at once, knowning
that each child will fork as well.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
---
 testcases/kernel/syscalls/ipc/msgstress/msgstress03.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
index 3cb70ab18..0c46927b8 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
@@ -109,7 +109,7 @@ int main(int argc, char **argv)
 		}
 	}
 
-	free_pids = tst_get_free_pids(cleanup);
+	free_pids = tst_get_free_pids(cleanup) / 2;
 	if (nprocs >= free_pids) {
 		tst_resm(TINFO,
 			 "Requested number of processes higher than limit (%d > %d), "
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] lib/tst_pid: Find cgroup pid.max programmatically

[Richard Palethorpe]

Hello,

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> In some distributions, the two files used in lib/tst_pid.c 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 patch changes the way read_session_pids_limit is called, not passing
> a format string to be completed anymore, but is still used the same way.
> A following patch will update this function.
>
> This fixes failures for msgstress04.
>
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
> ---
>  lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/lib/tst_pid.c b/lib/tst_pid.c
> index 5595e79bd..3d0be6dcd 100644
> --- a/lib/tst_pid.c
> +++ b/lib/tst_pid.c
> @@ -32,8 +32,6 @@
>  
>  #define PID_MAX_PATH "/proc/sys/kernel/pid_max"
>  #define THREADS_MAX_PATH "/proc/sys/kernel/threads-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
>  
> @@ -97,18 +95,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, ret = 0;
>  
>  	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);
> +	/* Check for generic cgroup v1 pid.max */
> +	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
> +						   "%*d:pids:%s\n", cgroup_pids);
> +	/*
> +	 * This is a bit of a hack of scanf format strings. Indeed, if all
> +	 * conversion specifications have been matched the return of scanf will be
> +	 * the same whether any outstanding literal characters match or not.
> +	 * As we want to match the literal part, we can add a catchall after it
> +	 * so that it won't be counted if the literal part doesn't match.
> +	 * This makes the macro go to the next line until the catchall, thus
> +	 * the literal parts, is matched.
> +	 *
> +	 * Assume that the root of the mount is '/'. It can be anything,
> +	 * but it should be '/' on any normal system.
> +	 */
> +	if (!ret)
> +		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
> +							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
> +							   path,
>  	&catchall);

Uhhff, I already implemented this logic in tst_cg_scan in tst_cgroup. In
there we scan the current CGroup hierarchy and build a data structure
which represents it.

I guess you are not aware of tst_cgroup?

> +
> +	if (!ret) {
> +		strncat(path, cgroup_pids, PATH_MAX);
> +		strncat(path, "/pids.max", PATH_MAX);
> +		return read_session_pids_limit(path, uid, cleanup_fn);
> +	}
>  
> -	if (max_pids < 0)
> -		return -1;
> +	/* Check for generic cgroup v2 pid.max */
> +	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
> +						   "%*d::%s\n",
> cgroup_pids);

This has not been added to tst_cgroup because usually tests that need a
cgroup feature are moved to a cgroup created by LTP. We also check that
any required CGroups are available and mount them if necessary.

I suppose in this case we do not care if there is no CGroup hierarchy or
the pids controller is absent?

In any case I think tst_cgroup should be used or extended.

> +	if (!ret)
> +		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
> +							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
> +							   path, &catchall);
> +
> +	if (!ret) {
> +		strncat(path, cgroup_pids, PATH_MAX);
> +		strncat(path, "/pids.max", PATH_MAX);
> +		return read_session_pids_limit(path, uid, cleanup_fn);
> +	}
>  
> -	return max_pids;
> +	return -1;
>  }
>  
>  int tst_get_free_pids_(void (*cleanup_fn) (void))
> -- 
> 2.25.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib/tst_pid: Go to parent cgroups for max value

[Richard Palethorpe]

Hello,

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> A cgroup resource limitation can be either a number or "max".

IIRC this is not true on V1, at least not historically. On some tests we
have to simulate "max" when V1 is detected.

Perhaps for pids.max specifically it is the case though?

> 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.
>
> Clean up uid related code as it is not used anymore.
>
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
> Co-developed-by: Beata Michalska <beata.michalska@arm.com>
> Signed-off-by: Beata Michalska <beata.michalska@arm.com>
> ---
>  lib/tst_pid.c | 96 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 54 insertions(+), 42 deletions(-)
>
> diff --git a/lib/tst_pid.c b/lib/tst_pid.c
> index 3d0be6dcd..ad1893290 100644
> --- a/lib/tst_pid.c
> +++ b/lib/tst_pid.c
> @@ -44,50 +44,69 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
>  	return pid;
>  }
>  
> -/*
> - * Get the effective session UID - either one invoking current test via sudo
> - * or the real UID.
> - */
> -static unsigned int get_session_uid(void)
> +static int __read_pids_limit(const char *path, void (*cleanup_fn) (void))
>  {
> -	const char *sudo_uid;
> +	char max_pids_value[100];
> +	int max_pids;
>  
> -	sudo_uid = getenv("SUDO_UID");
> -	if (sudo_uid) {
> -		unsigned int real_uid;
> -		int ret;
> +	if (access(path, R_OK) != 0) {
> +		tst_resm(TINFO, "Cannot read session user limits from '%s'", path);
> +		return -1;
> +	}
>  
> -		ret = sscanf(sudo_uid, "%u", &real_uid);
> -		if (ret == 1)
> -			return real_uid;
> +	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pids_value);
> +	if (strcmp(max_pids_value, "max")) {
> +		max_pids =  SAFE_STRTOL(max_pids_value, 0, INT_MAX);
> +		tst_resm(TINFO, "Found limit of processes %d (from %s)",
> +				max_pids, path);
> +	} else {
> +		max_pids = -1;
>  	}
>  
> -	return getuid();
> +	return max_pids;
>  }
>  
> -static int read_session_pids_limit(const char *path_fmt, int uid,
> -				   void (*cleanup_fn) (void))
> +/*
> + * Take the path to the cgroup mount and to the current cgroup pid controller
> + * and try to find the PID limit imposed by cgroup.
> + * Go up the cgroup hierarchy if needed, otherwise use the kernel PID limit.
> + */
> +static int read_session_pids_limit(const char *cgroup_mount,
> +				   const char *cgroup_path, void (*cleanup_fn) (void))
>  {
> -	int max_pids, ret;
> -	char max_pid_value[100];
> -	char path[PATH_MAX];
> -
> -	ret = snprintf(path, sizeof(path), path_fmt, uid);
> +	int ret, cgroup_depth = 0, max_pids = -1;
> +	char path[PATH_MAX + 1], file_path[PATH_MAX + 1];
> +	const char *sub_path = cgroup_path;
> +
> +	/* Find the number of groups we can go up. */
> +	do {
> +		cgroup_depth += 1;
> +		sub_path++;
> +		sub_path = strchr(sub_path, '/');
> +	} while (sub_path);
> +
> +	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;
> +	for (int i = 0 ; i < cgroup_depth ; i++) {
> +		/* Create a path to read from. */
> +		ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);
> +		if (ret < 0 || (size_t)ret >= sizeof(file_path))
> +			return -1;
> +
> +		max_pids = __read_pids_limit(file_path, cleanup_fn);
> +		if (max_pids >= 0)
> +			return max_pids;
> +
> +		strncat(path, "/..", PATH_MAX);
>  	}
>  
> -	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value);
> -	if (!strcmp(max_pid_value, "max")) {
> +	if (max_pids < 0) {
> +		/* Read kernel imposed limits */
>  		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);
> +		tst_resm(TINFO, "Using kernel processes limit of %d",
> +			 max_pids);
>  	}
>  
>  	return max_pids;
> @@ -98,9 +117,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>  	char path[PATH_MAX + 1];
>  	char cgroup_pids[PATH_MAX + 1];
>  	char catchall;
> -	int uid, ret = 0;
> +	int ret = 0;
>  
> -	uid = get_session_uid();
>  	/* Check for generic cgroup v1 pid.max */
>  	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>  						   "%*d:pids:%s\n", cgroup_pids);
> @@ -121,11 +139,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>  							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
>  							   path, &catchall);
>  
> -	if (!ret) {
> -		strncat(path, cgroup_pids, PATH_MAX);
> -		strncat(path, "/pids.max", PATH_MAX);
> -		return read_session_pids_limit(path, uid, cleanup_fn);
> -	}
> +	if (!ret)
> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>  
>  	/* Check for generic cgroup v2 pid.max */
>  	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
> @@ -135,11 +150,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>  							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
>  							   path, &catchall);
>  
> -	if (!ret) {
> -		strncat(path, cgroup_pids, PATH_MAX);
> -		strncat(path, "/pids.max", PATH_MAX);
> -		return read_session_pids_limit(path, uid, cleanup_fn);
> -	}
> +	if (!ret)
> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>  
>  	return -1;
>  }
> -- 
> 2.25.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] ipc/msgstress03: Assume all forks will run concurently

[Richard Palethorpe]

Hello,

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> It appears that msgstress03 doesn't account for all PIDs that its children
> can use, as it expects the tasks will terminate quickly and not reach
> the PID limit.
> On some systems, this assumption can be invalid and the PID limit
> will be hit.
> Change the limit to account for all possible children at once, knowning
> that each child will fork as well.

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
> ---
>  testcases/kernel/syscalls/ipc/msgstress/msgstress03.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
> index 3cb70ab18..0c46927b8 100644
> --- a/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
> +++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress03.c
> @@ -109,7 +109,7 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> -	free_pids = tst_get_free_pids(cleanup);
> +	free_pids = tst_get_free_pids(cleanup) / 2;
>  	if (nprocs >= free_pids) {
>  		tst_resm(TINFO,
>  			 "Requested number of processes higher than limit (%d > %d), "
> -- 
> 2.25.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] lib/tst_pid: Find cgroup pid.max programmatically

[Teo Couprie Diaz]

Hello Richard,
Thanks for taking the time to have a look !

On 28/02/2023 13:50, Richard Palethorpe wrote:
> Hello,
>
> Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:
>
>> In some distributions, the two files used in lib/tst_pid.c 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 patch changes the way read_session_pids_limit is called, not passing
>> a format string to be completed anymore, but is still used the same way.
>> A following patch will update this function.
>>
>> This fixes failures for msgstress04.
>>
>> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
>> ---
>>   lib/tst_pid.c | 53 +++++++++++++++++++++++++++++++++++++++++----------
>>   1 file changed, 43 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/tst_pid.c b/lib/tst_pid.c
>> index 5595e79bd..3d0be6dcd 100644
>> --- a/lib/tst_pid.c
>> +++ b/lib/tst_pid.c
>> @@ -32,8 +32,6 @@
>>   
>>   #define PID_MAX_PATH "/proc/sys/kernel/pid_max"
>>   #define THREADS_MAX_PATH "/proc/sys/kernel/threads-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
>>   
>> @@ -97,18 +95,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, ret = 0;
>>   
>>   	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);
>> +	/* Check for generic cgroup v1 pid.max */
>> +	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>> +						   "%*d:pids:%s\n", cgroup_pids);
>> +	/*
>> +	 * This is a bit of a hack of scanf format strings. Indeed, if all
>> +	 * conversion specifications have been matched the return of scanf will be
>> +	 * the same whether any outstanding literal characters match or not.
>> +	 * As we want to match the literal part, we can add a catchall after it
>> +	 * so that it won't be counted if the literal part doesn't match.
>> +	 * This makes the macro go to the next line until the catchall, thus
>> +	 * the literal parts, is matched.
>> +	 *
>> +	 * Assume that the root of the mount is '/'. It can be anything,
>> +	 * but it should be '/' on any normal system.
>> +	 */
>> +	if (!ret)
>> +		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
>> +							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
>> +							   path,
>>   	&catchall);
> Uhhff, I already implemented this logic in tst_cg_scan in tst_cgroup. In
> there we scan the current CGroup hierarchy and build a data structure
> which represents it.
>
> I guess you are not aware of tst_cgroup?
Indeed, it appears that I either missed it or misunderstood it while 
researching. That's on me, it is indeed already covered there.
>
>> +
>> +	if (!ret) {
>> +		strncat(path, cgroup_pids, PATH_MAX);
>> +		strncat(path, "/pids.max", PATH_MAX);
>> +		return read_session_pids_limit(path, uid, cleanup_fn);
>> +	}
>>   
>> -	if (max_pids < 0)
>> -		return -1;
>> +	/* Check for generic cgroup v2 pid.max */
>> +	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>> +						   "%*d::%s\n",
>> cgroup_pids);
> This has not been added to tst_cgroup because usually tests that need a
> cgroup feature are moved to a cgroup created by LTP. We also check that
> any required CGroups are available and mount them if necessary.
Interesting, that does make sense when testing cgroups and would make it 
much easier.
As I mentioned in the cover letter this patch series was motivated by 
msgstress03 and msgstress04 which do not set up a cgroup, they just try 
to read their current limit. This is why I went to the trouble of 
looking for the cgroup directly.
>
> I suppose in this case we do not care if there is no CGroup hierarchy or
> the pids controller is absent?
That is the case indeed : if there's no cgroup hierarchy or pid 
controller, it would use the kernel PID limit directly instead.
This cannot be done by default because if there is indeed a cgroup 
hierarchy and pid controller, the forks will probably fail far before 
the kernel pid limit is hit.
>
> In any case I think tst_cgroup should be used or extended.
I agree, hopefully I can find some time to rework this series to make 
use of tst_cgroup as much as possible.
My apologies for the noise and thanks again for the review.

Best regards,
Téo
>
>> +	if (!ret)
>> +		ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/mountinfo",
>> +							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
>> +							   path, &catchall);
>> +
>> +	if (!ret) {
>> +		strncat(path, cgroup_pids, PATH_MAX);
>> +		strncat(path, "/pids.max", PATH_MAX);
>> +		return read_session_pids_limit(path, uid, cleanup_fn);
>> +	}
>>   
>> -	return max_pids;
>> +	return -1;
>>   }
>>   
>>   int tst_get_free_pids_(void (*cleanup_fn) (void))
>> -- 
>> 2.25.1
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib/tst_pid: Go to parent cgroups for max value

[Teo Couprie Diaz]

Hello,

On 28/02/2023 14:07, Richard Palethorpe wrote:
> Hello,
>
> Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:
>
>> A cgroup resource limitation can be either a number or "max".
> IIRC this is not true on V1, at least not historically. On some tests we
> have to simulate "max" when V1 is detected.
>
> Perhaps for pids.max specifically it is the case though?
You would have more experience than me on that front ! However, from my 
understanding, for `pids.max` specifically it should be correct.
The kernel documentation[0] of the cgroup-v1 pid controller states :

 > To set a cgroup to have no limit, set pids.max to “max”. This is the 
default for all new cgroups [...]

And from my testing I was not able to generate a cgroup *without* 
pids.max: it was always present and set to "max".

However, there is no `pids.max` for the root cgroup at all; it cannot be 
enabled. Thus it could make sense that if the process is in the root 
cgroup you had to simulate "max", because there wouldn't be `pids.max` 
to read.


But as discussed on the first patch I will need to come back to this 
series anyway and rework it to make use of tst_cgroup, and extend _that_ 
if needed, rather than slap some more functionality on top of tst_pid.
If I do so and still need to add a patch similar to this one I will keep 
in mind that this might be only true for the pid controller, for cgroups 
other than the root cgroup.

Thanks again for the review,
Best regards
Téo

[0]: 
https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v1/pids.html#usage

>
>> 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.
>>
>> Clean up uid related code as it is not used anymore.
>>
>> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
>> Co-developed-by: Beata Michalska <beata.michalska@arm.com>
>> Signed-off-by: Beata Michalska <beata.michalska@arm.com>
>> ---
>>   lib/tst_pid.c | 96 +++++++++++++++++++++++++++++----------------------
>>   1 file changed, 54 insertions(+), 42 deletions(-)
>>
>> diff --git a/lib/tst_pid.c b/lib/tst_pid.c
>> index 3d0be6dcd..ad1893290 100644
>> --- a/lib/tst_pid.c
>> +++ b/lib/tst_pid.c
>> @@ -44,50 +44,69 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
>>   	return pid;
>>   }
>>   
>> -/*
>> - * Get the effective session UID - either one invoking current test via sudo
>> - * or the real UID.
>> - */
>> -static unsigned int get_session_uid(void)
>> +static int __read_pids_limit(const char *path, void (*cleanup_fn) (void))
>>   {
>> -	const char *sudo_uid;
>> +	char max_pids_value[100];
>> +	int max_pids;
>>   
>> -	sudo_uid = getenv("SUDO_UID");
>> -	if (sudo_uid) {
>> -		unsigned int real_uid;
>> -		int ret;
>> +	if (access(path, R_OK) != 0) {
>> +		tst_resm(TINFO, "Cannot read session user limits from '%s'", path);
>> +		return -1;
>> +	}
>>   
>> -		ret = sscanf(sudo_uid, "%u", &real_uid);
>> -		if (ret == 1)
>> -			return real_uid;
>> +	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pids_value);
>> +	if (strcmp(max_pids_value, "max")) {
>> +		max_pids =  SAFE_STRTOL(max_pids_value, 0, INT_MAX);
>> +		tst_resm(TINFO, "Found limit of processes %d (from %s)",
>> +				max_pids, path);
>> +	} else {
>> +		max_pids = -1;
>>   	}
>>   
>> -	return getuid();
>> +	return max_pids;
>>   }
>>   
>> -static int read_session_pids_limit(const char *path_fmt, int uid,
>> -				   void (*cleanup_fn) (void))
>> +/*
>> + * Take the path to the cgroup mount and to the current cgroup pid controller
>> + * and try to find the PID limit imposed by cgroup.
>> + * Go up the cgroup hierarchy if needed, otherwise use the kernel PID limit.
>> + */
>> +static int read_session_pids_limit(const char *cgroup_mount,
>> +				   const char *cgroup_path, void (*cleanup_fn) (void))
>>   {
>> -	int max_pids, ret;
>> -	char max_pid_value[100];
>> -	char path[PATH_MAX];
>> -
>> -	ret = snprintf(path, sizeof(path), path_fmt, uid);
>> +	int ret, cgroup_depth = 0, max_pids = -1;
>> +	char path[PATH_MAX + 1], file_path[PATH_MAX + 1];
>> +	const char *sub_path = cgroup_path;
>> +
>> +	/* Find the number of groups we can go up. */
>> +	do {
>> +		cgroup_depth += 1;
>> +		sub_path++;
>> +		sub_path = strchr(sub_path, '/');
>> +	} while (sub_path);
>> +
>> +	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;
>> +	for (int i = 0 ; i < cgroup_depth ; i++) {
>> +		/* Create a path to read from. */
>> +		ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);
>> +		if (ret < 0 || (size_t)ret >= sizeof(file_path))
>> +			return -1;
>> +
>> +		max_pids = __read_pids_limit(file_path, cleanup_fn);
>> +		if (max_pids >= 0)
>> +			return max_pids;
>> +
>> +		strncat(path, "/..", PATH_MAX);
>>   	}
>>   
>> -	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value);
>> -	if (!strcmp(max_pid_value, "max")) {
>> +	if (max_pids < 0) {
>> +		/* Read kernel imposed limits */
>>   		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);
>> +		tst_resm(TINFO, "Using kernel processes limit of %d",
>> +			 max_pids);
>>   	}
>>   
>>   	return max_pids;
>> @@ -98,9 +117,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>   	char path[PATH_MAX + 1];
>>   	char cgroup_pids[PATH_MAX + 1];
>>   	char catchall;
>> -	int uid, ret = 0;
>> +	int ret = 0;
>>   
>> -	uid = get_session_uid();
>>   	/* Check for generic cgroup v1 pid.max */
>>   	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>>   						   "%*d:pids:%s\n", cgroup_pids);
>> @@ -121,11 +139,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>   							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
>>   							   path, &catchall);
>>   
>> -	if (!ret) {
>> -		strncat(path, cgroup_pids, PATH_MAX);
>> -		strncat(path, "/pids.max", PATH_MAX);
>> -		return read_session_pids_limit(path, uid, cleanup_fn);
>> -	}
>> +	if (!ret)
>> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>>   
>>   	/* Check for generic cgroup v2 pid.max */
>>   	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>> @@ -135,11 +150,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>   							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
>>   							   path, &catchall);
>>   
>> -	if (!ret) {
>> -		strncat(path, cgroup_pids, PATH_MAX);
>> -		strncat(path, "/pids.max", PATH_MAX);
>> -		return read_session_pids_limit(path, uid, cleanup_fn);
>> -	}
>> +	if (!ret)
>> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>>   
>>   	return -1;
>>   }
>> -- 
>> 2.25.1
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib/tst_pid: Go to parent cgroups for max value

[Richard Palethorpe]

Hello,

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> Hello,
>
> On 28/02/2023 14:07, Richard Palethorpe wrote:
>> Hello,
>>
>> Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:
>>
>>> A cgroup resource limitation can be either a number or "max".
>> IIRC this is not true on V1, at least not historically. On some tests we
>> have to simulate "max" when V1 is detected.
>>
>> Perhaps for pids.max specifically it is the case though?
> You would have more experience than me on that front ! However, from
> my understanding, for `pids.max` specifically it should be correct.
> The kernel documentation[0] of the cgroup-v1 pid controller states :
>
>> To set a cgroup to have no limit, set pids.max to “max”. This is the
>   default for all new cgroups [...]

OK, thanks

>
> And from my testing I was not able to generate a cgroup *without*
> pids.max: it was always present and set to "max".
>
> However, there is no `pids.max` for the root cgroup at all; it cannot
> be enabled. Thus it could make sense that if the process is in the
> root cgroup you had to simulate "max", because there wouldn't be
> `pids.max` to read.

I haven't used pids.max, IIRC the memory controller had max added in
V2. The controllers have become more standard over time, but they still
all have their quirks.

>
>
> But as discussed on the first patch I will need to come back to this
> series anyway and rework it to make use of tst_cgroup, and extend
> _that_ if needed, rather than slap some more functionality on top of
> tst_pid.
> If I do so and still need to add a patch similar to this one I will
> keep in mind that this might be only true for the pid controller, for
> cgroups other than the root cgroup.

I think this would be a useful addition to the cgroups API. Most likely
there are a lot of cgroup limits which we would like to detect if
present.

At the moment we probably circumvent a lot of problems by putting tests
in new CGroups which are in the LTP cgroup. We create the LTP group in
root by default, but it can be created by the system in advance with
limits applied.

>
> Thanks again for the review,
> Best regards
> Téo
>
> [0]:
> https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v1/pids.html#usage
>
>>
>>> 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.
>>>
>>> Clean up uid related code as it is not used anymore.
>>>
>>> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
>>> Co-developed-by: Beata Michalska <beata.michalska@arm.com>
>>> Signed-off-by: Beata Michalska <beata.michalska@arm.com>
>>> ---
>>>   lib/tst_pid.c | 96 +++++++++++++++++++++++++++++----------------------
>>>   1 file changed, 54 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/lib/tst_pid.c b/lib/tst_pid.c
>>> index 3d0be6dcd..ad1893290 100644
>>> --- a/lib/tst_pid.c
>>> +++ b/lib/tst_pid.c
>>> @@ -44,50 +44,69 @@ pid_t tst_get_unused_pid_(void (*cleanup_fn) (void))
>>>   	return pid;
>>>   }
>>>   -/*
>>> - * Get the effective session UID - either one invoking current test via sudo
>>> - * or the real UID.
>>> - */
>>> -static unsigned int get_session_uid(void)
>>> +static int __read_pids_limit(const char *path, void (*cleanup_fn) (void))
>>>   {
>>> -	const char *sudo_uid;
>>> +	char max_pids_value[100];
>>> +	int max_pids;
>>>   -	sudo_uid = getenv("SUDO_UID");
>>> -	if (sudo_uid) {
>>> -		unsigned int real_uid;
>>> -		int ret;
>>> +	if (access(path, R_OK) != 0) {
>>> +		tst_resm(TINFO, "Cannot read session user limits from '%s'", path);
>>> +		return -1;
>>> +	}
>>>   -		ret = sscanf(sudo_uid, "%u", &real_uid);
>>> -		if (ret == 1)
>>> -			return real_uid;
>>> +	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pids_value);
>>> +	if (strcmp(max_pids_value, "max")) {
>>> +		max_pids =  SAFE_STRTOL(max_pids_value, 0, INT_MAX);
>>> +		tst_resm(TINFO, "Found limit of processes %d (from %s)",
>>> +				max_pids, path);
>>> +	} else {
>>> +		max_pids = -1;
>>>   	}
>>>   -	return getuid();
>>> +	return max_pids;
>>>   }
>>>   -static int read_session_pids_limit(const char *path_fmt, int
>>> uid,
>>> -				   void (*cleanup_fn) (void))
>>> +/*
>>> + * Take the path to the cgroup mount and to the current cgroup pid controller
>>> + * and try to find the PID limit imposed by cgroup.
>>> + * Go up the cgroup hierarchy if needed, otherwise use the kernel PID limit.
>>> + */
>>> +static int read_session_pids_limit(const char *cgroup_mount,
>>> +				   const char *cgroup_path, void (*cleanup_fn) (void))
>>>   {
>>> -	int max_pids, ret;
>>> -	char max_pid_value[100];
>>> -	char path[PATH_MAX];
>>> -
>>> -	ret = snprintf(path, sizeof(path), path_fmt, uid);
>>> +	int ret, cgroup_depth = 0, max_pids = -1;
>>> +	char path[PATH_MAX + 1], file_path[PATH_MAX + 1];
>>> +	const char *sub_path = cgroup_path;
>>> +
>>> +	/* Find the number of groups we can go up. */
>>> +	do {
>>> +		cgroup_depth += 1;
>>> +		sub_path++;
>>> +		sub_path = strchr(sub_path, '/');
>>> +	} while (sub_path);
>>> +
>>> +	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;
>>> +	for (int i = 0 ; i < cgroup_depth ; i++) {
>>> +		/* Create a path to read from. */
>>> +		ret = snprintf(file_path, sizeof(file_path), "%s/pids.max", path);
>>> +		if (ret < 0 || (size_t)ret >= sizeof(file_path))
>>> +			return -1;
>>> +
>>> +		max_pids = __read_pids_limit(file_path, cleanup_fn);
>>> +		if (max_pids >= 0)
>>> +			return max_pids;
>>> +
>>> +		strncat(path, "/..", PATH_MAX);
>>>   	}
>>>   -	SAFE_FILE_SCANF(cleanup_fn, path, "%s", max_pid_value);
>>> -	if (!strcmp(max_pid_value, "max")) {
>>> +	if (max_pids < 0) {
>>> +		/* Read kernel imposed limits */
>>>   		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);
>>> +		tst_resm(TINFO, "Using kernel processes limit of %d",
>>> +			 max_pids);
>>>   	}
>>>     	return max_pids;
>>> @@ -98,9 +117,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>>   	char path[PATH_MAX + 1];
>>>   	char cgroup_pids[PATH_MAX + 1];
>>>   	char catchall;
>>> -	int uid, ret = 0;
>>> +	int ret = 0;
>>>   -	uid = get_session_uid();
>>>   	/* Check for generic cgroup v1 pid.max */
>>>   	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>>>   						   "%*d:pids:%s\n", cgroup_pids);
>>> @@ -121,11 +139,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>>   							   "%*s %*s %*s %*s %s %*[^-] - cgroup %*s %*[rw],pid%c",
>>>   							   path, &catchall);
>>>   -	if (!ret) {
>>> -		strncat(path, cgroup_pids, PATH_MAX);
>>> -		strncat(path, "/pids.max", PATH_MAX);
>>> -		return read_session_pids_limit(path, uid, cleanup_fn);
>>> -	}
>>> +	if (!ret)
>>> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>>>     	/* Check for generic cgroup v2 pid.max */
>>>   	ret = FILE_LINES_SCANF(cleanup_fn, "/proc/self/cgroup",
>>> @@ -135,11 +150,8 @@ static int get_session_pids_limit(void (*cleanup_fn) (void))
>>>   							   "%*s %*s %*s %*s %s %*[^-] - cgroup2 %c",
>>>   							   path, &catchall);
>>>   -	if (!ret) {
>>> -		strncat(path, cgroup_pids, PATH_MAX);
>>> -		strncat(path, "/pids.max", PATH_MAX);
>>> -		return read_session_pids_limit(path, uid, cleanup_fn);
>>> -	}
>>> +	if (!ret)
>>> +		return read_session_pids_limit(path, cgroup_pids, cleanup_fn);
>>>     	return -1;
>>>   }
>>> -- 2.25.1
>>


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp