<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 16, 2015 at 2:58 PM, Jan Stancek <span dir="ltr"><<a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><br>
<br>
<br>
<br>
----- Original Message -----<br>
> From: "Li Wang" <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
> To: <a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</a><br>
> Cc: <a href="mailto:ltp@lists.linux.it" target="_blank">ltp@lists.linux.it</a><br>
> Sent: Wednesday, 16 September, 2015 7:06:51 AM<br>
> Subject: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with EAGAIN situation<br>
><br>
> Sometimes oom0{3,5} failed as the following results:<br>
><br>
> oom05 0 TINFO : start OOM testing for mlocked pages.<br>
> oom05 0 TINFO : expected victim is 3371.<br>
> oom05 0 TINFO : thread (3fff788ff1c0), allocating 3221225472 bytes.<br>
> ...<br>
> oom05 5 TFAIL : mem.c:153: victim unexpectedly ended with retcode:<br>
> 11, expected: 12<br>
><br>
> In the OOM test, that tries to consume all memory. But the test doesn't retry<br>
> to mlock, it simply<br>
> exits with errno returned by mlock. At the moment testcase is expecting<br>
> either ENOMEM or getting<br>
> killed by kernel.<br>
><br>
> Here do retry the function if mlock() fail with 'EAGAIN' errno.<br>
><br>
> Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
> Signed-off-by: Jan Stancek <<a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</a>><br>
<br>
</span>Hi,<br>
<br>
Please don't include my "Signed-off-by" for patches I haven't written/modified.<br></blockquote><div><br></div><div>Since the issue you provided some good advises on debugging work, so I include you.<br><br></div><div>Ok, sorry for my unclear the patch rules. <br><br></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">
<span><br>
> ---<br>
> testcases/kernel/mem/lib/mem.c | 9 +++++++--<br>
> 1 file changed, 7 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c<br>
> index 8fe4bf0..dc9d958 100644<br>
> --- a/testcases/kernel/mem/lib/mem.c<br>
> +++ b/testcases/kernel/mem/lib/mem.c<br>
> @@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)<br>
> if (s == MAP_FAILED)<br>
> return errno;<br>
><br>
> - if (testcase == MLOCK && mlock(s, length) == -1)<br>
> - return errno;<br>
> + if (testcase == MLOCK) {<br>
> + while (mlock(s, length) == -1) {<br>
> + if (EAGAIN != errno)<br>
> + return errno;<br>
> + }<br>
> + }<br>
> +<br>
<br>
</span>Isn't this going to introduce infinite loop? Shouldn't we limit<br>
the number of retries by some finite number before we let it<br>
touch the pages (and hit OOM)?<br></blockquote><div><br></div><div>Hmm, at first I'm trying to use finite loop for the mlock(), but think over, I feel it is not a big matter since mlock() desen't need so much memory to do the lock. and I did that.<br><br></div><div>Anyway, for security consideration, finite loop is also has itself advantages.<br><br></div><div>If so, How about this:<br><br>diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c<br>index 8fe4bf0..99eeb04 100644<br>--- a/testcases/kernel/mem/lib/mem.c<br>+++ b/testcases/kernel/mem/lib/mem.c<br>@@ -30,6 +30,7 @@ static int alloc_mem(long int length, int testcase)<br> {<br> char *s;<br> long i, pagesz = getpagesize();<br>+ int ln = 10;<br> <br> tst_resm(TINFO, "thread (%lx), allocating %ld bytes.",<br> (unsigned long) pthread_self(), length);<br>@@ -39,8 +40,14 @@ static int alloc_mem(long int length, int testcase)<br> if (s == MAP_FAILED)<br> return errno;<br> <br>- if (testcase == MLOCK && mlock(s, length) == -1)<br>- return errno;<br>+ if (testcase == MLOCK) {<br>+ while (mlock(s, length) == -1 && ln > 0) {<br>+ if (EAGAIN != errno)<br>+ return errno;<br>+ ln--;<br>+ }<br>+ }<br><br></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>
Regards,<br>
Jan<br>
<div><div><br>
> #ifdef HAVE_MADV_MERGEABLE<br>
> if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)<br>
> return errno;<br>
> --<br>
> 1.8.3.1<br>
><br>
><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Regards,<br></div>Li Wang<br></div><div>Email: <a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a><br></div></div></div></div></div></div>
</div></div>