All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] ksm: perform cleanup actions only when they are actually required
@ 2018-03-19 15:23 Stanislav Kholmanskikh
  2018-03-20 12:01 ` Cyril Hrubis
  0 siblings, 1 reply; 6+ messages in thread
From: Stanislav Kholmanskikh @ 2018-03-19 15:23 UTC (permalink / raw)
  To: ltp

This one is similar to commit 8ae3916674fa
("mem/oom, tunable: perform cleanup actions only when they are actually required")
i.e. we are making sure that ksm tests do not revert any not yet performed
actions.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/mem/ksm/ksm01.c |    6 +++++-
 testcases/kernel/mem/ksm/ksm02.c |   11 +++++++++--
 testcases/kernel/mem/ksm/ksm03.c |   11 +++++++++--
 testcases/kernel/mem/ksm/ksm04.c |   16 +++++++++++++---
 testcases/kernel/mem/ksm/ksm06.c |    5 ++++-
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index ce96983..0961927 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -59,6 +59,8 @@
 #include "mem.h"
 #include "ksm_common.h"
 
+static int max_page_sharing_saved;
+
 static void verify_ksm(void)
 {
 	create_same_memory(size, num, unit);
@@ -70,6 +72,7 @@ static void setup(void)
 		tst_brk(TCONF, "KSM configuration is not enabled");
 
 	save_max_page_sharing();
+	max_page_sharing_saved = 1;
 
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
@@ -93,7 +96,8 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	restore_max_page_sharing();
+	if (max_page_sharing_saved)
+		restore_max_page_sharing();
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 03b44c0..014ac1c 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -59,6 +59,9 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
+static int max_page_sharing_saved;
+static int cpuset_mounted;
+
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
@@ -86,9 +89,11 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	restore_max_page_sharing();
+	if (max_page_sharing_saved)
+		restore_max_page_sharing();
 
-	umount_mem(CPATH, CPATH_NEW);
+	if (cpuset_mounted)
+		umount_mem(CPATH, CPATH_NEW);
 }
 
 static void setup(void)
@@ -96,6 +101,7 @@ static void setup(void)
 	if (access(PATH_KSM, F_OK) == -1)
 		tst_brk(TCONF, "KSM configuration is not enabled");
 	save_max_page_sharing();
+	max_page_sharing_saved = 1;
 
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
@@ -106,6 +112,7 @@ static void setup(void)
 	}
 
 	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
+	cpuset_mounted = 1;
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 51cc923..79a8f87 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -59,6 +59,9 @@
 #include "mem.h"
 #include "ksm_common.h"
 
+static int max_page_sharing_saved;
+static int memcg_mounted;
+
 static void verify_ksm(void)
 {
 	write_memcg();
@@ -77,8 +80,10 @@ static void setup(void)
 	}
 
 	save_max_page_sharing();
+	max_page_sharing_saved = 1;
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
+	memcg_mounted = 1;
 }
 
 static void cleanup(void)
@@ -87,9 +92,11 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	restore_max_page_sharing();
+	if (max_page_sharing_saved)
+		restore_max_page_sharing();
 
-	umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	if (memcg_mounted)
+		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 835222f..a79e649 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -59,6 +59,10 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
+static int max_page_sharing_saved;
+static int cpuset_mounted;
+static int memcg_mounted;
+
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
@@ -88,10 +92,13 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	restore_max_page_sharing();
+	if (max_page_sharing_saved)
+		restore_max_page_sharing();
 
-	umount_mem(CPATH, CPATH_NEW);
-	umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	if (cpuset_mounted)
+		umount_mem(CPATH, CPATH_NEW);
+	if (memcg_mounted)
+		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
 }
 
 static void setup(void)
@@ -106,10 +113,13 @@ static void setup(void)
 	}
 
 	save_max_page_sharing();
+	max_page_sharing_saved = 1;
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
+	cpuset_mounted = 1;
 	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
+	memcg_mounted = 1;
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm06.c b/testcases/kernel/mem/ksm/ksm06.c
index 53d532c..92a36c1 100644
--- a/testcases/kernel/mem/ksm/ksm06.c
+++ b/testcases/kernel/mem/ksm/ksm06.c
@@ -42,6 +42,7 @@
 static int run = -1;
 static int sleep_millisecs = -1;
 static int merge_across_nodes = -1;
+static int max_page_sharing_saved;
 static unsigned long nr_pages;
 
 static char *n_opt;
@@ -66,6 +67,7 @@ static void setup(void)
 		tst_brk(TCONF, "no merge_across_nodes sysfs knob");
 
 	save_max_page_sharing();
+	max_page_sharing_saved = 1;
 
 	if (!is_numa(NULL, NH_MEMS, 2))
 		tst_brk(TCONF, "The case needs a NUMA system.");
@@ -91,7 +93,8 @@ static void cleanup(void)
 	if (run != -1)
 		FILE_PRINTF(PATH_KSM "run", "%d", run);
 
-	restore_max_page_sharing();
+	if (max_page_sharing_saved)
+		restore_max_page_sharing();
 }
 
 static struct tst_test test = {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH] ksm: perform cleanup actions only when they are actually required
  2018-03-19 15:23 [LTP] [PATCH] ksm: perform cleanup actions only when they are actually required Stanislav Kholmanskikh
@ 2018-03-20 12:01 ` Cyril Hrubis
  2018-03-20 13:02   ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Stanislav Kholmanskikh
  0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2018-03-20 12:01 UTC (permalink / raw)
  To: ltp

Hi!
> +static int max_page_sharing_saved;
> +
>  static void verify_ksm(void)
>  {
>  	create_same_memory(size, num, unit);
> @@ -70,6 +72,7 @@ static void setup(void)
>  		tst_brk(TCONF, "KSM configuration is not enabled");
>  
>  	save_max_page_sharing();
> +	max_page_sharing_saved = 1;

Given that we have the save_max_page_sharing() and
restore_max_page_sharing() functions it would be cleaner to put the flag
that tells us if we should restore it to the lib/mem.c so that we do not
have to repeat the code in all testcases.

Looking into kernel documentation is says that minimum for the
max_page_sharing is 2 so we may as well just do:

diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 4992843f4..eedfe260f 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -250,6 +250,9 @@ void save_max_page_sharing(void)
 
 void restore_max_page_sharing(void)
 {
+       if (!max_page_sharing)
+               return;
+
        if (access(PATH_KSM "max_page_sharing", F_OK) == 0)
                FILE_PRINTF(PATH_KSM "max_page_sharing",
                                 "%d", max_page_sharing);

-- 
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved
  2018-03-20 12:01 ` Cyril Hrubis
@ 2018-03-20 13:02   ` Stanislav Kholmanskikh
  2018-03-20 13:02     ` [LTP] [PATCH V2 2/2] ksm: perform cleanup actions only when they are actually required Stanislav Kholmanskikh
  2018-03-20 13:05     ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Cyril Hrubis
  0 siblings, 2 replies; 6+ messages in thread
From: Stanislav Kholmanskikh @ 2018-03-20 13:02 UTC (permalink / raw)
  To: ltp

Documentation/vm/ksm.txt states that the minimum value for max_page_sharing
is 2, so we can use it as a flag to test if save_max_page_sharing()
was called before.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
Changes to V1:
 - made the change to restore_max_page_sharing() following Cyril's proposal
 - split V1 into two patches, one - restore_max_page_sharing(), the other - ksm tests


 testcases/kernel/mem/lib/mem.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 4992843..f36b33a 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -250,6 +250,15 @@ void save_max_page_sharing(void)
 
 void restore_max_page_sharing(void)
 {
+	/*
+	 * Documentation/vm/ksm.txt states that the minimum
+	 * value for max_page_sharing is 2, so on
+	 * max_page_sharing != 0 after save_max_page_sharing()
+	 * returns.
+	 */
+	if (!max_page_sharing)
+		return;
+
 	if (access(PATH_KSM "max_page_sharing", F_OK) == 0)
 	        FILE_PRINTF(PATH_KSM "max_page_sharing",
 	                         "%d", max_page_sharing);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH V2 2/2] ksm: perform cleanup actions only when they are actually required
  2018-03-20 13:02   ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Stanislav Kholmanskikh
@ 2018-03-20 13:02     ` Stanislav Kholmanskikh
  2018-03-20 13:05     ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Cyril Hrubis
  1 sibling, 0 replies; 6+ messages in thread
From: Stanislav Kholmanskikh @ 2018-03-20 13:02 UTC (permalink / raw)
  To: ltp

This one is similar to commit 8ae3916674fa
("mem/oom, tunable: perform cleanup actions only when they are actually required")
i.e. we are making sure that ksm tests do not revert any not yet performed
actions.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/mem/ksm/ksm02.c |    6 +++++-
 testcases/kernel/mem/ksm/ksm03.c |    6 +++++-
 testcases/kernel/mem/ksm/ksm04.c |   11 +++++++++--
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 03b44c0..22c78cb 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -59,6 +59,8 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
+static int cpuset_mounted;
+
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
@@ -88,7 +90,8 @@ static void cleanup(void)
 
 	restore_max_page_sharing();
 
-	umount_mem(CPATH, CPATH_NEW);
+	if (cpuset_mounted)
+		umount_mem(CPATH, CPATH_NEW);
 }
 
 static void setup(void)
@@ -106,6 +109,7 @@ static void setup(void)
 	}
 
 	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
+	cpuset_mounted = 1;
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 51cc923..284f4ec 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -59,6 +59,8 @@
 #include "mem.h"
 #include "ksm_common.h"
 
+static int memcg_mounted;
+
 static void verify_ksm(void)
 {
 	write_memcg();
@@ -79,6 +81,7 @@ static void setup(void)
 	save_max_page_sharing();
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
+	memcg_mounted = 1;
 }
 
 static void cleanup(void)
@@ -89,7 +92,8 @@ static void cleanup(void)
 
 	restore_max_page_sharing();
 
-	umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	if (memcg_mounted)
+		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 835222f..9ffef4d 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -59,6 +59,9 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
+static int cpuset_mounted;
+static int memcg_mounted;
+
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
@@ -90,8 +93,10 @@ static void cleanup(void)
 
 	restore_max_page_sharing();
 
-	umount_mem(CPATH, CPATH_NEW);
-	umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	if (cpuset_mounted)
+		umount_mem(CPATH, CPATH_NEW);
+	if (memcg_mounted)
+		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
 }
 
 static void setup(void)
@@ -109,7 +114,9 @@ static void setup(void)
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
+	cpuset_mounted = 1;
 	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
+	memcg_mounted = 1;
 }
 
 static struct tst_test test = {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved
  2018-03-20 13:02   ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Stanislav Kholmanskikh
  2018-03-20 13:02     ` [LTP] [PATCH V2 2/2] ksm: perform cleanup actions only when they are actually required Stanislav Kholmanskikh
@ 2018-03-20 13:05     ` Cyril Hrubis
  2018-03-20 13:28       ` Stanislav Kholmanskikh
  1 sibling, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2018-03-20 13:05 UTC (permalink / raw)
  To: ltp

Hi!
Both patches looks good, acked.

-- 
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved
  2018-03-20 13:05     ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Cyril Hrubis
@ 2018-03-20 13:28       ` Stanislav Kholmanskikh
  0 siblings, 0 replies; 6+ messages in thread
From: Stanislav Kholmanskikh @ 2018-03-20 13:28 UTC (permalink / raw)
  To: ltp

Thank you very much. Pushed.

On 03/20/2018 04:05 PM, Cyril Hrubis wrote:
> Hi!
> Both patches looks good, acked.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-20 13:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-19 15:23 [LTP] [PATCH] ksm: perform cleanup actions only when they are actually required Stanislav Kholmanskikh
2018-03-20 12:01 ` Cyril Hrubis
2018-03-20 13:02   ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Stanislav Kholmanskikh
2018-03-20 13:02     ` [LTP] [PATCH V2 2/2] ksm: perform cleanup actions only when they are actually required Stanislav Kholmanskikh
2018-03-20 13:05     ` [LTP] [PATCH V2 1/2] restore_max_page_sharing: restore the value only if it was saved Cyril Hrubis
2018-03-20 13:28       ` Stanislav Kholmanskikh

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.