* [LTP] [PATCH] memcg: memcg_subgroup_charge.sh: Fix the parent memory limit
@ 2021-03-05 22:27 Masayoshi Mizuma
2021-03-08 8:07 ` Joerg Vehlow
0 siblings, 1 reply; 6+ messages in thread
From: Masayoshi Mizuma @ 2021-03-05 22:27 UTC (permalink / raw)
To: ltp
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
memcg_subgroup_charge.sh fails on v5.9 and later kernel.
That's because memory.limit_in_bytes isn't set the suitable value
so mem_process is killed by OOM accidentally.
The memory.limit_in_bytes is now wrong value because commit
3e38e0aaca9e ("mm: memcg: charge memcg percpu memory to the parent cgroup")
changed the charging memory usage. The percpu memory, which is
needed to create the subgroup, is charged to the parent's usage.
Since we can get the amount of the percpu memory as memory.usage_in_bytes
after the subgroup is created, extend the limit to limit_in_bytes + usage_in_bytes.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
.../memcg/functional/memcg_subgroup_charge.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
index 9b23177a4..512a4e3dd 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
@@ -20,9 +20,16 @@ TST_CNT=3
test_subgroup()
{
mkdir subgroup
- echo $1 > memory.limit_in_bytes
echo $2 > subgroup/memory.limit_in_bytes
+ # v5.9 and later kernel, percpu memory which is needed to create
+ # the subgroup is charged to the parent's usage.
+ # Extend the parent limit so that we can observe the rss of
+ # memcg_process correctly.
+ pre_used=$(cat memory.usage_in_bytes)
+ newlimit=$(( $1 + pre_used ))
+ echo $newlimit > memory.limit_in_bytes
+
start_memcg_process --mmap-anon -s $PAGESIZES
warmup
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH] memcg: memcg_subgroup_charge.sh: Fix the parent memory limit
2021-03-05 22:27 [LTP] [PATCH] memcg: memcg_subgroup_charge.sh: Fix the parent memory limit Masayoshi Mizuma
@ 2021-03-08 8:07 ` Joerg Vehlow
2021-09-14 8:34 ` Richard Palethorpe via ltp
0 siblings, 1 reply; 6+ messages in thread
From: Joerg Vehlow @ 2021-03-08 8:07 UTC (permalink / raw)
To: ltp
Hi,
On 3/5/2021 11:27 PM, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
>
> memcg_subgroup_charge.sh fails on v5.9 and later kernel.
> That's because memory.limit_in_bytes isn't set the suitable value
> so mem_process is killed by OOM accidentally.
>
> The memory.limit_in_bytes is now wrong value because commit
> 3e38e0aaca9e ("mm: memcg: charge memcg percpu memory to the parent cgroup")
> changed the charging memory usage. The percpu memory, which is
> needed to create the subgroup, is charged to the parent's usage.
>
> Since we can get the amount of the percpu memory as memory.usage_in_bytes
> after the subgroup is created, extend the limit to limit_in_bytes + usage_in_bytes.
Sounds reasonable, I guess the test always fails on 5.9?
But the problem is, this test also fails on older kernels sometimes.
When I looked at this the last time, I thought it was because sometimes
the kernel requires new pages for the page table and that is also taken
into account for the memory limit.
I still don't know why the test even sets a limit for the parent group
and would vote for completely removing the memory setting on the parent.
J?rg
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] memcg_subgroup_charge: Remove limiting of parent
@ 2021-09-14 8:34 ` Richard Palethorpe via ltp
2021-09-14 12:32 ` Cyril Hrubis
2021-09-20 5:20 ` Joerg Vehlow
0 siblings, 2 replies; 6+ messages in thread
From: Richard Palethorpe via ltp @ 2021-09-14 8:34 UTC (permalink / raw)
To: ltp; +Cc: Masayoshi Mizuma, Richard Palethorpe, Joerg Vehlow
It is not important how much memory is assigned to the parent
group. The stated purpose of the test is to check no memory is
assigned to the child group.
Also add the usage stats for the memcg_process because it appears
the test will fail because the starting memory counter already
includes some buffer/cache on linux-next. I'm not sure this
is exactly what happens, but displaying the stats might help.
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
Cc: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
.../controllers/memcg/functional/memcg_lib.sh | 2 +-
.../memcg/functional/memcg_subgroup_charge.sh | 16 +++++-----------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index ac9ad8268..1b76b6597 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -240,7 +240,7 @@ signal_memcg_process()
loops=$((loops - 1))
if [ $loops -le 0 ]; then
- tst_brk TBROK "timed out on memory.usage_in_bytes"
+ tst_brk TBROK "timed out on memory.usage_in_bytes" $usage $usage_start $size
fi
done
}
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
index 3fa016102..cda624923 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
@@ -18,22 +18,16 @@ TST_CNT=3
MEM_TO_ALLOC=$((PAGESIZES * 2))
# Test the memory charge won't move to subgroup
-# $1 - memory.limit_in_bytes in parent group
-# $2 - memory.limit_in_bytes in sub group
+# $1 - memory.limit_in_bytes in sub group
test_subgroup()
{
- local limit_parent=$1
- local limit_subgroup=$2
+ local limit_subgroup=$1
- if [ $limit_parent -ne 0 ]; then
- limit_parent=$(memcg_adjust_limit_for_kmem $limit_parent)
- fi
if [ $limit_subgroup -ne 0 ]; then
limit_subgroup=$(memcg_adjust_limit_for_kmem $limit_subgroup)
fi
ROD mkdir subgroup
- EXPECT_PASS echo $limit_parent \> memory.limit_in_bytes
EXPECT_PASS echo $limit_subgroup \> subgroup/memory.limit_in_bytes
start_memcg_process --mmap-anon -s $MEM_TO_ALLOC
@@ -60,17 +54,17 @@ test_subgroup()
test1()
{
tst_res TINFO "Test that group and subgroup have no relationship"
- test_subgroup $MEM_TO_ALLOC $((2 * MEM_TO_ALLOC))
+ test_subgroup $((2 * MEM_TO_ALLOC))
}
test2()
{
- test_subgroup $MEM_TO_ALLOC $MEM_TO_ALLOC
+ test_subgroup $MEM_TO_ALLOC
}
test3()
{
- test_subgroup $MEM_TO_ALLOC 0
+ test_subgroup 0
}
tst_run
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] memcg_subgroup_charge: Remove limiting of parent
@ 2021-09-14 12:32 ` Cyril Hrubis
0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2021-09-14 12:32 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: Masayoshi Mizuma, Joerg Vehlow, ltp
Hi!
> It is not important how much memory is assigned to the parent
> group. The stated purpose of the test is to check no memory is
> assigned to the child group.
>
> Also add the usage stats for the memcg_process because it appears
> the test will fail because the starting memory counter already
> includes some buffer/cache on linux-next. I'm not sure this
> is exactly what happens, but displaying the stats might help.
Uff just found that there is a similar patch from Joerg as well:
http://patchwork.ozlabs.org/project/ltp/patch/20191106061808.67330-1-lkml@jv-coder.de/
Either way we should merge one of them to get this finally fixed.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] memcg_subgroup_charge: Remove limiting of parent
2021-09-14 8:34 ` Richard Palethorpe via ltp
2021-09-14 12:32 ` Cyril Hrubis
@ 2021-09-20 5:20 ` Joerg Vehlow
2021-09-21 15:00 ` Cyril Hrubis
1 sibling, 1 reply; 6+ messages in thread
From: Joerg Vehlow @ 2021-09-20 5:20 UTC (permalink / raw)
To: ltp, Richard Palethorpe; +Cc: Masayoshi Mizuma
Hi
On 9/14/2021 10:34 AM, Richard Palethorpe via ltp wrote:
> It is not important how much memory is assigned to the parent
> group. The stated purpose of the test is to check no memory is
> assigned to the child group.
I still don't know why the test even wants to limit anything, when it is
just checking what is charged.
So I would still vote for completely removing the limits and simplifying
to just one test case.
But removing one limitation for now is a step in the right direction, so
I will not argue anymore :)
>
> Also add the usage stats for the memcg_process because it appears
> the test will fail because the starting memory counter already
> includes some buffer/cache on linux-next. I'm not sure this
> is exactly what happens, but displaying the stats might help.
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> Cc: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> ---
> .../controllers/memcg/functional/memcg_lib.sh | 2 +-
> .../memcg/functional/memcg_subgroup_charge.sh | 16 +++++-----------
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> index ac9ad8268..1b76b6597 100755
> --- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> +++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> @@ -240,7 +240,7 @@ signal_memcg_process()
>
> loops=$((loops - 1))
> if [ $loops -le 0 ]; then
> - tst_brk TBROK "timed out on memory.usage_in_bytes"
> + tst_brk TBROK "timed out on memory.usage_in_bytes" $usage $usage_start $size
> fi
> done
> }
> diff --git a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
> index 3fa016102..cda624923 100755
> --- a/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
> +++ b/testcases/kernel/controllers/memcg/functional/memcg_subgroup_charge.sh
> @@ -18,22 +18,16 @@ TST_CNT=3
> MEM_TO_ALLOC=$((PAGESIZES * 2))
>
> # Test the memory charge won't move to subgroup
> -# $1 - memory.limit_in_bytes in parent group
> -# $2 - memory.limit_in_bytes in sub group
> +# $1 - memory.limit_in_bytes in sub group
> test_subgroup()
> {
> - local limit_parent=$1
> - local limit_subgroup=$2
> + local limit_subgroup=$1
>
> - if [ $limit_parent -ne 0 ]; then
> - limit_parent=$(memcg_adjust_limit_for_kmem $limit_parent)
> - fi
> if [ $limit_subgroup -ne 0 ]; then
> limit_subgroup=$(memcg_adjust_limit_for_kmem $limit_subgroup)
> fi
>
> ROD mkdir subgroup
> - EXPECT_PASS echo $limit_parent \> memory.limit_in_bytes
> EXPECT_PASS echo $limit_subgroup \> subgroup/memory.limit_in_bytes
>
> start_memcg_process --mmap-anon -s $MEM_TO_ALLOC
> @@ -60,17 +54,17 @@ test_subgroup()
> test1()
> {
> tst_res TINFO "Test that group and subgroup have no relationship"
> - test_subgroup $MEM_TO_ALLOC $((2 * MEM_TO_ALLOC))
> + test_subgroup $((2 * MEM_TO_ALLOC))
> }
>
> test2()
> {
> - test_subgroup $MEM_TO_ALLOC $MEM_TO_ALLOC
> + test_subgroup $MEM_TO_ALLOC
> }
>
> test3()
> {
> - test_subgroup $MEM_TO_ALLOC 0
> + test_subgroup 0
> }
>
> tst_run
Joerg
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] memcg_subgroup_charge: Remove limiting of parent
2021-09-20 5:20 ` Joerg Vehlow
@ 2021-09-21 15:00 ` Cyril Hrubis
0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2021-09-21 15:00 UTC (permalink / raw)
To: Joerg Vehlow; +Cc: Masayoshi Mizuma, Richard Palethorpe, ltp
Hi!
> I still don't know why the test even wants to limit anything, when it is
> just checking what is charged.
> So I would still vote for completely removing the limits and simplifying
> to just one test case.
>
> But removing one limitation for now is a step in the right direction, so
> I will not argue anymore :)
I've split the patch from Ritchie into two and pushed it so that we have
this fixed for the release. We should review and rethink these tests
once the release is finished since this isn't the only problem there.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-21 15:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 22:27 [LTP] [PATCH] memcg: memcg_subgroup_charge.sh: Fix the parent memory limit Masayoshi Mizuma
2021-03-08 8:07 ` Joerg Vehlow
2021-09-14 8:34 ` [LTP] [PATCH] memcg_subgroup_charge: Remove limiting of parent Richard Palethorpe via ltp
2021-09-14 8:34 ` Richard Palethorpe via ltp
2021-09-14 12:32 ` Cyril Hrubis
2021-09-14 12:32 ` Cyril Hrubis
2021-09-20 5:20 ` Joerg Vehlow
2021-09-21 15:00 ` Cyril Hrubis
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.