<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Richard,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Thanks for the review.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 22, 2021 at 10:17 PM Richard Palethorpe <<a href="mailto:rpalethorpe@suse.de" target="_blank">rpalethorpe@suse.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Li,<br>
<br>
Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>> writes:<br>
<br>
> oom03 often gets fail while setting 'memory.swap.max = TESTMEM' in CGroup V2,<br>
> as in that scenario (lite == 1), child_alloc only start a single process to<br>
> dirty 'TESTMEM + MB' anonymous memory for testing:<br>
><br>
> testoom(, lite == 1, ,)<br>
> oom(, lite == 1, ,)<br>
> child_alloc(, lite == 1,)<br>
> alloc_mem(TESTMEM + MB, )<br>
><br>
> mem.c:224: TINFO: start normal OOM testing.<br>
> mem.c:146: TINFO: expected victim is 80466.<br>
> mem.c:38: TINFO: thread (7f411c69d740), allocating 1074790400 bytes.<br>
> mem.c:64: TINFO: swapped is 25546752 bytes. <------- swap occuring -----<br>
> mem.c:164: TFAIL: victim unexpectedly ended with retcode: 0, expected: 12<br>
><br>
> TBH, this can not really test the 'memory.swap.max' as expected, since in the<br>
> kernel side mem_cgroup_out_of_memory split OOM margin into two-part, one for<br>
> memory.max limit, another for memory.swap.max, if any of them get overflow,<br>
> then invoke out_of_memory to kill victim-process.<br>
><br>
> Theoretically, alloc_mem(TESTMEM + MB, ) should work while 'memory.max' is equal<br>
> to TESTMEM, but Cgroup v2 tracks memory and swap in separate, which splits memory<br>
> and swap counter. So with swappiness enable (default value is 60 on RHEL), it<br>
> likely has part of memory swapping out during the allocating, upon that the two<br>
> limit loss effect at the same time. Unless disable swap completely then memory.max<br>
> will take effect in precisely.<br>
><br>
> To get more opportunities to reach the swap limitation, let's scale down the<br>
> value of 'memory.swap.max' to only 1MB for CGroup v2.<br>
><br>
> But for CGroup v1, the memory.memsw.limit_in_bytes disallow to less than<br>
> memory.limit_in_bytes, so we'd better raise the child_alloc to the<br>
> twifold<br>
^twofold<br>
> of TESTMEM.<br>
<br>
Ah, this means "memory.swap.x" and "memory.memsw.x" are not really the<br>
same thing. This seems to be common pattern, so maybe we could translate<br>
V2 values to V1 in the library.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">+1 </div><div class="gmail_default" style="font-size:small">We can consider doing that in a new separate patch. And better to check</div><div class="gmail_default" style="font-size:small">more parameters to guarantee we have the correct understanding in use it:).</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
If I understand correctly `memory.swap.max = memory.memsw.limit_in_bytes<br>
- memory.limit_in_bytes`? Also "max" can be mapped to ~0UL or maybe<br>
~0ULL when -m32 is used.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">From the definition, yes, the memory.memsw.limit_in_bytes parameter represents the sum of memory and swap.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This is not important for the current patch.<br>
<br>
><br>
> Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
> ---<br>
><br>
> Notes:<br>
> v1 --> v2<br>
> * add comments for explaining why set 'memory.swap.max' to 1MB<br>
> * Scale down the value of 'memory.swap.max' to only 1MB for CGroup v2.<br>
><br>
> testcases/kernel/mem/lib/mem.c | 2 +-<br>
> testcases/kernel/mem/oom/oom03.c | 17 ++++++++++++++++-<br>
> 2 files changed, 17 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c<br>
> index 9f946b5c9..ac890491c 100644<br>
> --- a/testcases/kernel/mem/lib/mem.c<br>
> +++ b/testcases/kernel/mem/lib/mem.c<br>
> @@ -78,7 +78,7 @@ static void child_alloc(int testcase, int lite, int threads)<br>
> pthread_t *th;<br>
> <br>
> if (lite) {<br>
> - int ret = alloc_mem(TESTMEM + MB, testcase);<br>
> + int ret = alloc_mem(TESTMEM * 2 + MB, testcase);<br>
> exit(ret);<br>
> }<br>
> <br>
> diff --git a/testcases/kernel/mem/oom/oom03.c b/testcases/kernel/mem/oom/oom03.c<br>
> index 939413744..89d7711a5 100644<br>
> --- a/testcases/kernel/mem/oom/oom03.c<br>
> +++ b/testcases/kernel/mem/oom/oom03.c<br>
> @@ -46,7 +46,22 @@ static void verify_oom(void)<br>
> testoom(0, 0, ENOMEM, 1);<br>
> <br>
> if (SAFE_CGROUP_HAS(cg, "memory.swap.max")) {<br>
> - SAFE_CGROUP_PRINTF(cg, "memory.swap.max", "%lu", TESTMEM);<br>
> + /*<br>
> + * Cgroup v2 tracks memory and swap in separate, which splits<br>
> + * memory and swap counter. So with swappiness enable (default<br>
> + * value is 60 on RHEL), it likely has part of memory swapping<br>
> + * out during the allocating, upon that the two limit loss<br>
> + * effect at the same time.<br>
> + *<br>
> + * To get more opportunities to reach the swap limitation,<br>
> + * let's scale down the value of 'memory.swap.max' to only<br>
> + * 1MB for CGroup v2.<br>
> + */<br>
> + if (TST_CGROUP_VER(cg, "memory") != TST_CGROUP_V1)<br>
> + SAFE_CGROUP_PRINTF(cg, "memory.swap.max", "%lu", MB);<br>
> + else<br>
> + SAFE_CGROUP_PRINTF(cg, "memory.swap.max", "%lu", TESTMEM);<br>
> +<br>
<br>
To be consistent with V2; should this be TESTMEM + MB?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, that should be better.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> testoom(0, 1, ENOMEM, 1);<br>
> }<br>
> <br>
> -- <br>
> 2.31.1<br>
<br>
<br>
-- <br>
Thank you,<br>
Richard.<br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>