diff mbox series

memcg_use_hierarchy_test.sh: skip setting use_hierarchy if not available

Message ID 20200911092121.1917-1-chenhx.fnst@cn.fujitsu.com
State New
Headers show
Series memcg_use_hierarchy_test.sh: skip setting use_hierarchy if not available | expand

Commit Message

Chen, Hanxiao Sept. 11, 2020, 9:21 a.m. UTC
The precondition of this case is that we can disable use_hierarchy.
But some distributions such as CentOS 8 had enabled it in root
cgroup and hard to disabled.

As [1] describe:
    NOTE1: Enabling/disabling will fail if either the cgroup already has other
       cgroups created below it, or if the parent cgroup has use_hierarchy
       enabled.

We should check the precondition before testing.

[1] https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt

Signed-off-by: Chen Hanxiao <chenhx.fnst@cn.fujitsu.com>
---
 .../controllers/memcg/functional/memcg_lib.sh | 25 +++++++++++++++----
 .../functional/memcg_use_hierarchy_test.sh    | 12 +++++++--
 2 files changed, 30 insertions(+), 7 deletions(-)

Comments

Yang Xu Sept. 17, 2020, 5:21 a.m. UTC | #1
Hi hanxiao


> The precondition of this case is that we can disable use_hierarchy.
> But some distributions such as CentOS 8 had enabled it in root
> cgroup and hard to disabled.
> 
> As [1] describe:
>      NOTE1: Enabling/disabling will fail if either the cgroup already has other
>         cgroups created below it, or if the parent cgroup has use_hierarchy
>         enabled.
> 
> We should check the precondition before testing.
> 
> [1] https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
> 
> Signed-off-by: Chen Hanxiao <chenhx.fnst@cn.fujitsu.com>
> ---
>   .../controllers/memcg/functional/memcg_lib.sh | 25 +++++++++++++++----
>   .../functional/memcg_use_hierarchy_test.sh    | 12 +++++++--
>   2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> index 22ef4f5e2..4f99c6f59 100755
> --- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> +++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
> @@ -491,6 +491,10 @@ cleanup_test()
>   		orig_memory_use_hierarchy=""
>   	fi
>   
> +	if [ -n "$root_memory_use_hierarchy" ];then
> +                root_memory_use_hierarchy=""
> +	fi
> +
>   	killall -9 memcg_process 2>/dev/null
>   	wait
>   
> @@ -514,16 +518,27 @@ setup_test()
>   	# while there are distributions (RHEL7U0Beta for example) that sets
>   	# it to 1.
>   	orig_memory_use_hierarchy=$(cat /dev/memcg/memory.use_hierarchy)
> +        MEMCGROUP_PATH="/sys/fs/cgroup/memory"
> +        if [ -e "$MEMCGROUP_PATH" ];then
> +                root_memory_use_hierarchy=$(cat "$MEMCGROUP_PATH/memory.use_hierarchy")
> +        fi
> +
>   	if [ -z "$orig_memory_use_hierarchy" ];then
>   		tst_resm TINFO "cat /dev/memcg/memory.use_hierarchy failed"
>   	elif [ "$orig_memory_use_hierarchy" = "0" ];then
>   		orig_memory_use_hierarchy=""
>   	else
> -		echo 0 > /dev/memcg/memory.use_hierarchy
> -		if [ $? -ne 0 ];then
> -			tst_resm TINFO "set /dev/memcg/memory.use_hierarchy" \
> -				"to 0 failed"
> -		fi
> +                if [ "$root_memory_use_hierarchy" = "1" ]; then
> +                        tst_resm TINFO "root cgroup has use_hierarchy enabled, " \
> +                            "can't set /dev/memcg/memory.use_hierarchy to 0"
> +                        export root_memory_use_hierarchy
> +                else
> +                        echo 0 > /dev/memcg/memory.use_hierarchy
> +                        if [ $? -ne 0 ];then
> +                                tst_resm TINFO "set /dev/memcg/memory.use_hierarchy" \
> +                                    "to 0 failed"
> +                        fi
> +                fi
I test this patch on centos7 and testcase2 skips. On centos7(without 
installing docker), /sys/fs/cgroup/memory/memory.use_hierarchy value is 
equal to 1 and I still can disable value for 
/dev/memcg/memory.use_hierarchy.

So why not directly use /dev/memcg/memory.use_hierarchy value to judge 
in testcase after setting 0.

Best Regards
Yang Xu
>   	fi
>   
>   	ROD mkdir "/dev/memcg/$TEST_ID"
> diff --git a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
> index 4cf6b9fc2..1439b6352 100755
> --- a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
> +++ b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
> @@ -34,7 +34,9 @@ TST_TOTAL=3
>   # test if one of the ancestors goes over its limit, the proces will be killed
>   testcase_1()
>   {
> -	echo 1 > memory.use_hierarchy
> +        if [ "$root_memory_use_hierarchy" != "1" ]; then
> +                echo 1 > memory.use_hierarchy
> +        fi
>   	echo $PAGESIZE > memory.limit_in_bytes
>   
>   	mkdir subgroup
> @@ -48,6 +50,10 @@ testcase_1()
>   # test Enabling will fail if the cgroup already has other cgroups
>   testcase_2()
>   {
> +        if [ "$root_memory_use_hierarchy" = "1" ]; then
> +               tst_resm TCONF "root cgroup has use_hierarchy enabled, skip"
> +               return
> +        fi
>   	mkdir subgroup
>   	EXPECT_FAIL echo 1 \> memory.use_hierarchy
>   
> @@ -57,7 +63,9 @@ testcase_2()
>   # test disabling will fail if the parent cgroup has enabled hierarchy.
>   testcase_3()
>   {
> -	echo 1 > memory.use_hierarchy
> +        if [ "$root_memory_use_hierarchy" != "1" ]; then
> +               echo 1 > memory.use_hierarchy
> +        fi
>   	mkdir subgroup
>   	EXPECT_FAIL echo 0 \> subgroup/memory.use_hierarchy
>   
>
Chen, Hanxiao Sept. 21, 2020, 10:53 a.m. UTC | #2
Hi, Yang

> -----邮件原件-----
> 主题: Re: [LTP] [PATCH] memcg_use_hierarchy_test.sh: skip setting
> use_hierarchy if not available
> 
> Hi hanxiao
> 
> 
> > The precondition of this case is that we can disable use_hierarchy.
> > But some distributions such as CentOS 8 had enabled it in root cgroup
> > and hard to disabled.
[snip]
> /dev/memcg/memory.use_hierarchy" \
> > +                                    "to 0 failed"
> > +                        fi
> > +                fi
> I test this patch on centos7 and testcase2 skips. On centos7(without installing
> docker), /sys/fs/cgroup/memory/memory.use_hierarchy value is equal to 1 and I
> still can disable value for /dev/memcg/memory.use_hierarchy.
> 

The behavior above looks conflicting with https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt.

> So why not directly use /dev/memcg/memory.use_hierarchy value to judge in
> testcase after setting 0.

In setup_test from memcg_lib.sh, we actually did:
    mount -t cgroup -omemory memcg /dev/memcg
Then kernel will find a suitable cgroup root for us in cgroup1_mount.

In this case, /sys/fs/cgroup/memory/ would be the good choice.
So it's equivalent to read memory.use_hierarchy from either side.

Regards, 
- Hanxiao
> 
> Best Regards
> Yang Xu
> >   	fi
> >
> >   	ROD mkdir "/dev/memcg/$TEST_ID"
> > diff --git
> > a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
> > st.sh
> > b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
> > st.sh
> > index 4cf6b9fc2..1439b6352 100755
> > ---
> > a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
> > st.sh
> > +++ b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarch
> > +++ y_test.sh
> > @@ -34,7 +34,9 @@ TST_TOTAL=3
> >   # test if one of the ancestors goes over its limit, the proces will be killed
> >   testcase_1()
> >   {
> > -	echo 1 > memory.use_hierarchy
> > +        if [ "$root_memory_use_hierarchy" != "1" ]; then
> > +                echo 1 > memory.use_hierarchy
> > +        fi
> >   	echo $PAGESIZE > memory.limit_in_bytes
> >
> >   	mkdir subgroup
> > @@ -48,6 +50,10 @@ testcase_1()
> >   # test Enabling will fail if the cgroup already has other cgroups
> >   testcase_2()
> >   {
> > +        if [ "$root_memory_use_hierarchy" = "1" ]; then
> > +               tst_resm TCONF "root cgroup has use_hierarchy enabled,
> skip"
> > +               return
> > +        fi
> >   	mkdir subgroup
> >   	EXPECT_FAIL echo 1 \> memory.use_hierarchy
> >
> > @@ -57,7 +63,9 @@ testcase_2()
> >   # test disabling will fail if the parent cgroup has enabled hierarchy.
> >   testcase_3()
> >   {
> > -	echo 1 > memory.use_hierarchy
> > +        if [ "$root_memory_use_hierarchy" != "1" ]; then
> > +               echo 1 > memory.use_hierarchy
> > +        fi
> >   	mkdir subgroup
> >   	EXPECT_FAIL echo 0 \> subgroup/memory.use_hierarchy
> >
> >
Yang Xu Oct. 13, 2020, 3:19 a.m. UTC | #3
Hi Hanxiao

> Hi, Yang
>
>> -----邮件原件-----
>> 主题: Re: [LTP] [PATCH] memcg_use_hierarchy_test.sh: skip setting
>> use_hierarchy if not available
>>
>> Hi hanxiao
>>
>>
>>> The precondition of this case is that we can disable use_hierarchy.
>>> But some distributions such as CentOS 8 had enabled it in root cgroup
>>> and hard to disabled.
> [snip]
>> /dev/memcg/memory.use_hierarchy" \
>>> +                                    "to 0 failed"
>>> +                        fi
>>> +                fi
>> I test this patch on centos7 and testcase2 skips. On centos7(without installing
>> docker), /sys/fs/cgroup/memory/memory.use_hierarchy value is equal to 1 and I
>> still can disable value for /dev/memcg/memory.use_hierarchy.
>>
> The behavior above looks conflicting with https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt.
Yes. Centos7 behavior is different from the documentation.
>> So why not directly use /dev/memcg/memory.use_hierarchy value to judge in
>> testcase after setting 0.
> In setup_test from memcg_lib.sh, we actually did:
>     mount -t cgroup -omemory memcg /dev/memcg
> Then kernel will find a suitable cgroup root for us in cgroup1_mount.
>
> In this case, /sys/fs/cgroup/memory/ would be the good choice.
> So it's equivalent to read memory.use_hierarchy from either side.
I understand. Only a minor suggestion, please use tabs instead of spaces
at the beginning of the line.


This patch looks good to me,
Acked-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

@Li, I think this patch is ok, Do you have some comment about it?


Best Regards
Yang Xu


> Regards, 
> - Hanxiao
>> Best Regards
>> Yang Xu
>>>   	fi
>>>
>>>   	ROD mkdir "/dev/memcg/$TEST_ID"
>>> diff --git
>>> a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
>>> st.sh
>>> b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
>>> st.sh
>>> index 4cf6b9fc2..1439b6352 100755
>>> ---
>>> a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_te
>>> st.sh
>>> +++ b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarch
>>> +++ y_test.sh
>>> @@ -34,7 +34,9 @@ TST_TOTAL=3
>>>   # test if one of the ancestors goes over its limit, the proces will be killed
>>>   testcase_1()
>>>   {
>>> -	echo 1 > memory.use_hierarchy
>>> +        if [ "$root_memory_use_hierarchy" != "1" ]; then
>>> +                echo 1 > memory.use_hierarchy
>>> +        fi
>>>   	echo $PAGESIZE > memory.limit_in_bytes
>>>
>>>   	mkdir subgroup
>>> @@ -48,6 +50,10 @@ testcase_1()
>>>   # test Enabling will fail if the cgroup already has other cgroups
>>>   testcase_2()
>>>   {
>>> +        if [ "$root_memory_use_hierarchy" = "1" ]; then
>>> +               tst_resm TCONF "root cgroup has use_hierarchy enabled,
>> skip"
>>> +               return
>>> +        fi
>>>   	mkdir subgroup
>>>   	EXPECT_FAIL echo 1 \> memory.use_hierarchy
>>>
>>> @@ -57,7 +63,9 @@ testcase_2()
>>>   # test disabling will fail if the parent cgroup has enabled hierarchy.
>>>   testcase_3()
>>>   {
>>> -	echo 1 > memory.use_hierarchy
>>> +        if [ "$root_memory_use_hierarchy" != "1" ]; then
>>> +               echo 1 > memory.use_hierarchy
>>> +        fi
>>>   	mkdir subgroup
>>>   	EXPECT_FAIL echo 0 \> subgroup/memory.use_hierarchy
>>>
>>>
Li Wang Oct. 14, 2020, 6:27 a.m. UTC | #4
On Tue, Oct 13, 2020 at 11:19 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:

> Hi Hanxiao
>
> > Hi, Yang
> >
> >> -----閭欢鍘熶欢-----
> >> 涓婚: Re: [LTP] [PATCH] memcg_use_hierarchy_test.sh: skip setting
> >> use_hierarchy if not available
> >>
> >> Hi hanxiao
> >>
> >>
> >>> The precondition of this case is that we can disable use_hierarchy.
> >>> But some distributions such as CentOS 8 had enabled it in root cgroup
> >>> and hard to disabled.
> > [snip]
> >> /dev/memcg/memory.use_hierarchy" \
> >>> +                                    "to 0 failed"
> >>> +                        fi
> >>> +                fi
> >> I test this patch on centos7 and testcase2 skips. On centos7(without
> installing
> >> docker), /sys/fs/cgroup/memory/memory.use_hierarchy value is equal to 1
> and I
> >> still can disable value for /dev/memcg/memory.use_hierarchy.
> >>
> > The behavior above looks conflicting with
> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt.
> Yes. Centos7 behavior is different from the documentation.
> >> So why not directly use /dev/memcg/memory.use_hierarchy value to judge
> in
> >> testcase after setting 0.
> > In setup_test from memcg_lib.sh, we actually did:
> >     mount -t cgroup -omemory memcg /dev/memcg
> > Then kernel will find a suitable cgroup root for us in cgroup1_mount.
> >
> > In this case, /sys/fs/cgroup/memory/ would be the good choice.
> > So it's equivalent to read memory.use_hierarchy from either side.
> I understand. Only a minor suggestion, please use tabs instead of spaces
> at the beginning of the line.
>
>
> This patch looks good to me,
> Acked-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>
> @Li, I think this patch is ok, Do you have some comment about it?
>

I'm ok to go with memory.use_hierarchy checking in the preconditioning
phase.

But how can you assert the default memory cgroup is mount at path:
"/sys/fs/cgroup/memory", is there any possibility the default path mount at
other places(for different distribution)?
diff mbox series

Patch

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index 22ef4f5e2..4f99c6f59 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -491,6 +491,10 @@  cleanup_test()
 		orig_memory_use_hierarchy=""
 	fi
 
+	if [ -n "$root_memory_use_hierarchy" ];then
+                root_memory_use_hierarchy=""
+	fi
+
 	killall -9 memcg_process 2>/dev/null
 	wait
 
@@ -514,16 +518,27 @@  setup_test()
 	# while there are distributions (RHEL7U0Beta for example) that sets
 	# it to 1.
 	orig_memory_use_hierarchy=$(cat /dev/memcg/memory.use_hierarchy)
+        MEMCGROUP_PATH="/sys/fs/cgroup/memory"
+        if [ -e "$MEMCGROUP_PATH" ];then
+                root_memory_use_hierarchy=$(cat "$MEMCGROUP_PATH/memory.use_hierarchy")
+        fi
+
 	if [ -z "$orig_memory_use_hierarchy" ];then
 		tst_resm TINFO "cat /dev/memcg/memory.use_hierarchy failed"
 	elif [ "$orig_memory_use_hierarchy" = "0" ];then
 		orig_memory_use_hierarchy=""
 	else
-		echo 0 > /dev/memcg/memory.use_hierarchy
-		if [ $? -ne 0 ];then
-			tst_resm TINFO "set /dev/memcg/memory.use_hierarchy" \
-				"to 0 failed"
-		fi
+                if [ "$root_memory_use_hierarchy" = "1" ]; then
+                        tst_resm TINFO "root cgroup has use_hierarchy enabled, " \
+                            "can't set /dev/memcg/memory.use_hierarchy to 0"
+                        export root_memory_use_hierarchy
+                else
+                        echo 0 > /dev/memcg/memory.use_hierarchy
+                        if [ $? -ne 0 ];then
+                                tst_resm TINFO "set /dev/memcg/memory.use_hierarchy" \
+                                    "to 0 failed"
+                        fi
+                fi
 	fi
 
 	ROD mkdir "/dev/memcg/$TEST_ID"
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
index 4cf6b9fc2..1439b6352 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_use_hierarchy_test.sh
@@ -34,7 +34,9 @@  TST_TOTAL=3
 # test if one of the ancestors goes over its limit, the proces will be killed
 testcase_1()
 {
-	echo 1 > memory.use_hierarchy
+        if [ "$root_memory_use_hierarchy" != "1" ]; then
+                echo 1 > memory.use_hierarchy
+        fi
 	echo $PAGESIZE > memory.limit_in_bytes
 
 	mkdir subgroup
@@ -48,6 +50,10 @@  testcase_1()
 # test Enabling will fail if the cgroup already has other cgroups
 testcase_2()
 {
+        if [ "$root_memory_use_hierarchy" = "1" ]; then
+               tst_resm TCONF "root cgroup has use_hierarchy enabled, skip"
+               return
+        fi
 	mkdir subgroup
 	EXPECT_FAIL echo 1 \> memory.use_hierarchy
 
@@ -57,7 +63,9 @@  testcase_2()
 # test disabling will fail if the parent cgroup has enabled hierarchy.
 testcase_3()
 {
-	echo 1 > memory.use_hierarchy
+        if [ "$root_memory_use_hierarchy" != "1" ]; then
+               echo 1 > memory.use_hierarchy
+        fi
 	mkdir subgroup
 	EXPECT_FAIL echo 0 \> subgroup/memory.use_hierarchy