memcg_subgroup_charge: Remove limiting of parent

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(-)

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.

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

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.