All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems
@ 2021-06-14 16:14 Krzysztof Kozlowski
  2021-06-14 16:14 ` [LTP] [PATCH v2 2/2] controllers/cpuacct: fix rmdir failures on early test abort Krzysztof Kozlowski
  2021-06-15 16:40 ` [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Cyril Hrubis
  0 siblings, 2 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-14 16:14 UTC (permalink / raw)
  To: ltp

Running cpuacct_100_100 on a system with less than ~4 GB of RAM fails:

    cpuacct 1 TINFO: timeout per run is 0h 5m 0s
    cpuacct 1 TINFO: cpuacct: /sys/fs/cgroup/cpu,cpuacct
    cpuacct 1 TINFO: Creating 100 subgroups each with 100 processes
    testcases/bin/cpuacct.sh: 0: Cannot fork

In dmesg:

    LTP: starting cpuacct_100_100 (cpuacct.sh 100 100)
    cgroup: fork rejected by pids controller in /user.slice/user-1000.slice/session-1.scope

The reason is cgroups pid limit set by systemd user.slice.  For example
on 2 GB RAM machine it is set as:
    /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.max:5207

Add a check for both free memory and maximum number of pids (when using
systemd) to skip the test.  Systems not using systemd might still fail.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

---

Changes since v1:
1. Add checking for pids.
2. Accurately check the memory constraints.
---
 .../kernel/controllers/cpuacct/cpuacct.sh     | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/testcases/kernel/controllers/cpuacct/cpuacct.sh b/testcases/kernel/controllers/cpuacct/cpuacct.sh
index 323aa7513bf4..66a48dde679b 100755
--- a/testcases/kernel/controllers/cpuacct/cpuacct.sh
+++ b/testcases/kernel/controllers/cpuacct/cpuacct.sh
@@ -38,12 +38,68 @@ OPTIONS
 EOF
 }
 
+check_free_memory()
+{
+	local memneeded
+	local memfree=`awk '/MemAvailable/ {print $2}' /proc/meminfo`
+
+	if [ $? -ne 0 ]; then
+		local memcached
+
+		memfree=`awk '/MemFree/ {print $2}' /proc/meminfo`
+		test $? || return 0
+
+		memcached=`awk '/MemCached/ {print $2}' /proc/meminfo`
+		test $? || return 0
+
+		memfree=$((memfree + memcached))
+	fi
+
+	# On x86_64, each 100 of processes were using ~16 MB of memory,
+	# so try to estimate the needed free memory based on this.
+	memneeded=$((max * nbprocess * 16384 / 100))
+
+	if [ $memfree -lt $memneeded ]; then
+		tst_brk TCONF "not enough of free memory on this system (approximate need $memneeded kB, free $memfree kB)"
+	fi
+	tst_res TINFO "memory requirements fulfilled (approximate need $memneeded kB, free $memfree kB)"
+
+	return 0
+}
+
+check_limits()
+{
+	local realuid="$SUDO_UID"
+	local tasksneeded=$((max * nbprocess + 100))
+
+	if [ "$realuid" = "" ]; then
+		realuid=`id -u`
+		test $? || return 0
+	fi
+
+	local tasksmax=`systemctl show user-${real_uid}.slice | awk -F '=' '/TasksMax/ {print $2}'`
+	test $? || return 0
+
+	if [ $tasksmax -le $tasksneeded ]; then
+		tst_brk TCONF "limit of tasks is too low (approximate need $tasksneeded, limit $tasksmax)"
+	fi
+	tst_res TINFO "task limit fulfilled (approximate need $tasksneeded, limit $tasksmax)"
+
+	return 0
+}
+
 setup()
 {
 	if ! grep -q -w cpuacct /proc/cgroups; then
 		tst_brk TCONF "cpuacct not supported on this system"
 	fi
 
+	check_limits
+	# Don't bother with memory limit checks on smaller tests
+	if [ $max -ge 100 ] && [ $nbprocess -ge 100 ]; then
+		check_free_memory
+	fi
+
 	mount_point=`grep -w cpuacct /proc/mounts | cut -f 2 | cut -d " " -f2`
 	tst_res TINFO "cpuacct: $mount_point"
 	if [ "$mount_point" = "" ]; then
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2 2/2] controllers/cpuacct: fix rmdir failures on early test abort
  2021-06-14 16:14 [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Krzysztof Kozlowski
@ 2021-06-14 16:14 ` Krzysztof Kozlowski
  2021-06-15 16:40 ` [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Cyril Hrubis
  1 sibling, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-14 16:14 UTC (permalink / raw)
  To: ltp

The "testpath" variable is assigned at the end of setup(), so if test
exits early, the "rmdir $testpath" is wrong:

    cpuacct 1 TCONF: not enough of memory on this system (less than 3840 MB)
    cpuacct 1 TINFO: removing created directories
    rmdir: missing operand
    Try 'rmdir --help' for more information.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

---

Changes since v1:
1. None.
---
 testcases/kernel/controllers/cpuacct/cpuacct.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/controllers/cpuacct/cpuacct.sh b/testcases/kernel/controllers/cpuacct/cpuacct.sh
index 66a48dde679b..2e9ff5a5286c 100755
--- a/testcases/kernel/controllers/cpuacct/cpuacct.sh
+++ b/testcases/kernel/controllers/cpuacct/cpuacct.sh
@@ -127,12 +127,13 @@ cleanup()
 {
 	tst_res TINFO "removing created directories"
 
-	if [ -d "$testpath/subgroup_1" ]; then
-		rmdir $testpath/subgroup_*
+	if [ "$testpath" ]; then
+		if [ -d "$testpath/subgroup_1" ]; then
+			rmdir $testpath/subgroup_*
+		fi
+		rmdir $testpath
 	fi
 
-	rmdir $testpath
-
 	if [ "$mounted" -ne 1 ]; then
 		tst_res TINFO "Umounting cpuacct"
 		umount $mount_point
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems
  2021-06-14 16:14 [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Krzysztof Kozlowski
  2021-06-14 16:14 ` [LTP] [PATCH v2 2/2] controllers/cpuacct: fix rmdir failures on early test abort Krzysztof Kozlowski
@ 2021-06-15 16:40 ` Cyril Hrubis
  2021-06-16  7:27   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2021-06-15 16:40 UTC (permalink / raw)
  To: ltp

Hi!
> +check_free_memory()
> +{
> +	local memneeded
> +	local memfree=`awk '/MemAvailable/ {print $2}' /proc/meminfo`
> +
> +	if [ $? -ne 0 ]; then
> +		local memcached
> +
> +		memfree=`awk '/MemFree/ {print $2}' /proc/meminfo`
> +		test $? || return 0
> +
> +		memcached=`awk '/MemCached/ {print $2}' /proc/meminfo`
> +		test $? || return 0

I do not think that something as basic as awk on /proc/meminfo here will
fail...

> +		memfree=$((memfree + memcached))
> +	fi
> +
> +	# On x86_64, each 100 of processes were using ~16 MB of memory,
> +	# so try to estimate the needed free memory based on this.
> +	memneeded=$((max * nbprocess * 16384 / 100))
> +
> +	if [ $memfree -lt $memneeded ]; then

I would still add some memory margin to the memneeded here. At least
add a hundred of megabytes before we do the check.

> +		tst_brk TCONF "not enough of free memory on this system (approximate need $memneeded kB, free $memfree kB)"
> +	fi
> +	tst_res TINFO "memory requirements fulfilled (approximate need $memneeded kB, free $memfree kB)"
> +
> +	return 0
> +}
> +
> +check_limits()
> +{
> +	local realuid="$SUDO_UID"
> +	local tasksneeded=$((max * nbprocess + 100))
> +
> +	if [ "$realuid" = "" ]; then
> +		realuid=`id -u`
> +		test $? || return 0
> +	fi
> +
> +	local tasksmax=`systemctl show user-${real_uid}.slice | awk -F '=' '/TasksMax/ {print $2}'`
> +	test $? || return 0
> +
> +	if [ $tasksmax -le $tasksneeded ]; then
> +		tst_brk TCONF "limit of tasks is too low (approximate need $tasksneeded, limit $tasksmax)"
> +	fi
> +	tst_res TINFO "task limit fulfilled (approximate need $tasksneeded, limit $tasksmax)"

Huh, is this really needed? The test is supposed to run under root. The
user is supposed to login as a root or at least do 'su -' before
executing LTP anyways.

> +	return 0
> +}
> +
>  setup()
>  {
>  	if ! grep -q -w cpuacct /proc/cgroups; then
>  		tst_brk TCONF "cpuacct not supported on this system"
>  	fi
>  
> +	check_limits
> +	# Don't bother with memory limit checks on smaller tests
> +	if [ $max -ge 100 ] && [ $nbprocess -ge 100 ]; then

We should probably check if the $max * $mbprocess -ge 1000 or something
like that just in a case someone addds a test with large enough
$nbprocess.

> +		check_free_memory
> +	fi
> +
>  	mount_point=`grep -w cpuacct /proc/mounts | cut -f 2 | cut -d " " -f2`
>  	tst_res TINFO "cpuacct: $mount_point"
>  	if [ "$mount_point" = "" ]; then
> -- 
> 2.27.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems
  2021-06-15 16:40 ` [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Cyril Hrubis
@ 2021-06-16  7:27   ` Krzysztof Kozlowski
  2021-06-16  7:28     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-16  7:27 UTC (permalink / raw)
  To: ltp

On 15/06/2021 18:40, Cyril Hrubis wrote:
> Hi!
>> +check_free_memory()
>> +{
>> +	local memneeded
>> +	local memfree=`awk '/MemAvailable/ {print $2}' /proc/meminfo`
>> +
>> +	if [ $? -ne 0 ]; then
>> +		local memcached
>> +
>> +		memfree=`awk '/MemFree/ {print $2}' /proc/meminfo`
>> +		test $? || return 0
>> +
>> +		memcached=`awk '/MemCached/ {print $2}' /proc/meminfo`
>> +		test $? || return 0
> 
> I do not think that something as basic as awk on /proc/meminfo here will
> fail...

It's still nice pattern to check for return values but if you insist, I
can drop it.

> 
>> +		memfree=$((memfree + memcached))
>> +	fi
>> +
>> +	# On x86_64, each 100 of processes were using ~16 MB of memory,
>> +	# so try to estimate the needed free memory based on this.
>> +	memneeded=$((max * nbprocess * 16384 / 100))
>> +
>> +	if [ $memfree -lt $memneeded ]; then
> 
> I would still add some memory margin to the memneeded here. At least
> add a hundred of megabytes before we do the check.

Sure.

> 
>> +		tst_brk TCONF "not enough of free memory on this system (approximate need $memneeded kB, free $memfree kB)"
>> +	fi
>> +	tst_res TINFO "memory requirements fulfilled (approximate need $memneeded kB, free $memfree kB)"
>> +
>> +	return 0
>> +}
>> +
>> +check_limits()
>> +{
>> +	local realuid="$SUDO_UID"
>> +	local tasksneeded=$((max * nbprocess + 100))
>> +
>> +	if [ "$realuid" = "" ]; then
>> +		realuid=`id -u`
>> +		test $? || return 0
>> +	fi
>> +
>> +	local tasksmax=`systemctl show user-${real_uid}.slice | awk -F '=' '/TasksMax/ {print $2}'`
>> +	test $? || return 0
>> +
>> +	if [ $tasksmax -le $tasksneeded ]; then
>> +		tst_brk TCONF "limit of tasks is too low (approximate need $tasksneeded, limit $tasksmax)"
>> +	fi
>> +	tst_res TINFO "task limit fulfilled (approximate need $tasksneeded, limit $tasksmax)"
> 
> Huh, is this really needed? The test is supposed to run under root. The
> user is supposed to login as a root or at least do 'su -' before
> executing LTP anyways.

Yeah, because even root sessions (user-0.slice) have the PID limit set
by systemd.

$ ssh root@....
$ grep . /sys/fs/cgroup/pids/user.slice/*/*
/sys/fs/cgroup/pids/user.slice/user-0.slice/cgroup.clone_children:0
/sys/fs/cgroup/pids/user.slice/user-0.slice/notify_on_release:0
/sys/fs/cgroup/pids/user.slice/user-0.slice/pids.current:5
/sys/fs/cgroup/pids/user.slice/user-0.slice/pids.events:max 0
/sys/fs/cgroup/pids/user.slice/user-0.slice/pids.max:5207


> 
>> +	return 0
>> +}
>> +
>>  setup()
>>  {
>>  	if ! grep -q -w cpuacct /proc/cgroups; then
>>  		tst_brk TCONF "cpuacct not supported on this system"
>>  	fi
>>  
>> +	check_limits
>> +	# Don't bother with memory limit checks on smaller tests
>> +	if [ $max -ge 100 ] && [ $nbprocess -ge 100 ]; then
> 
> We should probably check if the $max * $mbprocess -ge 1000 or something
> like that just in a case someone addds a test with large enough
> $nbprocess.

Makes sense, optionally the check can be done always.

Thanks for the review!


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems
  2021-06-16  7:27   ` Krzysztof Kozlowski
@ 2021-06-16  7:28     ` Krzysztof Kozlowski
  2021-06-24 19:30       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-16  7:28 UTC (permalink / raw)
  To: ltp

On 16/06/2021 09:27, Krzysztof Kozlowski wrote:
> On 15/06/2021 18:40, Cyril Hrubis wrote:
>> Hi!
>>> +check_free_memory()
>>> +{
>>> +	local memneeded
>>> +	local memfree=`awk '/MemAvailable/ {print $2}' /proc/meminfo`
>>> +
>>> +	if [ $? -ne 0 ]; then
>>> +		local memcached
>>> +
>>> +		memfree=`awk '/MemFree/ {print $2}' /proc/meminfo`
>>> +		test $? || return 0
>>> +
>>> +		memcached=`awk '/MemCached/ {print $2}' /proc/meminfo`
>>> +		test $? || return 0
>>
>> I do not think that something as basic as awk on /proc/meminfo here will
>> fail...
> 
> It's still nice pattern to check for return values but if you insist, I
> can drop it.
> 
>>
>>> +		memfree=$((memfree + memcached))
>>> +	fi
>>> +
>>> +	# On x86_64, each 100 of processes were using ~16 MB of memory,
>>> +	# so try to estimate the needed free memory based on this.
>>> +	memneeded=$((max * nbprocess * 16384 / 100))
>>> +
>>> +	if [ $memfree -lt $memneeded ]; then
>>
>> I would still add some memory margin to the memneeded here. At least
>> add a hundred of megabytes before we do the check.
> 
> Sure.
> 
>>
>>> +		tst_brk TCONF "not enough of free memory on this system (approximate need $memneeded kB, free $memfree kB)"
>>> +	fi
>>> +	tst_res TINFO "memory requirements fulfilled (approximate need $memneeded kB, free $memfree kB)"
>>> +
>>> +	return 0
>>> +}
>>> +
>>> +check_limits()
>>> +{
>>> +	local realuid="$SUDO_UID"
>>> +	local tasksneeded=$((max * nbprocess + 100))
>>> +
>>> +	if [ "$realuid" = "" ]; then
>>> +		realuid=`id -u`
>>> +		test $? || return 0
>>> +	fi
>>> +
>>> +	local tasksmax=`systemctl show user-${real_uid}.slice | awk -F '=' '/TasksMax/ {print $2}'`
>>> +	test $? || return 0
>>> +
>>> +	if [ $tasksmax -le $tasksneeded ]; then
>>> +		tst_brk TCONF "limit of tasks is too low (approximate need $tasksneeded, limit $tasksmax)"
>>> +	fi
>>> +	tst_res TINFO "task limit fulfilled (approximate need $tasksneeded, limit $tasksmax)"
>>
>> Huh, is this really needed? The test is supposed to run under root. The
>> user is supposed to login as a root or at least do 'su -' before
>> executing LTP anyways.
> 
> Yeah, because even root sessions (user-0.slice) have the PID limit set
> by systemd.
> 
> $ ssh root@....
> $ grep . /sys/fs/cgroup/pids/user.slice/*/*
> /sys/fs/cgroup/pids/user.slice/user-0.slice/cgroup.clone_children:0
> /sys/fs/cgroup/pids/user.slice/user-0.slice/notify_on_release:0
> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.current:5
> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.events:max 0
> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.max:5207

I forgot to mention: that's a default Ubuntu 20.04 LTS system on 2 GB
RAM machine, with systemd 245.4-4ubuntu3.6.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems
  2021-06-16  7:28     ` Krzysztof Kozlowski
@ 2021-06-24 19:30       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-24 19:30 UTC (permalink / raw)
  To: ltp

On 16/06/2021 09:28, Krzysztof Kozlowski wrote:
> On 16/06/2021 09:27, Krzysztof Kozlowski wrote:
>> On 15/06/2021 18:40, Cyril Hrubis wrote:
>>> Hi!
>>>> +check_free_memory()
>>>> +{
>>>> +	local memneeded
>>>> +	local memfree=`awk '/MemAvailable/ {print $2}' /proc/meminfo`
>>>> +
>>>> +	if [ $? -ne 0 ]; then
>>>> +		local memcached
>>>> +
>>>> +		memfree=`awk '/MemFree/ {print $2}' /proc/meminfo`
>>>> +		test $? || return 0
>>>> +
>>>> +		memcached=`awk '/MemCached/ {print $2}' /proc/meminfo`
>>>> +		test $? || return 0
>>>
>>> I do not think that something as basic as awk on /proc/meminfo here will
>>> fail...
>>
>> It's still nice pattern to check for return values but if you insist, I
>> can drop it.
>>
>>>
>>>> +		memfree=$((memfree + memcached))
>>>> +	fi
>>>> +
>>>> +	# On x86_64, each 100 of processes were using ~16 MB of memory,
>>>> +	# so try to estimate the needed free memory based on this.
>>>> +	memneeded=$((max * nbprocess * 16384 / 100))
>>>> +
>>>> +	if [ $memfree -lt $memneeded ]; then
>>>
>>> I would still add some memory margin to the memneeded here. At least
>>> add a hundred of megabytes before we do the check.
>>
>> Sure.
>>
>>>
>>>> +		tst_brk TCONF "not enough of free memory on this system (approximate need $memneeded kB, free $memfree kB)"
>>>> +	fi
>>>> +	tst_res TINFO "memory requirements fulfilled (approximate need $memneeded kB, free $memfree kB)"
>>>> +
>>>> +	return 0
>>>> +}
>>>> +
>>>> +check_limits()
>>>> +{
>>>> +	local realuid="$SUDO_UID"
>>>> +	local tasksneeded=$((max * nbprocess + 100))
>>>> +
>>>> +	if [ "$realuid" = "" ]; then
>>>> +		realuid=`id -u`
>>>> +		test $? || return 0
>>>> +	fi
>>>> +
>>>> +	local tasksmax=`systemctl show user-${real_uid}.slice | awk -F '=' '/TasksMax/ {print $2}'`
>>>> +	test $? || return 0
>>>> +
>>>> +	if [ $tasksmax -le $tasksneeded ]; then
>>>> +		tst_brk TCONF "limit of tasks is too low (approximate need $tasksneeded, limit $tasksmax)"
>>>> +	fi
>>>> +	tst_res TINFO "task limit fulfilled (approximate need $tasksneeded, limit $tasksmax)"
>>>
>>> Huh, is this really needed? The test is supposed to run under root. The
>>> user is supposed to login as a root or at least do 'su -' before
>>> executing LTP anyways.
>>
>> Yeah, because even root sessions (user-0.slice) have the PID limit set
>> by systemd.
>>
>> $ ssh root@....
>> $ grep . /sys/fs/cgroup/pids/user.slice/*/*
>> /sys/fs/cgroup/pids/user.slice/user-0.slice/cgroup.clone_children:0
>> /sys/fs/cgroup/pids/user.slice/user-0.slice/notify_on_release:0
>> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.current:5
>> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.events:max 0
>> /sys/fs/cgroup/pids/user.slice/user-0.slice/pids.max:5207
> 
> I forgot to mention: that's a default Ubuntu 20.04 LTS system on 2 GB
> RAM machine, with systemd 245.4-4ubuntu3.6.

Any further comments here or something to fix? I think there were no
objections to this patch, right?


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-24 19:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 16:14 [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Krzysztof Kozlowski
2021-06-14 16:14 ` [LTP] [PATCH v2 2/2] controllers/cpuacct: fix rmdir failures on early test abort Krzysztof Kozlowski
2021-06-15 16:40 ` [LTP] [PATCH v2 1/2] controllers/cpuacct: skip cpuacct_100_100 on small memory systems Cyril Hrubis
2021-06-16  7:27   ` Krzysztof Kozlowski
2021-06-16  7:28     ` Krzysztof Kozlowski
2021-06-24 19:30       ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.