[LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[Jin Guojie]

V3:
* Fix initialization of root->memory_recursiveprot
* The use of tst_cg_memory_recursiveprot(leaf_cg[F]) is changed to safe check
* The type of memory_recursiveprot is changed to unsigned int
* Rebase the code to the latest branch

V2:
* Change the expected events in F depending on memory_recursiveprot

On distributions with memory_recursiveprot enabled by default (from
Ubuntu 22.04 to 24.10), running this passes：

memcontrol04.c:208: TPASS: Expect: (C oom events=0) == 0
memcontrol04.c:214: TPASS: Expect: (C low events=964) > 0
memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
memcontrol04.c:214: TPASS: Expect: (D low events=964) > 0
memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
memcontrol04.c:211: TPASS: Expect: (E low events=0) == 0
memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
memcontrol04.c:214: TPASS: Expect: (F low events=878) > 0

[1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
[2] https://lists.linux.it/pipermail/ltp/2024-December/041316.html

Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Michal Koutný <mkoutny@suse.com>
Suggested-by: Wei Gao <wegao@suse.com>
Suggested-by: Li Wang <liwang@redhat.com>
---
 include/tst_cgroup.h                              |  2 ++
 lib/tst_cgroup.c                                  | 13 +++++++++++++
 testcases/kernel/controllers/memcg/memcontrol04.c |  8 ++++----
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index d23a8e652..068ff8306 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -256,4 +256,6 @@ int safe_cg_occursin(const char *file, const int lineno,
                         const char *const file_name,
                         const char *const needle);

+int tst_cg_memory_recursiveprot(struct tst_cg_group *cg);
+
 #endif /* TST_CGROUP_H */
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 6055015eb..1f9354ba4 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -76,6 +76,8 @@ struct cgroup_root {
        int we_mounted_it:1;
        /* cpuset is in compatability mode */
        int no_cpuset_prefix:1;
+
+       unsigned int memory_recursiveprot:1;
 };

 /* Controller sub-systems */
@@ -576,6 +578,7 @@ static void cgroup_root_scan(const char *const mnt_type,
        uint32_t ctrl_field = 0;
        int no_prefix = 0;
        int nsdelegate = 0;
+       int memory_recursiveprot = 0;
        char buf[BUFSIZ];
        char *tok;
        const int mnt_dfd = SAFE_OPEN(mnt_dir, O_PATH | O_DIRECTORY);
@@ -592,6 +595,7 @@ static void cgroup_root_scan(const char *const mnt_type,
        }
        for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
                nsdelegate |= !strcmp("nsdelegate", tok);
+               memory_recursiveprot |= !strcmp("memory_recursiveprot", tok);
        }

        if (root->ver && ctrl_field == root->ctrl_field)
@@ -644,6 +648,7 @@ backref:
        root->ctrl_field = ctrl_field;
        root->no_cpuset_prefix = no_prefix;
        root->nsdelegate = nsdelegate;
+       root->memory_recursiveprot = memory_recursiveprot;

        for_each_ctrl(ctrl) {
                if (has_ctrl(root->ctrl_field, ctrl))
@@ -1509,3 +1514,11 @@ int safe_cg_occursin(const char *const file,
const int lineno,

        return !!strstr(buf, needle);
 }
+
+int tst_cg_memory_recursiveprot(struct tst_cg_group *cg)
+{
+       if (cg && cg->dirs_by_ctrl[0]->dir_root)
+               return cg->dirs_by_ctrl[0]->dir_root->memory_recursiveprot;
+       return 0;
+}
+
diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c
b/testcases/kernel/controllers/memcg/memcontrol04.c
index 1b8d115f8..0a77f6681 100644
--- a/testcases/kernel/controllers/memcg/memcontrol04.c
+++ b/testcases/kernel/controllers/memcg/memcontrol04.c
@@ -207,12 +207,12 @@ static void test_memcg_low(void)

                TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id, oom);

-               if (i < E) {
-                       TST_EXP_EXPR(low > 0,
-                                    "(%c low events=%ld) > 0", id, low);
-               } else {
+               if (i == E || ((i == F) &&
!tst_cg_memory_recursiveprot(leaf_cg[F]))) {
                        TST_EXP_EXPR(low == 0,
                                     "(%c low events=%ld) == 0", id, low);
+               } else {
+                       TST_EXP_EXPR(low > 0,
+                                    "(%c low events=%ld) > 0", id, low);
                }
        }

--
2.34.1

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

Re: [LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[Li Wang]

On Wed, Jan 15, 2025 at 2:33 PM Jin Guojie <guojie.jin@gmail.com> wrote:

> V3:
> * Fix initialization of root->memory_recursiveprot
> * The use of tst_cg_memory_recursiveprot(leaf_cg[F]) is changed to safe
> check
> * The type of memory_recursiveprot is changed to unsigned int
> * Rebase the code to the latest branch
>

Thanks for refining this, code looks good to me, but as I pointed out
can you rebase the patch on the latest branch? Otherwise, maintainer
gets a conflict when applying your patch.

Reviewed-by: Li Wang <liwang@redhat.com>

@Cyril, @Petr, I vote to merge this one before the release, and I also think
we need an additional tiny fix like below:

--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -44,7 +44,7 @@ struct cgroup_dir {
         */
        int dir_fd;

-       int we_created_it:1;
+       unsigned int we_created_it:1;
 };

 /* The root of a CGroup hierarchy/tree */
@@ -71,12 +71,12 @@ struct cgroup_root {
        /* CGroup for current test. Which may have children. */
        struct cgroup_dir test_dir;

-       int nsdelegate:1;
-       int memory_recursiveprot:1;
+       unsigned int nsdelegate:1;
+       unsigned int memory_recursiveprot:1;

-       int we_mounted_it:1;
+       unsigned int we_mounted_it:1;
        /* cpuset is in compatability mode */
-       int no_cpuset_prefix:1;
+       unsigned int no_cpuset_prefix:1;
 };




>
> V2:
> * Change the expected events in F depending on memory_recursiveprot
>
> On distributions with memory_recursiveprot enabled by default (from
> Ubuntu 22.04 to 24.10), running this passes：
>
> memcontrol04.c:208: TPASS: Expect: (C oom events=0) == 0
> memcontrol04.c:214: TPASS: Expect: (C low events=964) > 0
> memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
> memcontrol04.c:214: TPASS: Expect: (D low events=964) > 0
> memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
> memcontrol04.c:211: TPASS: Expect: (E low events=0) == 0
> memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
> memcontrol04.c:214: TPASS: Expect: (F low events=878) > 0
>
> [1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
> [2] https://lists.linux.it/pipermail/ltp/2024-December/041316.html
>
> Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Michal Koutný <mkoutny@suse.com>
> Suggested-by: Wei Gao <wegao@suse.com>
> Suggested-by: Li Wang <liwang@redhat.com>
> ---
>  include/tst_cgroup.h                              |  2 ++
>  lib/tst_cgroup.c                                  | 13 +++++++++++++
>  testcases/kernel/controllers/memcg/memcontrol04.c |  8 ++++----
>  3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
> index d23a8e652..068ff8306 100644
> --- a/include/tst_cgroup.h
> +++ b/include/tst_cgroup.h
> @@ -256,4 +256,6 @@ int safe_cg_occursin(const char *file, const int
> lineno,
>                          const char *const file_name,
>                          const char *const needle);
>
> +int tst_cg_memory_recursiveprot(struct tst_cg_group *cg);
> +
>  #endif /* TST_CGROUP_H */
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 6055015eb..1f9354ba4 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -76,6 +76,8 @@ struct cgroup_root {
>         int we_mounted_it:1;
>         /* cpuset is in compatability mode */
>         int no_cpuset_prefix:1;
> +
> +       unsigned int memory_recursiveprot:1;
>  };
>
>  /* Controller sub-systems */
> @@ -576,6 +578,7 @@ static void cgroup_root_scan(const char *const
> mnt_type,
>         uint32_t ctrl_field = 0;
>         int no_prefix = 0;
>         int nsdelegate = 0;
> +       int memory_recursiveprot = 0;
>         char buf[BUFSIZ];
>         char *tok;
>         const int mnt_dfd = SAFE_OPEN(mnt_dir, O_PATH | O_DIRECTORY);
> @@ -592,6 +595,7 @@ static void cgroup_root_scan(const char *const
> mnt_type,
>         }
>         for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
>                 nsdelegate |= !strcmp("nsdelegate", tok);
> +               memory_recursiveprot |= !strcmp("memory_recursiveprot",
> tok);
>         }
>
>         if (root->ver && ctrl_field == root->ctrl_field)
> @@ -644,6 +648,7 @@ backref:
>         root->ctrl_field = ctrl_field;
>         root->no_cpuset_prefix = no_prefix;
>         root->nsdelegate = nsdelegate;
> +       root->memory_recursiveprot = memory_recursiveprot;
>
>         for_each_ctrl(ctrl) {
>                 if (has_ctrl(root->ctrl_field, ctrl))
> @@ -1509,3 +1514,11 @@ int safe_cg_occursin(const char *const file,
> const int lineno,
>
>         return !!strstr(buf, needle);
>  }
> +
> +int tst_cg_memory_recursiveprot(struct tst_cg_group *cg)
> +{
> +       if (cg && cg->dirs_by_ctrl[0]->dir_root)
> +               return cg->dirs_by_ctrl[0]->dir_root->memory_recursiveprot;
> +       return 0;
> +}
> +
> diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c
> b/testcases/kernel/controllers/memcg/memcontrol04.c
> index 1b8d115f8..0a77f6681 100644
> --- a/testcases/kernel/controllers/memcg/memcontrol04.c
> +++ b/testcases/kernel/controllers/memcg/memcontrol04.c
> @@ -207,12 +207,12 @@ static void test_memcg_low(void)
>
>                 TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id,
> oom);
>
> -               if (i < E) {
> -                       TST_EXP_EXPR(low > 0,
> -                                    "(%c low events=%ld) > 0", id, low);
> -               } else {
> +               if (i == E || ((i == F) &&
> !tst_cg_memory_recursiveprot(leaf_cg[F]))) {
>                         TST_EXP_EXPR(low == 0,
>                                      "(%c low events=%ld) == 0", id, low);
> +               } else {
> +                       TST_EXP_EXPR(low > 0,
> +                                    "(%c low events=%ld) > 0", id, low);
>                 }
>         }
>
> --
> 2.34.1
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[Petr Vorel]

Hi Li, Jin Guojie all,

> On Wed, Jan 15, 2025 at 2:33 PM Jin Guojie <guojie.jin@gmail.com> wrote:

Jin Guojie, thanks for handling this!

> > V3:
> > * Fix initialization of root->memory_recursiveprot
> > * The use of tst_cg_memory_recursiveprot(leaf_cg[F]) is changed to safe
> > check
> > * The type of memory_recursiveprot is changed to unsigned int
> > * Rebase the code to the latest branch
nit: if you add notes like version changelog (or other notes which should not be
part of the final commit message) below --- (above of the list of changed
files) we will see them in ML, but it will not be in git commit message.

> Thanks for refining this, code looks good to me, but as I pointed out
> can you rebase the patch on the latest branch? Otherwise, maintainer
> gets a conflict when applying your patch.

> Reviewed-by: Li Wang <liwang@redhat.com>

Acked-by: Petr Vorel <pvorel@suse.cz>

I also updated patchwork states for all versions of this.

> @Cyril, @Petr, I vote to merge this one before the release, and I also think

+1

> we need an additional tiny fix like below:

+1, Li, could you please send a regular patch for it?

Kind regards,
Petr

> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -44,7 +44,7 @@ struct cgroup_dir {
>          */
>         int dir_fd;

> -       int we_created_it:1;
> +       unsigned int we_created_it:1;
>  };

>  /* The root of a CGroup hierarchy/tree */
> @@ -71,12 +71,12 @@ struct cgroup_root {
>         /* CGroup for current test. Which may have children. */
>         struct cgroup_dir test_dir;

> -       int nsdelegate:1;
> -       int memory_recursiveprot:1;
> +       unsigned int nsdelegate:1;
> +       unsigned int memory_recursiveprot:1;

> -       int we_mounted_it:1;
> +       unsigned int we_mounted_it:1;
>         /* cpuset is in compatability mode */
> -       int no_cpuset_prefix:1;
> +       unsigned int no_cpuset_prefix:1;
>  };

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

Re: [LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[Li Wang]

On Wed, Jan 15, 2025 at 8:17 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li, Jin Guojie all,
>
> > On Wed, Jan 15, 2025 at 2:33 PM Jin Guojie <guojie.jin@gmail.com> wrote:
>
> Jin Guojie, thanks for handling this!
>
> > > V3:
> > > * Fix initialization of root->memory_recursiveprot
> > > * The use of tst_cg_memory_recursiveprot(leaf_cg[F]) is changed to safe
> > > check
> > > * The type of memory_recursiveprot is changed to unsigned int
> > > * Rebase the code to the latest branch
> nit: if you add notes like version changelog (or other notes which should
> not be
> part of the final commit message) below --- (above of the list of changed
> files) we will see them in ML, but it will not be in git commit message.
>
> > Thanks for refining this, code looks good to me, but as I pointed out
> > can you rebase the patch on the latest branch? Otherwise, maintainer
> > gets a conflict when applying your patch.
>
> > Reviewed-by: Li Wang <liwang@redhat.com>
>
> Acked-by: Petr Vorel <pvorel@suse.cz>
>
> I also updated patchwork states for all versions of this.
>
> > @Cyril, @Petr, I vote to merge this one before the release, and I also
> think
>
> +1
>
> > we need an additional tiny fix like below:
>
> +1, Li, could you please send a regular patch for it?
>

Yes, I will send it once Guojie gives patch v4.


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v3] memcg/memcontrol04: Fix judgment for recursive_protection

[Li Wang]

Hi Guojie,

Could have a look and send patch v4 based on the above comments?
I do want to merge this one for ltp Cgroup library :).

-- 
Regards,
Li Wang

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