lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation

PATCH New Li Wang 2 participants 5 messages Sep 16, 2015

lore

Share & tools

lore

Li Wang

author

2015-09-16 05:06 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

Signed-off-by Jan Stancek <[email protected]>

---
 testcases/kernel/mem/lib/mem.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

testcases/kernel/mem/lib/mem.c +7 -2

index 8fe4bf0..dc9d958 100644
@@ -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;

Signature · 1 line

1.8.3.1

Jan Stancek

2015-09-16 06:58 UTC #1 lore raw

PATCH 1/1 lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation

----- Original Message -----

Quoted 21 lines

From: "Li Wang" <[email protected]>
To: [email protected]
Cc: [email protected]
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.

Quoted 4 lines

Here do retry the function if mlock() fail with 'EAGAIN' errno.
Signed-off-by Li Wang <[email protected]>
Signed-off-by Jan Stancek <[email protected]>

Hi,

Please don't include my "Signed-off-by" for patches I haven't written/modified.

Quoted 11 lines

---
 testcases/kernel/mem/lib/mem.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

testcases/kernel/mem/lib/mem.c +0 -0

index 8fe4bf0..dc9d958 100644
@@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)
 	if (s == MAP_FAILED)
 		return errno;

testcases/kernel/mem/lib/mem.c static int alloc_mem(long int length, int testcase) · line 48

Quoted 10 lines

testcases/kernel/mem/lib/mem.c +7 -2

 
-	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

Quoted 7 lines

testcases/kernel/mem/lib/mem.c +0 -1

 #ifdef HAVE_MADV_MERGEABLE
 	if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
 		return errno;
--

1.8.3.1

Li Wang

author

2015-09-16 08:02 UTC #2 lore raw

PATCH 1/1 lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation

Hi,

Quoted 32 lines · Jan Stancek

----- Original Message -----
Quoted 5 lines
From: "Li Wang" <[email protected]>
To: [email protected]
Cc: [email protected]
Sent: Wednesday, 16 September, 2015 7:06:51 AM
Subject: [PATCH] lib/mem.c: handle the case oom0{3,5} exit with EAGAIN
situation
Quoted 6 lines
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.
Quoted 2 lines
...
oom05 5 TFAIL : mem.c:153: victim unexpectedly ended with
retcode:
Quoted 3 lines
11, expected: 12
In the OOM test, that tries to consume all memory. But the test doesn't
retry
Quoted 7 lines
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.

Quoted 8 lines

Quoted 2 lines
Signed-off-by Li Wang <[email protected]>
Signed-off-by Jan Stancek <[email protected]>
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.

Quoted 14 lines

Review
Quoted 6 lines
---
 testcases/kernel/mem/lib/mem.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/testcases/kernel/mem/lib/mem.c
Quoted 7 lines
index 8fe4bf0..dc9d958 100644
testcases/kernel/mem/lib/mem.c +0 -0
@@ -39,8 +39,13 @@ static int alloc_mem(long int length, int testcase)
      if (s == MAP_FAILED)
              return errno;
 

Quoted 14 lines

testcases/kernel/mem/lib/mem.c static int alloc_mem(long int length, int testcase) · line 48
Quoted 9 lines
testcases/kernel/mem/lib/mem.c +7 -2
-     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:

testcases/kernel/mem/lib/mem.c +8 -2

index 8fe4bf0..99eeb04 100644
@@ -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--;
+               }
+       }

Quoted 12 lines

Regards,
Jan

Quoted 7 lines

testcases/kernel/mem/lib/mem.c +0 -1

 #ifdef HAVE_MADV_MERGEABLE
      if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
              return errno;
--

1.8.3.1

Signature · 6 lines

Regards,
Li Wang
Email: [email protected]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20150916/886c22cf/attachment-0001.html>

Jan Stancek

2015-09-16 09:31 UTC #3 lore raw

PATCH 1/1 lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation

----- Original Message -----

Quoted 10 lines

From: "Li Wang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
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
Quoted 4 lines
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)?

Quoted 8 lines

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

Quoted 50 lines

testcases/kernel/mem/lib/mem.c +8 -2

index 8fe4bf0..99eeb04 100644
@@ -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--;
+               }
+       }

Quoted 13 lines

Regards,
Jan

Quoted 7 lines

 #ifdef HAVE_MADV_MERGEABLE
      if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) == -1)
              return errno;

--
1.8.3.1

--
Regards,
Li Wang
Email: [email protected]

Li Wang

author

2015-09-16 09:37 UTC #4 lore raw

PATCH 1/1 lib/mem.c: handle the case oom0{3, 5} exit with EAGAIN situation

Hi,

Quoted 20 lines · Jan Stancek

----- Original Message -----
Quoted 5 lines
From: "Li Wang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
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
Quoted 9 lines
Quoted 4 lines
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.

Quoted 8 lines

Quoted 4 lines
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.

Quoted 57 lines

Regards,
Jan

Review

Quoted 2 lines

(quoted diff) +0 -0

b/testcases/kernel/mem/lib/mem.c

Quoted 29 lines

index 8fe4bf0..99eeb04 100644

testcases/kernel/mem/lib/mem.c +8 -2

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

Quoted 2 lines

Quoted 5 lines

Regards,
Jan

Quoted 2 lines

 #ifdef HAVE_MADV_MERGEABLE
      if (testcase == KSM && madvise(s, length, MADV_MERGEABLE) ==

-1)

Quoted 14 lines

Quoted 6 lines
Quoted 5 lines
              return errno;
--
1.8.3.1
--
Regards,
Li Wang
Email: [email protected]

Signature · 6 lines

Regards,
Li Wang
Email: [email protected]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20150916/4a893c62/attachment.html>