diff mbox series

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

Message ID 20230215145440.78482-3-teo.coupriediaz@arm.com
State Changes Requested
Headers show
Series lib/tst_pid: Find session PID limit more reliably | expand

Commit Message

Teo Couprie Diaz Feb. 15, 2023, 2:54 p.m. UTC
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(-)

Comments

Richard Palethorpe Feb. 28, 2023, 2:07 p.m. UTC | #1
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
Teo Couprie Diaz March 1, 2023, 10:33 a.m. UTC | #2
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
>
Richard Palethorpe March 2, 2023, 8:17 a.m. UTC | #3
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
>>
diff mbox series

Patch

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;
 }