* [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation
@ 2015-09-16 5:06 Li Wang
2015-09-16 6:58 ` Jan Stancek
0 siblings, 1 reply; 5+ messages in thread
From: Li Wang @ 2015-09-16 5:06 UTC (permalink / raw)
To: ltp
Sometimes oom0{3,5} failed as the following results:
oom05 0 TINFO : start OOM testing for mlocked pages.
oom05 0 TINFO : expected victim is 3371.
oom05 0 TINFO : thread (3fff788ff1c0), allocating 3221225472 bytes.
...
oom05 5 TFAIL : mem.c:153: victim unexpectedly ended with retcode: 11, expected: 12
In the OOM test, that tries to consume all memory. But the test doesn't retry to mlock, it simply
exits with errno returned by mlock. At the moment testcase is expecting either ENOMEM or getting
killed by kernel.
Here do retry the function if mlock() fail with 'EAGAIN' errno.
Signed-off-by: Li Wang <liwang@redhat.com>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/mem/lib/mem.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 8fe4bf0..dc9d958 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)
if (s == MAP_FAILED)
return errno;
- if (testcase == MLOCK && mlock(s, length) == -1)
- return errno;
+ if (testcase == MLOCK) {
+ while (mlock(s, length) == -1) {
+ if (EAGAIN != errno)
+ return errno;
+ }
+ }
+
#ifdef HAVE_MADV_MERGEABLE
if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
return errno;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation
2015-09-16 5:06 [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation Li Wang
@ 2015-09-16 6:58 ` Jan Stancek
2015-09-16 8:02 ` Li Wang
0 siblings, 1 reply; 5+ messages in thread
From: Jan Stancek @ 2015-09-16 6:58 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Li Wang" <liwang@redhat.com>
> To: jstancek@redhat.com
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 16 September, 2015 7:06:51 AM
> Subject: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with EAGAIN situation
>
> Sometimes oom0{3,5} failed as the following results:
>
> oom05 0 TINFO : start OOM testing for mlocked pages.
> oom05 0 TINFO : expected victim is 3371.
> oom05 0 TINFO : thread (3fff788ff1c0), allocating 3221225472 bytes.
> ...
> oom05 5 TFAIL : mem.c:153: victim unexpectedly ended with retcode:
> 11, expected: 12
>
> In the OOM test, that tries to consume all memory. But the test doesn't retry
> to mlock, it simply
> exits with errno returned by mlock. At the moment testcase is expecting
> either ENOMEM or getting
> killed by kernel.
>
> Here do retry the function if mlock() fail with 'EAGAIN' errno.
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
Hi,
Please don't include my "Signed-off-by" for patches I haven't written/modified.
> ---
> testcases/kernel/mem/lib/mem.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
> index 8fe4bf0..dc9d958 100644
> --- a/testcases/kernel/mem/lib/mem.c
> +++ b/testcases/kernel/mem/lib/mem.c
> @@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)
> if (s == MAP_FAILED)
> return errno;
>
> - if (testcase == MLOCK && mlock(s, length) == -1)
> - return errno;
> + if (testcase == MLOCK) {
> + while (mlock(s, length) == -1) {
> + if (EAGAIN != errno)
> + return errno;
> + }
> + }
> +
Isn't this going to introduce infinite loop? Shouldn't we limit
the number of retries by some finite number before we let it
touch the pages (and hit OOM)?
Regards,
Jan
> #ifdef HAVE_MADV_MERGEABLE
> if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
> return errno;
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation
2015-09-16 6:58 ` Jan Stancek
@ 2015-09-16 8:02 ` Li Wang
2015-09-16 9:31 ` Jan Stancek
0 siblings, 1 reply; 5+ messages in thread
From: Li Wang @ 2015-09-16 8:02 UTC (permalink / raw)
To: ltp
Hi,
On Wed, Sep 16, 2015 at 2:58 PM, Jan Stancek <jstancek@redhat.com> wrote:
>
>
>
>
> ----- Original Message -----
> > From: "Li Wang" <liwang@redhat.com>
> > To: jstancek@redhat.com
> > Cc: ltp@lists.linux.it
> > Sent: Wednesday, 16 September, 2015 7:06:51 AM
> > Subject: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with EAGAIN
> situation
> >
> > Sometimes oom0{3,5} failed as the following results:
> >
> > oom05 0 TINFO : start OOM testing for mlocked pages.
> > oom05 0 TINFO : expected victim is 3371.
> > oom05 0 TINFO : thread (3fff788ff1c0), allocating 3221225472
> bytes.
> > ...
> > oom05 5 TFAIL : mem.c:153: victim unexpectedly ended with
> retcode:
> > 11, expected: 12
> >
> > In the OOM test, that tries to consume all memory. But the test doesn't
> retry
> > to mlock, it simply
> > exits with errno returned by mlock. At the moment testcase is expecting
> > either ENOMEM or getting
> > killed by kernel.
> >
> > Here do retry the function if mlock() fail with 'EAGAIN' errno.
> >
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
>
> Hi,
>
> Please don't include my "Signed-off-by" for patches I haven't
> written/modified.
>
Since the issue you provided some good advises on debugging work, so I
include you.
Ok, sorry for my unclear the patch rules.
>
> > ---
> > testcases/kernel/mem/lib/mem.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/testcases/kernel/mem/lib/mem.c
> b/testcases/kernel/mem/lib/mem.c
> > index 8fe4bf0..dc9d958 100644
> > --- a/testcases/kernel/mem/lib/mem.c
> > +++ b/testcases/kernel/mem/lib/mem.c
> > @@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)
> > if (s == MAP_FAILED)
> > return errno;
> >
> > - if (testcase == MLOCK && mlock(s, length) == -1)
> > - return errno;
> > + if (testcase == MLOCK) {
> > + while (mlock(s, length) == -1) {
> > + if (EAGAIN != errno)
> > + return errno;
> > + }
> > + }
> > +
>
> Isn't this going to introduce infinite loop? Shouldn't we limit
> the number of retries by some finite number before we let it
> touch the pages (and hit OOM)?
>
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.
Anyway, for security consideration, finite loop is also has itself
advantages.
If so, How about this:
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 8fe4bf0..99eeb04 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -30,6 +30,7 @@ static int alloc_mem(long int length, int testcase)
{
char *s;
long i, pagesz = getpagesize();
+ int ln = 10;
tst_resm(TINFO, "thread (%lx), allocating %ld bytes.",
(unsigned long) pthread_self(), length);
@@ -39,8 +40,14 @@ static int alloc_mem(long int length, int testcase)
if (s == MAP_FAILED)
return errno;
- if (testcase == MLOCK && mlock(s, length) == -1)
- return errno;
+ if (testcase == MLOCK) {
+ while (mlock(s, length) == -1 && ln > 0) {
+ if (EAGAIN != errno)
+ return errno;
+ ln--;
+ }
+ }
>
> Regards,
> Jan
>
> > #ifdef HAVE_MADV_MERGEABLE
> > if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
> > return errno;
> > --
> > 1.8.3.1
> >
> >
>
--
Regards,
Li Wang
Email: liwang@redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20150916/886c22cf/attachment-0001.html>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation
2015-09-16 8:02 ` Li Wang
@ 2015-09-16 9:31 ` Jan Stancek
2015-09-16 9:37 ` Li Wang
0 siblings, 1 reply; 5+ messages in thread
From: Jan Stancek @ 2015-09-16 9:31 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Li Wang" <liwang@redhat.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 16 September, 2015 10:02:42 AM
> Subject: Re: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with EAGAIN situation
> > Isn't this going to introduce infinite loop? Shouldn't we limit
> > the number of retries by some finite number before we let it
> > touch the pages (and hit OOM)?
> >
>
> 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.
>
> Anyway, for security consideration, finite loop is also has itself
> advantages.
>
> If so, How about this:
I think that's fine, I'd suggest we add some small usleep in the loop
too (e.g. 250ms-500ms) to give it a moment before we give up on mlock().
Regards,
Jan
>
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
> index 8fe4bf0..99eeb04 100644
> --- a/testcases/kernel/mem/lib/mem.c
> +++ b/testcases/kernel/mem/lib/mem.c
> @@ -30,6 +30,7 @@ static int alloc_mem(long int length, int testcase)
> {
> char *s;
> long i, pagesz = getpagesize();
> + int ln = 10;
>
> tst_resm(TINFO, "thread (%lx), allocating %ld bytes.",
> (unsigned long) pthread_self(), length);
> @@ -39,8 +40,14 @@ static int alloc_mem(long int length, int testcase)
> if (s == MAP_FAILED)
> return errno;
>
> - if (testcase == MLOCK && mlock(s, length) == -1)
> - return errno;
> + if (testcase == MLOCK) {
> + while (mlock(s, length) == -1 && ln > 0) {
> + if (EAGAIN != errno)
> + return errno;
> + ln--;
> + }
> + }
>
>
>
>
> >
> > Regards,
> > Jan
> >
> > > #ifdef HAVE_MADV_MERGEABLE
> > > if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
> > > return errno;
> > > --
> > > 1.8.3.1
> > >
> > >
> >
>
>
>
> --
> Regards,
> Li Wang
> Email: liwang@redhat.com
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation
2015-09-16 9:31 ` Jan Stancek
@ 2015-09-16 9:37 ` Li Wang
0 siblings, 0 replies; 5+ messages in thread
From: Li Wang @ 2015-09-16 9:37 UTC (permalink / raw)
To: ltp
Hi,
On Wed, Sep 16, 2015 at 5:31 PM, Jan Stancek <jstancek@redhat.com> wrote:
>
>
>
>
> ----- Original Message -----
> > From: "Li Wang" <liwang@redhat.com>
> > To: "Jan Stancek" <jstancek@redhat.com>
> > Cc: ltp@lists.linux.it
> > Sent: Wednesday, 16 September, 2015 10:02:42 AM
> > Subject: Re: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with
> EAGAIN situation
> > > Isn't this going to introduce infinite loop? Shouldn't we limit
> > > the number of retries by some finite number before we let it
> > > touch the pages (and hit OOM)?
> > >
> >
> > 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.
> >
> > Anyway, for security consideration, finite loop is also has itself
> > advantages.
> >
> > If so, How about this:
>
> I think that's fine, I'd suggest we add some small usleep in the loop
> too (e.g. 250ms-500ms) to give it a moment before we give up on mlock().
>
Good. I will post v2. Thank you for reviewing.
>
> Regards,
> Jan
>
> >
> > diff --git a/testcases/kernel/mem/lib/mem.c
> b/testcases/kernel/mem/lib/mem.c
> > index 8fe4bf0..99eeb04 100644
> > --- a/testcases/kernel/mem/lib/mem.c
> > +++ b/testcases/kernel/mem/lib/mem.c
> > @@ -30,6 +30,7 @@ static int alloc_mem(long int length, int testcase)
> > {
> > char *s;
> > long i, pagesz = getpagesize();
> > + int ln = 10;
> >
> > tst_resm(TINFO, "thread (%lx), allocating %ld bytes.",
> > (unsigned long) pthread_self(), length);
> > @@ -39,8 +40,14 @@ static int alloc_mem(long int length, int testcase)
> > if (s == MAP_FAILED)
> > return errno;
> >
> > - if (testcase == MLOCK && mlock(s, length) == -1)
> > - return errno;
> > + if (testcase == MLOCK) {
> > + while (mlock(s, length) == -1 && ln > 0) {
> > + if (EAGAIN != errno)
> > + return errno;
> > + ln--;
> > + }
> > + }
> >
> >
> >
> >
> > >
> > > Regards,
> > > Jan
> > >
> > > > #ifdef HAVE_MADV_MERGEABLE
> > > > if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) ==
> -1)
> > > > return errno;
> > > > --
> > > > 1.8.3.1
> > > >
> > > >
> > >
> >
> >
> >
> > --
> > Regards,
> > Li Wang
> > Email: liwang@redhat.com
> >
>
--
Regards,
Li Wang
Email: liwang@redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20150916/4a893c62/attachment.html>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-16 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-16 5:06 [LTP] [PATCH] lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation Li Wang
2015-09-16 6:58 ` Jan Stancek
2015-09-16 8:02 ` Li Wang
2015-09-16 9:31 ` Jan Stancek
2015-09-16 9:37 ` 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.