All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
@ 2021-06-17  7:07 Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 1/3] controllers/memcg: accept range of max_usage_in_bytes Krzysztof Kozlowski
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-17  7:07 UTC (permalink / raw)
  To: ltp

Hi,

This is a resend of previous Github pull:
https://github.com/linux-test-project/ltp/pull/830

The patches fix several test failures we are hitting since months, see:
https://bugs.launchpad.net/bugs/1829979
https://bugs.launchpad.net/bugs/1829984

Best regards,
Krzysztof


Krzysztof Kozlowski (3):
  controllers/memcg: accept range of max_usage_in_bytes
  controllers/memcg: accept range of usage_in_bytes
  controllers/memcg: accept non-zero max_usage_in_bytes after reset

 .../controllers/memcg/functional/memcg_lib.sh | 22 ++++++++++++++-----
 .../memcg_max_usage_in_bytes_test.sh          | 12 ++++++++--
 .../memcg/functional/memcg_stat_rss.sh        | 20 ++++++++---------
 .../memcg/functional/memcg_stat_test.sh       |  8 +++----
 .../functional/memcg_usage_in_bytes_test.sh   | 10 +++++++--
 5 files changed, 49 insertions(+), 23 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/3] controllers/memcg: accept range of max_usage_in_bytes
  2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
@ 2021-06-17  7:07 ` Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 2/3] controllers/memcg: accept range of usage_in_bytes Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-17  7:07 UTC (permalink / raw)
  To: ltp

Several Linux kernel versions report higher max_usage_in_bytes than
expected size of 1024 pages. For example v5.4, v5.8, v5.10
and 5.13.0-rc5:

    memcg_max_usage_in_bytes_test 1 TINFO: Test memory.max_usage_in_bytes
    memcg_max_usage_in_bytes_test 1 TINFO: Running memcg_process --mmap-anon -s 4194304
    memcg_max_usage_in_bytes_test 1 TINFO: Warming up pid: 1393215
    memcg_max_usage_in_bytes_test 1 TINFO: Process is still here after warm up: 1393215
    memcg_max_usage_in_bytes_test 1 TFAIL: memory.max_usage_in_bytes is 4325376, 4194304 expected

It seems that recent Linux kernel reports more memory used than
expected.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../kernel/controllers/memcg/functional/memcg_lib.sh  | 11 +++++++++--
 .../memcg/functional/memcg_max_usage_in_bytes_test.sh |  6 +++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index d9bb6d94324d..083ef376e120 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -140,7 +140,8 @@ shmmax_cleanup()
 
 # Check size in memcg
 # $1 - Item name
-# $2 - Expected size
+# $2 - Expected size lower bound
+# $3 - Expected size upper bound (optional)
 check_mem_stat()
 {
 	local item_size
@@ -151,7 +152,13 @@ check_mem_stat()
 		item_size=$(grep -w $1 memory.stat | cut -d " " -f 2)
 	fi
 
-	if [ "$2" = "$item_size" ]; then
+	if [ "$3" ]; then
+		if [ $item_size -ge $2 ] && [ $item_size -le $3 ]; then
+			tst_res TPASS "$1 is ${2}-${3} as expected"
+		else
+			tst_res TFAIL "$1 is $item_size, ${2}-${3} expected"
+		fi
+	elif [ "$2" = "$item_size" ]; then
 		tst_res TPASS "$1 is $2 as expected"
 	else
 		tst_res TFAIL "$1 is $item_size, $2 expected"
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
index 14daa4651798..a940606cbd34 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
@@ -12,6 +12,10 @@ TST_CNT=4
 . memcg_lib.sh
 
 MEM_TO_ALLOC=$((PAGESIZE * 1024))
+# Recent Linux kernels (at least v5.4) started reporting memory usage of 32
+# pages higher than requested allocation. Cause is not known, so let's just be
+# flexible.
+MEM_EXPECTED_UPPER=$((MEM_TO_ALLOC + PAGESIZE * 32))
 MEM_LIMIT=$((MEM_TO_ALLOC * 2))
 
 # Run test cases which checks memory.[memsw.]max_usage_in_bytes after make
@@ -32,7 +36,7 @@ test_max_usage_in_bytes()
 	signal_memcg_process $MEM_TO_ALLOC
 	signal_memcg_process $MEM_TO_ALLOC
 
-	check_mem_stat $item $MEM_TO_ALLOC
+	check_mem_stat $item $MEM_TO_ALLOC $MEM_EXPECTED_UPPER
 
 	if [ $check_after_reset -eq 1 ]; then
 		echo 0 > $item
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/3] controllers/memcg: accept range of usage_in_bytes
  2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 1/3] controllers/memcg: accept range of max_usage_in_bytes Krzysztof Kozlowski
@ 2021-06-17  7:07 ` Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 3/3] controllers/memcg: accept non-zero max_usage_in_bytes after reset Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-17  7:07 UTC (permalink / raw)
  To: ltp

Several Linux kernel versions report higher usage_in_bytes than
expected size of 1024 pages. For example v5.4, v5.8, v5.10
and 5.13.0-rc5:

    memcg_usage_in_bytes_test 1 TINFO: Test memory.usage_in_bytes
    memcg_usage_in_bytes_test 1 TINFO: Running memcg_process --mmap-anon -s 4194304
    memcg_usage_in_bytes_test 1 TINFO: Warming up pid: 1160
    memcg_usage_in_bytes_test 1 TINFO: Process is still here after warm up: 1160
    memcg_usage_in_bytes_test 1 TFAIL: memory.usage_in_bytes is 4325376, 4194304 expected

It seems that recent Linux kernel reports more memory used than
expected.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../controllers/memcg/functional/memcg_lib.sh | 11 +++++++---
 .../memcg/functional/memcg_stat_rss.sh        | 20 +++++++++----------
 .../memcg/functional/memcg_stat_test.sh       |  8 ++++----
 .../functional/memcg_usage_in_bytes_test.sh   | 10 ++++++++--
 4 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index 083ef376e120..d44bb027076c 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -239,8 +239,9 @@ test_mem_stat()
 	local size=$2
 	local total_size=$3
 	local stat_name=$4
-	local exp_stat_size=$5
-	local check_after_free=$6
+	local exp_stat_size_low=$5
+	local exp_stat_size_up=$6
+	local check_after_free=$7
 
 	start_memcg_process $memtypes -s $size
 
@@ -251,7 +252,11 @@ test_mem_stat()
 	echo $MEMCG_PROCESS_PID > tasks
 	signal_memcg_process $size
 
-	check_mem_stat $stat_name $exp_stat_size
+	if [ "$exp_stat_size_low" = "$exp_stat_size_up" ]; then
+		check_mem_stat $stat_name $exp_stat_size_low
+	else
+		check_mem_stat $stat_name $exp_stat_size_low $exp_stat_size_up
+	fi
 
 	signal_memcg_process $size
 	if $check_after_free; then
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_stat_rss.sh b/testcases/kernel/controllers/memcg/functional/memcg_stat_rss.sh
index 1a6128a6dba8..d9b4ec287b5f 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_stat_rss.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_stat_rss.sh
@@ -18,54 +18,54 @@ TST_CNT=10
 # Test the management and counting of memory
 test1()
 {
-	test_mem_stat "--mmap-anon" $PAGESIZES $PAGESIZES "rss" $PAGESIZES false
+	test_mem_stat "--mmap-anon" $PAGESIZES $PAGESIZES "rss" $PAGESIZES $PAGESIZES false
 }
 
 test2()
 {
-	test_mem_stat "--mmap-file" $PAGESIZE $PAGESIZE "rss" 0 false
+	test_mem_stat "--mmap-file" $PAGESIZE $PAGESIZE "rss" 0 0 false
 }
 
 test3()
 {
-	test_mem_stat "--shm -k 3" $PAGESIZE $PAGESIZE "rss" 0 false
+	test_mem_stat "--shm -k 3" $PAGESIZE $PAGESIZE "rss" 0 0 false
 }
 
 test4()
 {
 	test_mem_stat "--mmap-anon --mmap-file --shm" \
-		$PAGESIZES $((PAGESIZES * 3)) "rss" $PAGESIZES false
+		$PAGESIZES $((PAGESIZES * 3)) "rss" $PAGESIZES $PAGESIZES false
 }
 
 test5()
 {
-	test_mem_stat "--mmap-lock1" $PAGESIZES $PAGESIZES "rss" $PAGESIZES false
+	test_mem_stat "--mmap-lock1" $PAGESIZES $PAGESIZES "rss" $PAGESIZES $PAGESIZES false
 }
 
 test6()
 {
-	test_mem_stat "--mmap-anon" $PAGESIZES $PAGESIZES "rss" $PAGESIZES true
+	test_mem_stat "--mmap-anon" $PAGESIZES $PAGESIZES "rss" $PAGESIZES $PAGESIZES true
 }
 
 test7()
 {
-	test_mem_stat "--mmap-file" $PAGESIZE $PAGESIZE "rss" 0 true
+	test_mem_stat "--mmap-file" $PAGESIZE $PAGESIZE "rss" 0 0 true
 }
 
 test8()
 {
-	test_mem_stat "--shm -k 8" $PAGESIZE $PAGESIZE "rss" 0 true
+	test_mem_stat "--shm -k 8" $PAGESIZE $PAGESIZE "rss" 0 0 true
 }
 
 test9()
 {
 	test_mem_stat "--mmap-anon --mmap-file --shm" \
-		$PAGESIZES $((PAGESIZES * 3)) "rss" $PAGESIZES true
+		$PAGESIZES $((PAGESIZES * 3)) "rss" $PAGESIZES $PAGESIZES true
 }
 
 test10()
 {
-	test_mem_stat "--mmap-lock1" $PAGESIZES $PAGESIZES "rss" $PAGESIZES true
+	test_mem_stat "--mmap-lock1" $PAGESIZES $PAGESIZES "rss" $PAGESIZES $PAGESIZES true
 }
 
 tst_run
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
index 925c4ecf87bc..e5eb7e5d0001 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_stat_test.sh
@@ -14,28 +14,28 @@ TST_CNT=8
 test1()
 {
 	tst_res TINFO "Test cache"
-	test_mem_stat "--shm -k 3" $PAGESIZES $PAGESIZES "cache" $PAGESIZES false
+	test_mem_stat "--shm -k 3" $PAGESIZES $PAGESIZES "cache" $PAGESIZES $PAGESIZES false
 }
 
 test2()
 {
 	tst_res TINFO "Test mapped_file"
 	test_mem_stat "--mmap-file" $PAGESIZES $PAGESIZES \
-		"mapped_file" $PAGESIZES false
+		"mapped_file" $PAGESIZES $PAGESIZES false
 }
 
 test3()
 {
 	tst_res TINFO "Test unevictable with MAP_LOCKED"
 	test_mem_stat "--mmap-lock1" $PAGESIZES $PAGESIZES \
-		"unevictable" $PAGESIZES false
+		"unevictable" $PAGESIZES $PAGESIZES false
 }
 
 test4()
 {
 	tst_res TINFO "Test unevictable with mlock"
 	test_mem_stat "--mmap-lock2" $PAGESIZES $PAGESIZES \
-		"unevictable" $PAGESIZES false
+		"unevictable" $PAGESIZES $PAGESIZES false
 }
 
 test5()
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
index e77d6bf2ef23..b5761a4e4716 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_usage_in_bytes_test.sh
@@ -12,13 +12,18 @@ TST_CNT=2
 . memcg_lib.sh
 
 MEM_TO_ALLOC=$((PAGESIZE * 1024))
+# Recent Linux kernels (at least v5.4) started reporting memory usage of 32
+# pages higher than requested allocation. Cause is not known, so let's just be
+# flexible.
+MEM_EXPECTED_UPPER=$((MEM_TO_ALLOC + PAGESIZE * 32))
 MEM_LIMIT=$((MEM_TO_ALLOC * 2))
 
 test1()
 {
 	tst_res TINFO "Test memory.usage_in_bytes"
 	test_mem_stat "--mmap-anon" $MEM_TO_ALLOC $MEM_TO_ALLOC \
-		"memory.usage_in_bytes" $MEM_TO_ALLOC false
+		"memory.usage_in_bytes" $MEM_TO_ALLOC \
+		$MEM_EXPECTED_UPPER false
 }
 
 test2()
@@ -29,7 +34,8 @@ test2()
 	echo $MEM_LIMIT > memory.limit_in_bytes
 	echo $MEM_LIMIT > memory.memsw.limit_in_bytes
 	test_mem_stat "--mmap-anon" $MEM_TO_ALLOC $MEM_TO_ALLOC \
-		"memory.memsw.usage_in_bytes" $MEM_TO_ALLOC false
+		"memory.memsw.usage_in_bytes" $MEM_TO_ALLOC \
+		$MEM_EXPECTED_UPPER false
 }
 
 tst_run
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 3/3] controllers/memcg: accept non-zero max_usage_in_bytes after reset
  2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 1/3] controllers/memcg: accept range of max_usage_in_bytes Krzysztof Kozlowski
  2021-06-17  7:07 ` [LTP] [PATCH v2 2/3] controllers/memcg: accept range of usage_in_bytes Krzysztof Kozlowski
@ 2021-06-17  7:07 ` Krzysztof Kozlowski
  2021-06-17  7:32 ` [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Richard Palethorpe
  2021-06-24 19:31 ` Krzysztof Kozlowski
  4 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-17  7:07 UTC (permalink / raw)
  To: ltp

Several Linux kernel versions report a non-zero max_usage_in_bytes after
resetting the counter.  For example v5.4, v5.8, v5.10, v5.11, v5.12 and
5.13.0-rc5:

    memcg_max_usage_in_bytes_test 4 TINFO: Test reset memory.memsw.max_usage_in_bytes
    memcg_max_usage_in_bytes_test 4 TINFO: Running memcg_process --mmap-anon -s 4194304
    memcg_max_usage_in_bytes_test 4 TINFO: Warming up pid: 1416
    memcg_max_usage_in_bytes_test 4 TINFO: Process is still here after warm up: 1416
    memcg_max_usage_in_bytes_test 4 TFAIL: memory.memsw.max_usage_in_bytes is 4325376, 4194304 expected
    memcg_max_usage_in_bytes_test 4 TFAIL: memory.memsw.max_usage_in_bytes is 122880, 0 expected

It seems that recent Linux kernel still notices some memory allocation
by the memcg tool.  Accept therefore a range from 0 to 32 pages.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../memcg/functional/memcg_max_usage_in_bytes_test.sh       | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
index a940606cbd34..8f0fc33996f3 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_max_usage_in_bytes_test.sh
@@ -40,7 +40,11 @@ test_max_usage_in_bytes()
 
 	if [ $check_after_reset -eq 1 ]; then
 		echo 0 > $item
-		check_mem_stat $item 0
+		# Recent Linux kernels (at least v5.4) started reporting
+		# a non-zero max_usage_in_bytes after resetting the counter.
+		# The typical values are 0, 4096, 8096 and up to 122880.
+		# Cause is not known, so let's just be flexible.
+		check_mem_stat $item 0 $((PAGESIZE * 32))
 	fi
 
 	stop_memcg_process
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2021-06-17  7:07 ` [LTP] [PATCH v2 3/3] controllers/memcg: accept non-zero max_usage_in_bytes after reset Krzysztof Kozlowski
@ 2021-06-17  7:32 ` Richard Palethorpe
  2021-06-24 19:31 ` Krzysztof Kozlowski
  4 siblings, 0 replies; 11+ messages in thread
From: Richard Palethorpe @ 2021-06-17  7:32 UTC (permalink / raw)
  To: ltp

Hello,

Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> writes:

> Hi,
>
> This is a resend of previous Github pull:
> https://github.com/linux-test-project/ltp/pull/830

LGTM (Reviewed on Github).

I doubt tests that expect a precise allocation figure are valid.

>
> The patches fix several test failures we are hitting since months, see:
> https://bugs.launchpad.net/bugs/1829979
> https://bugs.launchpad.net/bugs/1829984
>
> Best regards,
> Krzysztof
>
>
> Krzysztof Kozlowski (3):
>   controllers/memcg: accept range of max_usage_in_bytes
>   controllers/memcg: accept range of usage_in_bytes
>   controllers/memcg: accept non-zero max_usage_in_bytes after reset
>
>  .../controllers/memcg/functional/memcg_lib.sh | 22 ++++++++++++++-----
>  .../memcg_max_usage_in_bytes_test.sh          | 12 ++++++++--
>  .../memcg/functional/memcg_stat_rss.sh        | 20 ++++++++---------
>  .../memcg/functional/memcg_stat_test.sh       |  8 +++----
>  .../functional/memcg_usage_in_bytes_test.sh   | 10 +++++++--
>  5 files changed, 49 insertions(+), 23 deletions(-)
>
> -- 
> 2.27.0


-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2021-06-17  7:32 ` [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Richard Palethorpe
@ 2021-06-24 19:31 ` Krzysztof Kozlowski
  2021-06-25  8:21   ` Li Wang
  4 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-24 19:31 UTC (permalink / raw)
  To: ltp

On 17/06/2021 09:07, Krzysztof Kozlowski wrote:
> Hi,
> 
> This is a resend of previous Github pull:
> https://github.com/linux-test-project/ltp/pull/830
> 
> The patches fix several test failures we are hitting since months, see:
> https://bugs.launchpad.net/bugs/1829979
> https://bugs.launchpad.net/bugs/1829984
> 
> Best regards,
> Krzysztof
> 
> 
> Krzysztof Kozlowski (3):
>   controllers/memcg: accept range of max_usage_in_bytes
>   controllers/memcg: accept range of usage_in_bytes
>   controllers/memcg: accept non-zero max_usage_in_bytes after reset


Hi everyone,

The patchset got positive LGTM on the Github. Any further comments for
it or can it be applied?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-24 19:31 ` Krzysztof Kozlowski
@ 2021-06-25  8:21   ` Li Wang
  2021-06-25  9:13     ` Krzysztof Kozlowski
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Li Wang @ 2021-06-25  8:21 UTC (permalink / raw)
  To: ltp

On Fri, Jun 25, 2021 at 3:31 AM Krzysztof Kozlowski <
krzysztof.kozlowski@canonical.com> wrote:

> On 17/06/2021 09:07, Krzysztof Kozlowski wrote:
> > Hi,
> >
> > This is a resend of previous Github pull:
> > https://github.com/linux-test-project/ltp/pull/830
> >
> > The patches fix several test failures we are hitting since months, see:
> > https://bugs.launchpad.net/bugs/1829979
> > https://bugs.launchpad.net/bugs/1829984
> >
> > Best regards,
> > Krzysztof
> >
> >
> > Krzysztof Kozlowski (3):
> >   controllers/memcg: accept range of max_usage_in_bytes
> >   controllers/memcg: accept range of usage_in_bytes
> >   controllers/memcg: accept non-zero max_usage_in_bytes after reset
>
>
> Hi everyone,
>
> The patchset got positive LGTM on the Github. Any further comments for
> it or can it be applied?
>

I slightly agree with Richard that we need an explanation/investigation
on where the 32*PAGE_SIZE comes from. Otherwise, we are very possible
to mask a counter bug if only to make the test happy.

Forgive me pour cold water on the method though it looks good in coding:).

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210625/0f253bc8/attachment.htm>

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-25  8:21   ` Li Wang
@ 2021-06-25  9:13     ` Krzysztof Kozlowski
  2021-06-25  9:24     ` Joerg Vehlow
  2021-07-02 10:49     ` Krzysztof Kozlowski
  2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-25  9:13 UTC (permalink / raw)
  To: ltp

On 25/06/2021 10:21, Li Wang wrote:
> 
> 
> On Fri, Jun 25, 2021 at 3:31 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com
> <mailto:krzysztof.kozlowski@canonical.com>> wrote:
> 
>     Hi everyone,
> 
>     The patchset got positive LGTM on the Github. Any further comments for
>     it or can it be applied?
> 
> 
> I slightly agree with Richard that we need an explanation/investigation
> on where the 32*PAGE_SIZE comes from. Otherwise, we are very possible
> to mask a counter bug if only to make the test happy.

I don't know where 32*PAGE_SIZE comes from and investigation would
require effort/time which I don't have.

I don't think we mask current bug as this appears in multiple kernels -
I tested from v5.4 up to 5.13.0-rc5-next-20210608.

It is possible this will mask future bugs but that's life of a project
depending on kernel internals, not on API or ABI. The kernel is allowed
to change such details any moment because it is neither part of API nor
ABI. Therefore you just have to live with inaccurate limits or keep
investigating every x-months.

For now the test is simply unreliable.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-25  8:21   ` Li Wang
  2021-06-25  9:13     ` Krzysztof Kozlowski
@ 2021-06-25  9:24     ` Joerg Vehlow
  2021-06-25 11:55       ` Krzysztof Kozlowski
  2021-07-02 10:49     ` Krzysztof Kozlowski
  2 siblings, 1 reply; 11+ messages in thread
From: Joerg Vehlow @ 2021-06-25  9:24 UTC (permalink / raw)
  To: ltp

Hi,

On 6/25/2021 10:21 AM, Li Wang wrote:
> On Fri, Jun 25, 2021 at 3:31 AM Krzysztof Kozlowski 
> <krzysztof.kozlowski@canonical.com 
> <mailto:krzysztof.kozlowski@canonical.com>> wrote:
>
>     On 17/06/2021 09:07, Krzysztof Kozlowski wrote:
>
>
>     The patchset got positive LGTM on the Github. Any further comments for
>     it or can it be applied?
>
>
> I slightly agree with Richard that we need an explanation/investigation
> on where the 32*PAGE_SIZE comes from. Otherwise, we are very possible
> to mask a counter bug if only to make the test happy.
+1. I think it has something to do with batch processing of counter updates.
The memory handling code is heavily batched per cpu and only committed 
to a global counter after the batch size overflows.
At least for the subgroup_charge test I once found that page table cache 
stored in kmem contributes to the memory counter used in the test.
Maybe something in that region was changed in recent kernel version. 
Maybe you could ask on the kernel mailing list?


>
> Forgive me pour cold water on the method though it looks good in coding:).
I would just suggest to make 32 * PAGESIZE a constant calculated in 
memcg_lib.sh, instead of calculating (and defining) it everywhere.
>
> -- 
> Regards,
> Li Wang
>
J?rg

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-25  9:24     ` Joerg Vehlow
@ 2021-06-25 11:55       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-25 11:55 UTC (permalink / raw)
  To: ltp

On 25/06/2021 11:24, Joerg Vehlow wrote:
>> Forgive me pour cold water on the method though it looks good in coding:).
> I would just suggest to make 32 * PAGESIZE a constant calculated in 
> memcg_lib.sh, instead of calculating (and defining) it everywhere.

I understand that you want to define constant in memcg_lib for a
accepted higher bound of some specific memory limit checks? Won't that
be extra confusing? Trying to generalize some constant without actually
knowing what is it about and to which limits it applies?

Now it's this unknown part is quite local and documented in three
places. I can move it to bigger scope and pretend it's generic, if you want.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels
  2021-06-25  8:21   ` Li Wang
  2021-06-25  9:13     ` Krzysztof Kozlowski
  2021-06-25  9:24     ` Joerg Vehlow
@ 2021-07-02 10:49     ` Krzysztof Kozlowski
  2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-07-02 10:49 UTC (permalink / raw)
  To: ltp

On 25/06/2021 10:21, Li Wang wrote:
> 
> 
> On Fri, Jun 25, 2021 at 3:31 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com
> <mailto:krzysztof.kozlowski@canonical.com>> wrote:
> 
>     On 17/06/2021 09:07, Krzysztof Kozlowski wrote:
>     > Hi,
>     >
>     > This is a resend of previous Github pull:
>     > https://github.com/linux-test-project/ltp/pull/830
>     <https://github.com/linux-test-project/ltp/pull/830>
>     >
>     > The patches fix several test failures we are hitting since months,
>     see:
>     > https://bugs.launchpad.net/bugs/1829979
>     <https://bugs.launchpad.net/bugs/1829979>
>     > https://bugs.launchpad.net/bugs/1829984
>     <https://bugs.launchpad.net/bugs/1829984>
>     >
>     > Best regards,
>     > Krzysztof
>     >
>     >
>     > Krzysztof Kozlowski (3):
>     >? ?controllers/memcg: accept range of max_usage_in_bytes
>     >? ?controllers/memcg: accept range of usage_in_bytes
>     >? ?controllers/memcg: accept non-zero max_usage_in_bytes after reset
> 
> 
>     Hi everyone,
> 
>     The patchset got positive LGTM on the Github. Any further comments for
>     it or can it be applied?
> 
> 
> I slightly agree with Richard that we need an explanation/investigation
> on where the 32*PAGE_SIZE comes from. Otherwise, we are very possible
> to mask a counter bug if only to make the test happy.

On newer v5.11 the max_usage_in_bytes go even above 32 pages up to 34.

I got some explanation from Michal Hocko [1] from which one can try to
conclude:
1. There is significant caching in memory accounting. Not only charging
of cgroup is once per MEMCG_CHARGE_BATCH batch (try_charge()), but also
statistics are gathered from per-cpu if threshold exceed
MEMCG_CHARGE_BATCH (__mod_memcg_state()).

2. Depending on machine (different amount of CPUs), the memory group
charging be less or more accurate.

3. The accuracy of group memory accounting is not considered as
important thus test relying on it, will be failing from time to time.


I'll send a v3 with significantly increased limits and some explanation.

[1]
https://lore.kernel.org/linux-mm/85b8a4f9-b9e9-a6ca-5d0c-c1ecb8c11ef3@canonical.com/T/#m6459b3be3a86f5eaf2cfc48dd586b6faf949e440


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-07-02 10:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17  7:07 [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Krzysztof Kozlowski
2021-06-17  7:07 ` [LTP] [PATCH v2 1/3] controllers/memcg: accept range of max_usage_in_bytes Krzysztof Kozlowski
2021-06-17  7:07 ` [LTP] [PATCH v2 2/3] controllers/memcg: accept range of usage_in_bytes Krzysztof Kozlowski
2021-06-17  7:07 ` [LTP] [PATCH v2 3/3] controllers/memcg: accept non-zero max_usage_in_bytes after reset Krzysztof Kozlowski
2021-06-17  7:32 ` [LTP] [PATCH v2 0/3] controllers/memcg: fixes for newer kernels Richard Palethorpe
2021-06-24 19:31 ` Krzysztof Kozlowski
2021-06-25  8:21   ` Li Wang
2021-06-25  9:13     ` Krzysztof Kozlowski
2021-06-25  9:24     ` Joerg Vehlow
2021-06-25 11:55       ` Krzysztof Kozlowski
2021-07-02 10:49     ` 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.