mem/oom: remove non-existent case OVERCOMMIT from oom()

PATCH New Han Pingtian 3 participants 7 messages Dec 28, 2015 – Jan 26, 2016

lore

Share & tools

lore

Han Pingtian

author

2015-12-28 07:49 UTC #0 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.

Signed-off-by Han Pingtian <[email protected]>

---
 testcases/kernel/mem/include/mem.h | 7 +++----
 testcases/kernel/mem/lib/mem.c     | 2 +-
 testcases/kernel/mem/oom/oom01.c   | 4 ++--
 3 files changed, 6 insertions(+), 7 deletions(-)

testcases/kernel/mem/include/mem.h +3 -4

index 4a18799..041c437 100644
@@ -32,10 +32,9 @@ static inline void clean_node(unsigned long *array)
 
 #define LENGTH			(3UL<<30)
 #define TESTMEM			(1UL<<30)
-#define OVERCOMMIT		1
-#define NORMAL			2
-#define MLOCK			3
-#define KSM			4
+#define NORMAL			1
+#define MLOCK			2
+#define KSM			3
 
 long overcommit;
 void oom(int testcase, int lite, int retcode, int allow_sigkill);

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

index cee4e91..56104cb 100644
@@ -110,7 +110,7 @@ out:
  * oom - allocates memory according to specified testcase and checks
  *       desired outcome (e.g. child killed, operation failed with ENOMEM)
  * @testcase: selects how child allocates memory
- *            valid choices are: OVERCOMMIT, NORMAL, MLOCK and KSM
+ *            valid choices are: NORMAL, MLOCK and KSM
  * @lite: if non-zero, child makes only single TESTMEM+MB allocation
  *        if zero, child keeps allocating memory until it gets killed
  *        or some operation fails

testcases/kernel/mem/oom/oom01.c +2 -2

index 712d7b5..e39394b 100644
@@ -58,12 +58,12 @@ int main(int argc, char *argv[])
 
 		/* we expect mmap to fail before OOM is hit */
 		set_sys_tune("overcommit_memory", 2, 1);
-		oom(OVERCOMMIT, 0, ENOMEM, 0);
+		oom(NORMAL, 0, ENOMEM, 0);
 
 		/* with overcommit_memory set to 0 or 1 there's no
 		 * guarantee that mmap fails before OOM */
 		set_sys_tune("overcommit_memory", 0, 1);
-		oom(OVERCOMMIT, 0, ENOMEM, 1);
+		oom(NORMAL, 0, ENOMEM, 1);
 
 		set_sys_tune("overcommit_memory", 1, 1);
 		testoom(0, 0, ENOMEM, 1);

Signature · 1 line

1.9.3

Jan Stancek

2016-01-07 11:57 UTC #1 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

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

Quoted 9 lines

From: "Han Pingtian" <[email protected]>
To: [email protected]
Sent: Monday, 28 December, 2015 8:49:19 AM
Subject: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from oom()
Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.
Signed-off-by Han Pingtian <[email protected]>

Reviewed-by Jan Stancek <[email protected]>

Looks good to me, regards,
Jan

Quoted 65 lines

---
 testcases/kernel/mem/include/mem.h | 7 +++----
 testcases/kernel/mem/lib/mem.c     | 2 +-
 testcases/kernel/mem/oom/oom01.c   | 4 ++--
 3 files changed, 6 insertions(+), 7 deletions(-)

testcases/kernel/mem/include/mem.h +3 -4

index 4a18799..041c437 100644
@@ -32,10 +32,9 @@ static inline void clean_node(unsigned long *array)
 
 #define LENGTH			(3UL<<30)
 #define TESTMEM			(1UL<<30)
-#define OVERCOMMIT		1
-#define NORMAL			2
-#define MLOCK			3
-#define KSM			4
+#define NORMAL			1
+#define MLOCK			2
+#define KSM			3
 
 long overcommit;
 void oom(int testcase, int lite, int retcode, int allow_sigkill);

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

index cee4e91..56104cb 100644
@@ -110,7 +110,7 @@ out:
  * oom - allocates memory according to specified testcase and checks
  *       desired outcome (e.g. child killed, operation failed with ENOMEM)
  * @testcase: selects how child allocates memory
- *            valid choices are: OVERCOMMIT, NORMAL, MLOCK and KSM
+ *            valid choices are: NORMAL, MLOCK and KSM
  * @lite: if non-zero, child makes only single TESTMEM+MB allocation
  *        if zero, child keeps allocating memory until it gets killed
  *        or some operation fails

testcases/kernel/mem/oom/oom01.c +2 -3

index 712d7b5..e39394b 100644
@@ -58,12 +58,12 @@ int main(int argc, char *argv[])
 
 		/* we expect mmap to fail before OOM is hit */
 		set_sys_tune("overcommit_memory", 2, 1);
-		oom(OVERCOMMIT, 0, ENOMEM, 0);
+		oom(NORMAL, 0, ENOMEM, 0);
 
 		/* with overcommit_memory set to 0 or 1 there's no
 		 * guarantee that mmap fails before OOM */
 		set_sys_tune("overcommit_memory", 0, 1);
-		oom(OVERCOMMIT, 0, ENOMEM, 1);
+		oom(NORMAL, 0, ENOMEM, 1);
 
 		set_sys_tune("overcommit_memory", 1, 1);
 		testoom(0, 0, ENOMEM, 1);
--

1.9.3

--
Mailing list info: http://lists.linux.it/listinfo/ltp

Cyril Hrubis

2016-01-07 14:54 UTC #2 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

Hi!

Quoted 3 lines

Reviewed-by Jan Stancek <[email protected]>
Looks good to me, regards,

Looks correct to me as well.

Signature · 2 lines

Cyril Hrubis
[email protected]

Han Pingtian

author

2016-01-08 06:59 UTC #3 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

In reply to Jan Stancek

Quoted 11 lines · Jan Stancek

----- Original Message -----
Quoted 8 lines
From: "Han Pingtian" <[email protected]>
To: [email protected]
Sent: Monday, 28 December, 2015 8:49:19 AM
Subject: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from oom()
Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.

Quoted 7 lines

Quoted 1 line
Signed-off-by Han Pingtian <[email protected]>
Reviewed-by Jan Stancek <[email protected]>
Looks good to me, regards,
Jan

Hi Jan,

Thanks for reviewing! I have another question: I think when "overcommit_memory" being set to 1, mmap() will always succeed. It's that correct? I read the code of kernel, looks like in __vm_enough_memory(), if sysctl_overcommit_memory == OVERCOMMIT_ALWAYS, it will always return 0. So I think in oom01.c, we should call the testoom() as testoom(0, 0, 0, 1).

Thanks.

Jan Stancek

2016-01-08 07:42 UTC #4 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

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

Quoted 26 lines

From: "Han Pingtian" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Friday, 8 January, 2016 7:59:41 AM
Subject: Re: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from oom()
Quoted 12 lines · Jan Stancek
----- Original Message -----
Quoted 9 lines
From: "Han Pingtian" <[email protected]>
To: [email protected]
Sent: Monday, 28 December, 2015 8:49:19 AM
Subject: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from
	oom()
Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.
Quoted 7 lines
Quoted 1 line
Signed-off-by Han Pingtian <[email protected]>
Reviewed-by Jan Stancek <[email protected]>
Looks good to me, regards,
Jan

Quoted 8 lines

Hi Jan,
Thanks for reviewing! I have another question: I think when "overcommit_memory" being set to 1, mmap() will always succeed. It's that correct? I read the code of kernel, looks like in __vm_enough_memory(), if sysctl_overcommit_memory == OVERCOMMIT_ALWAYS, it will always return 0. So I think in oom01.c, we should call the testoom() as testoom(0, 0, 0, 1).

It can also fail if you reach max_map_count (or if map size is too big, but that shouldn't be the case in this test).

This is current call in oom01:
testoom(0, 0, ENOMEM, 1);

Third parameter (retcode) does not apply only to mmap(). If you look at testoom(), it makes 3 tests:
oom(NORMAL, lite, retcode, allow_sigkill); oom(MLOCK, lite, retcode, allow_sigkill); oom(KSM, lite, retcode, allow_sigkill);

oom() compares that retcode to exit code of alloc_mem(), which will exit when either of these 3 fails: mmap, mlock or madvise.

Regards,
Jan

Quoted 5 lines

Thanks.

Han Pingtian

author

2016-01-11 06:08 UTC #5 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

Quoted 46 lines · Jan Stancek

----- Original Message -----
Quoted 26 lines
From: "Han Pingtian" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Friday, 8 January, 2016 7:59:41 AM
Subject: Re: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from oom()
Quoted 12 lines · Jan Stancek
----- Original Message -----
Quoted 9 lines
From: "Han Pingtian" <[email protected]>
To: [email protected]
Sent: Monday, 28 December, 2015 8:49:19 AM
Subject: [LTP] [PATCH] mem/oom: remove non-existent case OVERCOMMIT from
	oom()
Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.
Quoted 7 lines
Quoted 1 line
Signed-off-by Han Pingtian <[email protected]>
Reviewed-by Jan Stancek <[email protected]>
Looks good to me, regards,
Jan
Quoted 8 lines
Hi Jan,
Thanks for reviewing! I have another question: I think when "overcommit_memory" being set to 1, mmap() will always succeed. It's that correct? I read the code of kernel, looks like in __vm_enough_memory(), if sysctl_overcommit_memory == OVERCOMMIT_ALWAYS, it will always return 0. So I think in oom01.c, we should call the testoom() as testoom(0, 0, 0, 1).
It can also fail if you reach max_map_count (or if map size is too big, but that shouldn't be the case in this test).
This is current call in oom01:
testoom(0, 0, ENOMEM, 1);

Quoted 9 lines

Third parameter (retcode) does not apply only to mmap(). If you look at testoom(), it makes 3 tests:
oom(NORMAL, lite, retcode, allow_sigkill); oom(MLOCK, lite, retcode, allow_sigkill); oom(KSM, lite, retcode, allow_sigkill);
oom() compares that retcode to exit code of alloc_mem(), which will exit when either of these 3 fails: mmap, mlock or madvise.

Got it and thanks! I didn't know there is a vm.max_map_count.

Cyril Hrubis

2016-01-26 14:14 UTC #6 lore raw

PATCH 1/1 mem/oom: remove non-existent case OVERCOMMIT from oom()

In reply to Han Pingtian

Hi!

Quoted 2 lines

Looks like there is no such a case "OVERCOMMIT" in oom(), so we can just remove it.

Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]