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

[Richard Palethorpe <rpalethorpe@suse.de>]

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