All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection
       [not found] <31564d54-4fea-4450-997d-45e14c4957f4.jinguojie.jgj@alibaba-inc.com>
@ 2024-12-19  6:08 ` Jin Guojie
  2024-12-24  2:40   ` Wei Gao via ltp
  2024-12-25 11:38   ` Li Wang
  0 siblings, 2 replies; 5+ messages in thread
From: Jin Guojie @ 2024-12-19  6:08 UTC (permalink / raw)
  To: ltp; +Cc: Michal Koutný

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

In Patch v1[1], the role of recursive_protection in cgoup v2 was not considered.

By carefully reading the algorithm in the kernel function
effective_protection(). When memory_recursiveprot is enabled, a
subgroup with usage > 0 can get unclaimed greater than 0.

The purpose of doing this should be to achieve the essential purpose
of recursive_protection: the sum of all subgroups' unprotected values
is equal to parent's unprotected values.

Even though the documentation does not give an explicit description
for recursive_memoryprot, it looks like the kernel's processing
algorithm is reasonable.

Based on the idea of [2], Patch v2 is rewritten to determine whether
memory_recursiveprot is enabled.

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:211: TPASS: Expect: (C low events=966) > 0
memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
memcontrol04.c:211: TPASS: Expect: (D low events=966) > 0
memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
memcontrol04.c:214: TPASS: Expect: (E low events=0) == 0
memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
memcontrol04.c:211: TPASS: Expect: (F low events=874) > 0

[1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
[2] https://lore.kernel.org/ltp/20220222144511.GA12037@blackbody.suse.cz/

Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Michal Koutný <mkoutny@suse.com>
---
 include/tst_cgroup.h                              |  2 ++
 lib/tst_cgroup.c                                  | 12 ++++++++++++
 testcases/kernel/controllers/memcg/memcontrol04.c |  2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

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..9e3b21ed0 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;
+
+       int memory_recursiveprot:1;
 };

 /* Controller sub-systems */
@@ -592,6 +594,7 @@ static void cgroup_root_scan(const char *const mnt_type,
        }
        for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
                nsdelegate |= !strcmp("nsdelegate", tok);
+               root->memory_recursiveprot |=
!strcmp("memory_recursiveprot", tok);
        }

        if (root->ver && ctrl_field == root->ctrl_field)
@@ -718,6 +721,7 @@ mount:
                tst_res(TINFO, "Mounted V2 CGroups on %s", mnt_path);
                tst_cg_scan();
                roots[0].we_mounted_it = 1;
+               roots[0].memory_recursiveprot = 1;
                return;
        }

@@ -1509,3 +1513,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..9e9d6ab6e 100644
--- a/testcases/kernel/controllers/memcg/memcontrol04.c
+++ b/testcases/kernel/controllers/memcg/memcontrol04.c
@@ -207,7 +207,7 @@ static void test_memcg_low(void)

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

-               if (i < E) {
+               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 {
--
2.34.1

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection
  2024-12-19  6:08 ` [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection Jin Guojie
@ 2024-12-24  2:40   ` Wei Gao via ltp
  2024-12-25 11:38   ` Li Wang
  1 sibling, 0 replies; 5+ messages in thread
From: Wei Gao via ltp @ 2024-12-24  2:40 UTC (permalink / raw)
  To: Jin Guojie; +Cc: Michal Koutný, ltp

On Thu, Dec 19, 2024 at 02:08:59PM +0800, Jin Guojie wrote:
> V2:
> * Change the expected events in F depending on memory_recursiveprot
> 
> In Patch v1[1], the role of recursive_protection in cgoup v2 was not considered.
> 
> By carefully reading the algorithm in the kernel function
> effective_protection(). When memory_recursiveprot is enabled, a
> subgroup with usage > 0 can get unclaimed greater than 0.
> 
> The purpose of doing this should be to achieve the essential purpose
> of recursive_protection: the sum of all subgroups' unprotected values
> is equal to parent's unprotected values.
> 
> Even though the documentation does not give an explicit description
> for recursive_memoryprot, it looks like the kernel's processing
> algorithm is reasonable.
> 
> Based on the idea of [2], Patch v2 is rewritten to determine whether
> memory_recursiveprot is enabled.
> 
> 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:211: TPASS: Expect: (C low events=966) > 0
> memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
> memcontrol04.c:211: TPASS: Expect: (D low events=966) > 0
> memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
> memcontrol04.c:214: TPASS: Expect: (E low events=0) == 0
> memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
> memcontrol04.c:211: TPASS: Expect: (F low events=874) > 0
> 
> [1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
> [2] https://lore.kernel.org/ltp/20220222144511.GA12037@blackbody.suse.cz/
> 
> Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Michal Koutný <mkoutny@suse.com>
> ---
>  include/tst_cgroup.h                              |  2 ++
>  lib/tst_cgroup.c                                  | 12 ++++++++++++
>  testcases/kernel/controllers/memcg/memcontrol04.c |  2 +-
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> 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..9e3b21ed0 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;
> +
> +       int memory_recursiveprot:1;
>  };
> 
>  /* Controller sub-systems */
> @@ -592,6 +594,7 @@ static void cgroup_root_scan(const char *const mnt_type,
>         }
>         for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
>                 nsdelegate |= !strcmp("nsdelegate", tok);
> +               root->memory_recursiveprot |=
> !strcmp("memory_recursiveprot", tok);
>         }
> 
>         if (root->ver && ctrl_field == root->ctrl_field)
> @@ -718,6 +721,7 @@ mount:
>                 tst_res(TINFO, "Mounted V2 CGroups on %s", mnt_path);
>                 tst_cg_scan();
>                 roots[0].we_mounted_it = 1;
> +               roots[0].memory_recursiveprot = 1;
>                 return;
>         }
> 
> @@ -1509,3 +1513,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..9e9d6ab6e 100644
> --- a/testcases/kernel/controllers/memcg/memcontrol04.c
> +++ b/testcases/kernel/controllers/memcg/memcontrol04.c
> @@ -207,7 +207,7 @@ static void test_memcg_low(void)
> 
>                 TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id, oom);
> 
> -               if (i < E) {
> +               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 {
> --
Maybe just give a warning instead of error if event of F > 0, since event of F > 0 has no
real impact on end user, at the same time maybe we will get a fix in kernel side to correct
this behavior.
> 2.34.1
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection
  2024-12-19  6:08 ` [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection Jin Guojie
  2024-12-24  2:40   ` Wei Gao via ltp
@ 2024-12-25 11:38   ` Li Wang
  2024-12-25 11:56     ` Li Wang
  2025-01-09  6:53     ` Li Wang
  1 sibling, 2 replies; 5+ messages in thread
From: Li Wang @ 2024-12-25 11:38 UTC (permalink / raw)
  To: Jin Guojie; +Cc: Michal Koutný, ltp, Richard Palethorpe

Hi Jin,

On Thu, Dec 19, 2024 at 2:09 PM Jin Guojie <guojie.jin@gmail.com> wrote:

> V2:
> * Change the expected events in F depending on memory_recursiveprot
>
> In Patch v1[1], the role of recursive_protection in cgoup v2 was not
> considered.
>
> By carefully reading the algorithm in the kernel function
> effective_protection(). When memory_recursiveprot is enabled, a
> subgroup with usage > 0 can get unclaimed greater than 0.
>
> The purpose of doing this should be to achieve the essential purpose
> of recursive_protection: the sum of all subgroups' unprotected values
> is equal to parent's unprotected values.
>
> Even though the documentation does not give an explicit description
> for recursive_memoryprot, it looks like the kernel's processing
> algorithm is reasonable.
>
> Based on the idea of [2], Patch v2 is rewritten to determine whether
> memory_recursiveprot is enabled.
>
> 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:211: TPASS: Expect: (C low events=966) > 0
> memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
> memcontrol04.c:211: TPASS: Expect: (D low events=966) > 0
> memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
> memcontrol04.c:214: TPASS: Expect: (E low events=0) == 0
> memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
> memcontrol04.c:211: TPASS: Expect: (F low events=874) > 0
>
> [1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
> [2] https://lore.kernel.org/ltp/20220222144511.GA12037@blackbody.suse.cz/
>
> Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Michal Koutný <mkoutny@suse.com>
> ---
>  include/tst_cgroup.h                              |  2 ++
>  lib/tst_cgroup.c                                  | 12 ++++++++++++
>  testcases/kernel/controllers/memcg/memcontrol04.c |  2 +-
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> 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..9e3b21ed0 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;
> +
> +       int memory_recursiveprot:1;
>

I prefer to move it up to put it together with the nsdelegate.



>  };
>
>  /* Controller sub-systems */
> @@ -592,6 +594,7 @@ static void cgroup_root_scan(const char *const
> mnt_type,
>         }
>         for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
>                 nsdelegate |= !strcmp("nsdelegate", tok);
> +               root->memory_recursiveprot |=
> !strcmp("memory_recursiveprot", tok);
>


Why not define a single variable memory_recursiveprot to save the value
and move the root->memory_recursiveprot assignment to the "backref" part?
(just like what we did for nsdelegate)



>         }
>
>         if (root->ver && ctrl_field == root->ctrl_field)
> @@ -718,6 +721,7 @@ mount:
>                 tst_res(TINFO, "Mounted V2 CGroups on %s", mnt_path);
>                 tst_cg_scan();
>                 roots[0].we_mounted_it = 1;
> +               roots[0].memory_recursiveprot = 1;
>

This is completely wrong, as the first mount operation may fail,
while CGroupV2 falls back to the default mount and succeeds.

Hence we should remove the line here, as tst_cg_scan() helps
to automatically detect and set "root->memory_recursiveprot"
to the correct value.



>                 return;
>         }
>
> @@ -1509,3 +1513,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..9e9d6ab6e 100644
> --- a/testcases/kernel/controllers/memcg/memcontrol04.c
> +++ b/testcases/kernel/controllers/memcg/memcontrol04.c
> @@ -207,7 +207,7 @@ static void test_memcg_low(void)
>
>                 TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id,
> oom);
>
> -               if (i < E) {
> +               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 {
> --
> 2.34.1
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>


-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection
  2024-12-25 11:38   ` Li Wang
@ 2024-12-25 11:56     ` Li Wang
  2025-01-09  6:53     ` Li Wang
  1 sibling, 0 replies; 5+ messages in thread
From: Li Wang @ 2024-12-25 11:56 UTC (permalink / raw)
  To: Jin Guojie; +Cc: Michal Koutný, ltp, Richard Palethorpe

Hi All,

There is another problem in that tst_cgroup lib that
the definition can lead to -1 during the | operation.

Because If the field is uninitialized (contains garbage),
it could hold 0b1, which is interpreted as -1 in signed arithmetic.

At least we need separate fix:

--- 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;
 };


-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection
  2024-12-25 11:38   ` Li Wang
  2024-12-25 11:56     ` Li Wang
@ 2025-01-09  6:53     ` Li Wang
  1 sibling, 0 replies; 5+ messages in thread
From: Li Wang @ 2025-01-09  6:53 UTC (permalink / raw)
  To: Jin Guojie; +Cc: Michal Koutný, ltp, Richard Palethorpe

Hi Jin,

Could you help resend a new patch version based on the review comments?
(and rebase the code to the latest branch to avoid patch conflict errors)

It makes a lot of sense to merge your work before the new LTP release.



On Wed, Dec 25, 2024 at 7:38 PM Li Wang <liwang@redhat.com> wrote:

> Hi Jin,
>
> On Thu, Dec 19, 2024 at 2:09 PM Jin Guojie <guojie.jin@gmail.com> wrote:
>
>> V2:
>> * Change the expected events in F depending on memory_recursiveprot
>>
>> In Patch v1[1], the role of recursive_protection in cgoup v2 was not
>> considered.
>>
>> By carefully reading the algorithm in the kernel function
>> effective_protection(). When memory_recursiveprot is enabled, a
>> subgroup with usage > 0 can get unclaimed greater than 0.
>>
>> The purpose of doing this should be to achieve the essential purpose
>> of recursive_protection: the sum of all subgroups' unprotected values
>> is equal to parent's unprotected values.
>>
>> Even though the documentation does not give an explicit description
>> for recursive_memoryprot, it looks like the kernel's processing
>> algorithm is reasonable.
>>
>> Based on the idea of [2], Patch v2 is rewritten to determine whether
>> memory_recursiveprot is enabled.
>>
>> 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:211: TPASS: Expect: (C low events=966) > 0
>> memcontrol04.c:208: TPASS: Expect: (D oom events=0) == 0
>> memcontrol04.c:211: TPASS: Expect: (D low events=966) > 0
>> memcontrol04.c:208: TPASS: Expect: (E oom events=0) == 0
>> memcontrol04.c:214: TPASS: Expect: (E low events=0) == 0
>> memcontrol04.c:208: TPASS: Expect: (F oom events=0) == 0
>> memcontrol04.c:211: TPASS: Expect: (F low events=874) > 0
>>
>> [1] https://lists.linux.it/pipermail/ltp/2024-November/040946.html
>> [2] https://lore.kernel.org/ltp/20220222144511.GA12037@blackbody.suse.cz/
>>
>> Signed-off-by: Jin Guojie <guojie.jin@gmail.com>
>> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
>> Suggested-by: Michal Koutný <mkoutny@suse.com>
>> ---
>>  include/tst_cgroup.h                              |  2 ++
>>  lib/tst_cgroup.c                                  | 12 ++++++++++++
>>  testcases/kernel/controllers/memcg/memcontrol04.c |  2 +-
>>  3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> 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..9e3b21ed0 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;
>> +
>> +       int memory_recursiveprot:1;
>>
>
> I prefer to move it up to put it together with the nsdelegate.
>
>
>
>>  };
>>
>>  /* Controller sub-systems */
>> @@ -592,6 +594,7 @@ static void cgroup_root_scan(const char *const
>> mnt_type,
>>         }
>>         for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ",")) {
>>                 nsdelegate |= !strcmp("nsdelegate", tok);
>> +               root->memory_recursiveprot |=
>> !strcmp("memory_recursiveprot", tok);
>>
>
>
> Why not define a single variable memory_recursiveprot to save the value
> and move the root->memory_recursiveprot assignment to the "backref" part?
> (just like what we did for nsdelegate)
>
>
>
>>         }
>>
>>         if (root->ver && ctrl_field == root->ctrl_field)
>> @@ -718,6 +721,7 @@ mount:
>>                 tst_res(TINFO, "Mounted V2 CGroups on %s", mnt_path);
>>                 tst_cg_scan();
>>                 roots[0].we_mounted_it = 1;
>> +               roots[0].memory_recursiveprot = 1;
>>
>
> This is completely wrong, as the first mount operation may fail,
> while CGroupV2 falls back to the default mount and succeeds.
>
> Hence we should remove the line here, as tst_cg_scan() helps
> to automatically detect and set "root->memory_recursiveprot"
> to the correct value.
>
>
>
>>                 return;
>>         }
>>
>> @@ -1509,3 +1513,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..9e9d6ab6e 100644
>> --- a/testcases/kernel/controllers/memcg/memcontrol04.c
>> +++ b/testcases/kernel/controllers/memcg/memcontrol04.c
>> @@ -207,7 +207,7 @@ static void test_memcg_low(void)
>>
>>                 TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id,
>> oom);
>>
>> -               if (i < E) {
>> +               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 {
>> --
>> 2.34.1
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
>
>
> --
> Regards,
> Li Wang
>


-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-01-09  6:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <31564d54-4fea-4450-997d-45e14c4957f4.jinguojie.jgj@alibaba-inc.com>
2024-12-19  6:08 ` [LTP] [PATCH v2] memcg/memcontrol04: Fix judgment for recursive_protection Jin Guojie
2024-12-24  2:40   ` Wei Gao via ltp
2024-12-25 11:38   ` Li Wang
2024-12-25 11:56     ` Li Wang
2025-01-09  6:53     ` Li Wang

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.