[LTP] [PATCH v1] controllers.memcg_regression: add trap to clean up directories

[LTP] [PATCH v1] controllers.memcg_regression: add trap to clean up directories

[Edward Liaw via ltp]

The memcg_regression test creates a memcg/ directory that can be left
behind if the test exits unexpectedly.  Use a trap to clean up the
directories on exit.

Signed-off-by: Edward Liaw <edliaw@google.com>
---
 .../memcg/regression/memcg_regression_test.sh | 20 +++++++++++--------
 .../memcg/regression/memcg_test_4.sh          | 11 ++++++----
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
index c91a4069e..2467ae0e6 100755
--- a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
+++ b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
@@ -54,6 +54,15 @@ nr_null=0
 nr_warning=0
 nr_lockdep=0
 
+clean_up()
+{
+	# remove the cgroup
+	rmdir memcg/0 2> /dev/null
+	# unmount cgroup if still mounted
+	umount memcg/
+	rmdir memcg/
+}
+
 # check_kernel_bug - check if some kind of kernel bug happened
 check_kernel_bug()
 {
@@ -102,12 +111,12 @@ check_kernel_bug()
 #---------------------------------------------------------------------------
 test_1()
 {
-	mkdir memcg/0/
+	mkdir memcg/0
 	echo 0 > memcg/0/memory.limit_in_bytes
 
 	./memcg_test_1
 
-	rmdir memcg/0/
+	rmdir memcg/0
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
@@ -211,14 +220,12 @@ test_4()
 	killall -9 memcg_test_4 2> /dev/null
 	killall -9 memcg_test_4.sh 2> /dev/null
 
-	# if test_4.sh gets killed, it won't clean cgroup it created
-	rmdir memcg/0 2> /dev/null
-
 	swapon -a
 }
 
 # main
 failed=0
+trap clean_up EXIT
 mkdir memcg/
 
 for cur in $(seq 1 $TST_TOTAL); do
@@ -236,7 +243,4 @@ for cur in $(seq 1 $TST_TOTAL); do
 	umount memcg/
 done
 
-rmdir memcg/
-
 exit $failed
-
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
index 620031366..d002601f1 100755
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
@@ -22,6 +22,13 @@
 ##                                                                            ##
 ################################################################################
 
+clean_up()
+{
+	# remove the cgroup when exiting
+	rmdir memcg/0
+}
+
+trap clean_up EXIT
 # attach current task to memcg/0/
 mkdir memcg/0
 echo $$ > memcg/0/tasks
@@ -42,7 +49,3 @@ swapoff -a
 sleep 1
 echo $pid > memcg/tasks 2> /dev/null
 echo $$ > memcg/tasks 2> /dev/null
-
-# now remove the cgroup
-rmdir memcg/0
-
-- 
2.37.0.rc0.161.g10f37bed90-goog


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] controllers.memcg_regression: add trap to clean up directories

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 3454 bytes --]

Hi Edward,

Thanks for your patch and I believe it makes sense.
But the whole memcg test has been re-written by Luke's patchset which is
still reviewing.
https://lists.linux.it/pipermail/ltp/2022-April/028777.html

@Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
Luke, I guess V3 should be the final version (if you take the time to do
rebase/modification) and we'll happy to help merge those patches next:).



On Sat, Jul 2, 2022 at 5:38 AM Edward Liaw via ltp <ltp@lists.linux.it>
wrote:

> The memcg_regression test creates a memcg/ directory that can be left
> behind if the test exits unexpectedly.  Use a trap to clean up the
> directories on exit.
>
> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
>  .../memcg/regression/memcg_regression_test.sh | 20 +++++++++++--------
>  .../memcg/regression/memcg_test_4.sh          | 11 ++++++----
>  2 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git
> a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> index c91a4069e..2467ae0e6 100755
> ---
> a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> +++
> b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> @@ -54,6 +54,15 @@ nr_null=0
>  nr_warning=0
>  nr_lockdep=0
>
> +clean_up()
> +{
> +       # remove the cgroup
> +       rmdir memcg/0 2> /dev/null
> +       # unmount cgroup if still mounted
> +       umount memcg/
> +       rmdir memcg/
> +}
> +
>  # check_kernel_bug - check if some kind of kernel bug happened
>  check_kernel_bug()
>  {
> @@ -102,12 +111,12 @@ check_kernel_bug()
>
>  #---------------------------------------------------------------------------
>  test_1()
>  {
> -       mkdir memcg/0/
> +       mkdir memcg/0
>         echo 0 > memcg/0/memory.limit_in_bytes
>
>         ./memcg_test_1
>
> -       rmdir memcg/0/
> +       rmdir memcg/0
>
>         check_kernel_bug
>         if [ $? -eq 1 ]; then
> @@ -211,14 +220,12 @@ test_4()
>         killall -9 memcg_test_4 2> /dev/null
>         killall -9 memcg_test_4.sh 2> /dev/null
>
> -       # if test_4.sh gets killed, it won't clean cgroup it created
> -       rmdir memcg/0 2> /dev/null
> -
>         swapon -a
>  }
>
>  # main
>  failed=0
> +trap clean_up EXIT
>  mkdir memcg/
>
>  for cur in $(seq 1 $TST_TOTAL); do
> @@ -236,7 +243,4 @@ for cur in $(seq 1 $TST_TOTAL); do
>         umount memcg/
>  done
>
> -rmdir memcg/
> -
>  exit $failed
> -
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> index 620031366..d002601f1 100755
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> @@ -22,6 +22,13 @@
>  ##
>     ##
>
>  ################################################################################
>
> +clean_up()
> +{
> +       # remove the cgroup when exiting
> +       rmdir memcg/0
> +}
> +
> +trap clean_up EXIT
>  # attach current task to memcg/0/
>  mkdir memcg/0
>  echo $$ > memcg/0/tasks
> @@ -42,7 +49,3 @@ swapoff -a
>  sleep 1
>  echo $pid > memcg/tasks 2> /dev/null
>  echo $$ > memcg/tasks 2> /dev/null
> -
> -# now remove the cgroup
> -rmdir memcg/0
> -
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 5380 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] controllers.memcg_regression: add trap to clean up directories

[Luke Nowakowski-Krijger]

[-- Attachment #1.1: Type: text/plain, Size: 3818 bytes --]

Hey,

On Tue, Jul 5, 2022 at 11:00 PM Li Wang <liwang@redhat.com> wrote:

> Hi Edward,
>
> Thanks for your patch and I believe it makes sense.
> But the whole memcg test has been re-written by Luke's patchset which is
> still reviewing.
> https://lists.linux.it/pipermail/ltp/2022-April/028777.html
>
> @Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> Luke, I guess V3 should be the final version (if you take the time to do
> rebase/modification) and we'll happy to help merge those patches next:).
>
>
Let me finally get around to putting up the (hopefully) final revision so
that people don't start doing things I already did :)


>
>
> On Sat, Jul 2, 2022 at 5:38 AM Edward Liaw via ltp <ltp@lists.linux.it>
> wrote:
>
>> The memcg_regression test creates a memcg/ directory that can be left
>> behind if the test exits unexpectedly.  Use a trap to clean up the
>> directories on exit.
>>
>> Signed-off-by: Edward Liaw <edliaw@google.com>
>> ---
>>  .../memcg/regression/memcg_regression_test.sh | 20 +++++++++++--------
>>  .../memcg/regression/memcg_test_4.sh          | 11 ++++++----
>>  2 files changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git
>> a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
>> b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
>> index c91a4069e..2467ae0e6 100755
>> ---
>> a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
>> +++
>> b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
>> @@ -54,6 +54,15 @@ nr_null=0
>>  nr_warning=0
>>  nr_lockdep=0
>>
>> +clean_up()
>> +{
>> +       # remove the cgroup
>> +       rmdir memcg/0 2> /dev/null
>> +       # unmount cgroup if still mounted
>> +       umount memcg/
>> +       rmdir memcg/
>> +}
>> +
>>  # check_kernel_bug - check if some kind of kernel bug happened
>>  check_kernel_bug()
>>  {
>> @@ -102,12 +111,12 @@ check_kernel_bug()
>>
>>  #---------------------------------------------------------------------------
>>  test_1()
>>  {
>> -       mkdir memcg/0/
>> +       mkdir memcg/0
>>         echo 0 > memcg/0/memory.limit_in_bytes
>>
>>         ./memcg_test_1
>>
>> -       rmdir memcg/0/
>> +       rmdir memcg/0
>>
>>         check_kernel_bug
>>         if [ $? -eq 1 ]; then
>> @@ -211,14 +220,12 @@ test_4()
>>         killall -9 memcg_test_4 2> /dev/null
>>         killall -9 memcg_test_4.sh 2> /dev/null
>>
>> -       # if test_4.sh gets killed, it won't clean cgroup it created
>> -       rmdir memcg/0 2> /dev/null
>> -
>>         swapon -a
>>  }
>>
>>  # main
>>  failed=0
>> +trap clean_up EXIT
>>  mkdir memcg/
>>
>>  for cur in $(seq 1 $TST_TOTAL); do
>> @@ -236,7 +243,4 @@ for cur in $(seq 1 $TST_TOTAL); do
>>         umount memcg/
>>  done
>>
>> -rmdir memcg/
>> -
>>  exit $failed
>> -
>> diff --git
>> a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
>> b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
>> index 620031366..d002601f1 100755
>> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
>> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
>> @@ -22,6 +22,13 @@
>>  ##
>>       ##
>>
>>  ################################################################################
>>
>> +clean_up()
>> +{
>> +       # remove the cgroup when exiting
>> +       rmdir memcg/0
>> +}
>> +
>> +trap clean_up EXIT
>>  # attach current task to memcg/0/
>>  mkdir memcg/0
>>  echo $$ > memcg/0/tasks
>> @@ -42,7 +49,3 @@ swapoff -a
>>  sleep 1
>>  echo $pid > memcg/tasks 2> /dev/null
>>  echo $$ > memcg/tasks 2> /dev/null
>> -
>> -# now remove the cgroup
>> -rmdir memcg/0
>> -
>> --
>> 2.37.0.rc0.161.g10f37bed90-goog
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
>>
>
> --
> Regards,
> Li Wang
>

- Luke

[-- Attachment #1.2: Type: text/html, Size: 5971 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] controllers.memcg_regression: add trap to clean up directories

[Petr Vorel]

Hi all,

> Hey,

> On Tue, Jul 5, 2022 at 11:00 PM Li Wang <liwang@redhat.com> wrote:

> > Hi Edward,

> > Thanks for your patch and I believe it makes sense.
> > But the whole memcg test has been re-written by Luke's patchset which is
> > still reviewing.
> > https://lists.linux.it/pipermail/ltp/2022-April/028777.html

> > @Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> > Luke, I guess V3 should be the final version (if you take the time to do
> > rebase/modification) and we'll happy to help merge those patches next:).


> Let me finally get around to putting up the (hopefully) final revision so
> that people don't start doing things I already did :)

Thanks Luke!

Edward, FYI Luke fixed this in his big cleanup
https://patchwork.ozlabs.org/project/ltp/list/?series=311414&state=*

Therefore closing this in patchwork as "Not Applicable".

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp