<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 3, 2019 at 9:10 PM Cyril Hrubis <<a href="mailto:chrubis@suse.cz">chrubis@suse.cz</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">Hi!<br>
> +                     if (ret == EINVAL) {<br>
>                               SAFE_KILL(cpid, SIGKILL);<br>
>                               SAFE_WAITPID(cpid, &status, 0);<br>
>                               SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);<br>
>                               tst_res(TCONF,<br>
>                                       "madvise() didn't support MADV_SOFT_OFFLINE");<br>
>                               return;<br>
> +                     } else if (ret == EBUSY) {<br>
> +                             SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);<br>
> +                             goto out;<br>
<br>
Shouldn't we continue with the test here rather than exit?<br>
<br>
I guess that there is no harm in doing a few more iterations if we<br>
manage to hit EBUSY, or is there a good reason to exit the test here?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, we can do more iterations then, but it probably makes no sense. </div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">The reason I guess is that, if we get an EBUSY on the hugepage offline, that means the page is already being isolated by move_pages() in the child at that moment and we can't really release it. So in the next iteration, the mmap() will be failed with ENOMEM(since we only have 1 huge page in /proc/.../nr_hugepages).</div></div><div> </div><div><div class="gmail_default" style="font-size:small">To confirm that, I change the code to continue after get EBUSY, but it couldn't:</div><div class="gmail_default" style="font-size:small"><br></div># ./move_pages12 <br>tst_test.c:1100: INFO: Timeout per run is 0h 05m 00s<br>move_pages12.c:251: INFO: Free RAM 30860672 kB<br>move_pages12.c:269: INFO: Increasing 2048kB hugepages pool on node 0 to 4<br>move_pages12.c:279: INFO: Increasing 2048kB hugepages pool on node 1 to 5<br>move_pages12.c:195: INFO: Allocating and freeing 4 hugepages on node 0<br>move_pages12.c:195: INFO: Allocating and freeing 4 hugepages on node 1<br>move_pages12.c:185: PASS: Bug not reproduced<br>move_pages12.c:146: CONF: Cannot allocate hugepage, memory too fragmented?<br><div class="gmail_default" style="font-size:small"></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>
Otherwise the patch looks good.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Thanks for review.</div></div><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>