linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] KSM: support smart-scan feature
@ 2023-12-01 21:09 Stefan Roesch
  2023-12-01 21:09 ` [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests Stefan Roesch
  2023-12-01 21:09 ` [PATCH v2 2/2] add ksm test for smart-scan feature Stefan Roesch
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-01 21:09 UTC (permalink / raw)
  To: kernel-team; +Cc: shr, david, oliver.sang, linux-mm, ltp, pvorel, liwang

This patch series makes two changes:
- Disable the smart-scan feature for ksm01 - ksm04
  This is necessary to make sure that the volatile metrics have the
  expected counts.
- Add a new test for the smart-scan feature
  The new test verifies that the smart-scan feature skips pages when it has
  been enabled for a VMA.

Versions:
- V2:
  - Disable smart_scan in the test setup structure
  - Remove the changes in create_same_memory()
  - Add the new testcase ksm07 for the smart scan test
  

Stefan Roesch (2):
  mem: disable KSM smart scan for ksm tests
  add ksm test for smart-scan feature

 testcases/kernel/mem/.gitignore    |  1 +
 testcases/kernel/mem/include/mem.h |  1 +
 testcases/kernel/mem/ksm/ksm01.c   |  2 +
 testcases/kernel/mem/ksm/ksm02.c   |  2 +
 testcases/kernel/mem/ksm/ksm03.c   |  2 +
 testcases/kernel/mem/ksm/ksm04.c   |  2 +
 testcases/kernel/mem/ksm/ksm07.c   | 69 +++++++++++++++++++++++++++++
 testcases/kernel/mem/lib/mem.c     | 70 ++++++++++++++++++++++++++++++
 8 files changed, 149 insertions(+)
 create mode 100644 testcases/kernel/mem/ksm/ksm07.c


base-commit: 8c89ef3d451087ed6e18750bd5eedd10e5ab3d2e
-- 
2.39.3



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests
  2023-12-01 21:09 [PATCH v2 0/2] KSM: support smart-scan feature Stefan Roesch
@ 2023-12-01 21:09 ` Stefan Roesch
  2023-12-04 10:45   ` Petr Vorel
  2023-12-01 21:09 ` [PATCH v2 2/2] add ksm test for smart-scan feature Stefan Roesch
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Roesch @ 2023-12-01 21:09 UTC (permalink / raw)
  To: kernel-team; +Cc: shr, david, oliver.sang, linux-mm, ltp, pvorel, liwang

This disables the "smart scan" KSM feature to make sure that the volatile
count remains at 0.

Signed-off-by: Stefan Roesch <shr@devkernel.io>

Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202311161132.13d8ce5a-oliver.sang@intel.com
---
 testcases/kernel/mem/ksm/ksm01.c | 2 ++
 testcases/kernel/mem/ksm/ksm02.c | 2 ++
 testcases/kernel/mem/ksm/ksm03.c | 2 ++
 testcases/kernel/mem/ksm/ksm04.c | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index bcd095865..e2d3d9e00 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -86,6 +86,8 @@ static struct tst_test test = {
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
 		{"/sys/kernel/mm/ksm/merge_across_nodes", "1",
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
+		{"/sys/kernel/mm/ksm/smart_scan", "0",
+			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
 		{}
 	},
 	.needs_kconfigs = (const char *const[]){
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index bce639dce..3707de95d 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -107,6 +107,8 @@ static struct tst_test test = {
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
 		{"/sys/kernel/mm/ksm/merge_across_nodes", "1",
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
+		{"/sys/kernel/mm/ksm/smart_scan", "0",
+			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
 		{}
 	},
 	.needs_kconfigs = (const char *const[]){
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 4a733269f..cff74700d 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -89,6 +89,8 @@ static struct tst_test test = {
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
 		{"/sys/kernel/mm/ksm/merge_across_nodes", "1",
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
+		{"/sys/kernel/mm/ksm/smart_scan", "0",
+			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
 		{}
 	},
 	.needs_kconfigs = (const char *const[]){
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 4f1f2f721..9935e32d7 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -109,6 +109,8 @@ static struct tst_test test = {
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
 		{"/sys/kernel/mm/ksm/merge_across_nodes", "1",
 			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
+		{"/sys/kernel/mm/ksm/smart_scan", "0",
+			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
 		{}
 	},
 	.needs_kconfigs = (const char *const[]){
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] add ksm test for smart-scan feature
  2023-12-01 21:09 [PATCH v2 0/2] KSM: support smart-scan feature Stefan Roesch
  2023-12-01 21:09 ` [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests Stefan Roesch
@ 2023-12-01 21:09 ` Stefan Roesch
  2023-12-04  9:21   ` Petr Vorel
  2023-12-04 10:39   ` Petr Vorel
  1 sibling, 2 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-01 21:09 UTC (permalink / raw)
  To: kernel-team; +Cc: shr, david, oliver.sang, linux-mm, ltp, pvorel, liwang

This adds a new ksm (kernel samepage merging) test to evaluate the new
smart scan feature. It allocates a page and fills it with 'a'
characters. It captures the pages_skipped counter, waits for a few
iterations and captures the pages_skipped counter again. The expectation
is that over 50% of the page scans are skipped (There is only one page
that has KSM enabled and it gets scanned during each iteration and it
cannot be de-duplicated).

Signed-off-by: Stefan Roesch <shr@devkernel.io>
---
 testcases/kernel/mem/.gitignore    |  1 +
 testcases/kernel/mem/include/mem.h |  1 +
 testcases/kernel/mem/ksm/ksm07.c   | 69 +++++++++++++++++++++++++++++
 testcases/kernel/mem/lib/mem.c     | 70 ++++++++++++++++++++++++++++++
 4 files changed, 141 insertions(+)
 create mode 100644 testcases/kernel/mem/ksm/ksm07.c

diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 7258489ed..c96fe8bfc 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -53,6 +53,7 @@
 /ksm/ksm04
 /ksm/ksm05
 /ksm/ksm06
+/ksm/ksm07
 /mem/mem02
 /mmapstress/mmap-corruption01
 /mmapstress/mmapstress01
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index cdc3ca146..dbc3eb9ec 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -48,6 +48,7 @@ void testoom(int mempolicy, int lite, int retcode, int allow_sigkill);
 /* KSM */
 
 void create_same_memory(int size, int num, int unit);
+void create_memory_for_smartscan(void);
 void test_ksm_merge_across_nodes(unsigned long nr_pages);
 void ksm_group_check(int run, int pg_shared, int pg_sharing, int pg_volatile,
                      int pg_unshared, int sleep_msecs, int pages_to_scan);
diff --git a/testcases/kernel/mem/ksm/ksm07.c b/testcases/kernel/mem/ksm/ksm07.c
new file mode 100644
index 000000000..9cde64c82
--- /dev/null
+++ b/testcases/kernel/mem/ksm/ksm07.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2010-2017  Red Hat, Inc.
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ *
+ * Kernel Samepage Merging (KSM)
+ *
+ * This adds a new ksm (kernel samepage merging) test to evaluate the new
+ * smart scan feature. It allocates a page and fills it with 'a'
+ * characters. It captures the pages_skipped counter, waits for a few
+ * iterations and captures the pages_skipped counter again. The expectation
+ * is that over 50% of the page scans are skipped (There is only one page
+ * that has KSM enabled and it gets scanned during each iteration and it
+ * cannot be de-duplicated).
+ *
+ * Prerequisites:
+ *
+ * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
+ *    distrub the testing as they also change some ksm tunables depends
+ *    on current workloads.
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "../include/mem.h"
+#include "ksm_common.h"
+
+static void verify_ksm(void)
+{
+	create_memory_for_smartscan();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{}
+	},
+	.save_restore = (const struct tst_path_val[]) {
+		{"/sys/kernel/mm/ksm/run", NULL, TST_SR_TBROK},
+		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TBROK},
+		{"/sys/kernel/mm/ksm/smart_scan", "1",
+			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
+		{}
+	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
+	.test_all = verify_ksm,
+};
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index fbfeef026..b27a017fc 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -438,6 +438,76 @@ static void resume_ksm_children(int *child, int num)
 	fflush(stdout);
 }
 
+/* This test allocates one page, fills the page with a's, captures the
+ * full_scan and pages_skipped counters. Then it makes sure at least 3
+ * full scans have been performed and measures the above counters again.
+ * The expectation is that at least 50% of the pages are skipped.
+ *
+ * To wait for at least 3 scans it uses the wait_ksmd_full_scan() function. In
+ * reality, it will be a lot more scans as the wait_ksmd_full_scan() function
+ * sleeps for one second.
+ */
+void create_memory_for_smartscan(void)
+{
+	int status;
+	int full_scans_begin;
+	int full_scans_end;
+	int pages_skipped_begin;
+	int pages_skipped_end;
+	int diff_pages;
+	int diff_scans;
+	unsigned long page_size;
+	char *memory;
+
+	/* Apply for the space for memory. */
+	page_size = sysconf(_SC_PAGE_SIZE);
+	memory = SAFE_MALLOC(page_size);
+
+	for (int i = 0; i < 1; i++) {
+		memory = SAFE_MMAP(NULL, page_size, PROT_READ|PROT_WRITE,
+			MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+#ifdef HAVE_DECL_MADV_MERGEABLE
+		if (madvise(memory, page_size, MADV_MERGEABLE) == -1)
+			tst_brk(TBROK|TERRNO, "madvise");
+#endif
+	}
+	memset(memory, 'a', page_size);
+
+	tst_res(TINFO, "KSM merging...");
+	if (access(PATH_KSM "max_page_sharing", F_OK) == 0) {
+		SAFE_FILE_PRINTF(PATH_KSM "run", "2");
+	}
+
+	/* Set defalut ksm scan values. */
+	SAFE_FILE_PRINTF(PATH_KSM "run", "1");
+	SAFE_FILE_PRINTF(PATH_KSM "pages_to_scan", "%ld", 100l);
+	SAFE_FILE_PRINTF(PATH_KSM "sleep_millisecs", "0");
+
+	/* Measure pages skipped aka "smart scan". */
+	SAFE_FILE_SCANF(PATH_KSM "full_scans", "%d", &full_scans_begin);
+	SAFE_FILE_SCANF(PATH_KSM "pages_skipped", "%d", &pages_skipped_begin);
+	wait_ksmd_full_scan();
+
+	tst_res(TINFO, "stop KSM.");
+	SAFE_FILE_PRINTF(PATH_KSM "run", "0");
+
+	SAFE_FILE_SCANF(PATH_KSM "full_scans", "%d", &full_scans_end);
+	SAFE_FILE_SCANF(PATH_KSM "pages_skipped", "%d", &pages_skipped_end);
+	diff_pages = pages_skipped_end - pages_skipped_begin;
+	diff_scans = full_scans_end - full_scans_begin;
+
+	if (diff_pages < diff_scans * 50 / 100) {
+		tst_res(TFAIL, "not enough pages have been skipped by smart_scan.");
+	} else {
+		tst_res(TPASS, "smart_scan skipped more than 50%% of the pages.");
+	}
+
+	while (waitpid(-1, &status, 0) > 0)
+		if (WEXITSTATUS(status) != 0)
+			tst_res(TFAIL, "child exit status is %d",
+					WEXITSTATUS(status));
+}
+
 void create_same_memory(int size, int num, int unit)
 {
 	int i, j, status, *child;
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] add ksm test for smart-scan feature
  2023-12-01 21:09 ` [PATCH v2 2/2] add ksm test for smart-scan feature Stefan Roesch
@ 2023-12-04  9:21   ` Petr Vorel
  2023-12-04 18:41     ` Stefan Roesch
  2023-12-04 10:39   ` Petr Vorel
  1 sibling, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2023-12-04  9:21 UTC (permalink / raw)
  To: Stefan Roesch; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang

Hi Stefan,

>  testcases/kernel/mem/.gitignore    |  1 +
>  testcases/kernel/mem/include/mem.h |  1 +
>  testcases/kernel/mem/ksm/ksm07.c   | 69 +++++++++++++++++++++++++++++
>  testcases/kernel/mem/lib/mem.c     | 70 ++++++++++++++++++++++++++++++
You need to add 'ksm07 ksm07' line to runtest/mm.
If it's the only error, we can fix this before merge.

Kind regards,
Petr


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] add ksm test for smart-scan feature
  2023-12-01 21:09 ` [PATCH v2 2/2] add ksm test for smart-scan feature Stefan Roesch
  2023-12-04  9:21   ` Petr Vorel
@ 2023-12-04 10:39   ` Petr Vorel
  2023-12-04 18:42     ` Stefan Roesch
  1 sibling, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2023-12-04 10:39 UTC (permalink / raw)
  To: Stefan Roesch; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang

Hi Stefan,

...
> +++ b/testcases/kernel/mem/ksm/ksm07.c
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2010-2017  Red Hat, Inc.
> + *

> + * This program is free software;  you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY;  without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + * the GNU General Public License for more details.
NOTE: we use SPDX instead of verbose GPL (see ksm06.c).
> + *

NOTE: we have special doc format which starts like this (see ksm06.c):
/*\
 * [Description]
 *
 * ...
> + * Kernel Samepage Merging (KSM)
> + *
> + * This adds a new ksm (kernel samepage merging) test to evaluate the new
> + * smart scan feature. It allocates a page and fills it with 'a'
> + * characters. It captures the pages_skipped counter, waits for a few
> + * iterations and captures the pages_skipped counter again. The expectation
> + * is that over 50% of the page scans are skipped (There is only one page
> + * that has KSM enabled and it gets scanned during each iteration and it
> + * cannot be de-duplicated).
> + *
> + * Prerequisites:
> + *
> + * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
> + *    distrub the testing as they also change some ksm tunables depends
> + *    on current workloads.
Hm, we don't have to any helper in LTP API to detect this, so at least we
document this.
> + *
> + */

The result is then uploaded:
https://github.com/linux-test-project/ltp/releases/download/20230929/metadata.20230929.html

Therefore please use:

// SPDX-License-Identifier: GPL-2.0-or-later
/*
 * Copyright (C) 2010-2017  Red Hat, Inc.
 */
/*\
 * [Description]
 *
 * Kernel Samepage Merging (KSM)
 *
 * This adds a new ksm (kernel samepage merging) test to evaluate the new
 * smart scan feature. It allocates a page and fills it with 'a'
 * characters. It captures the pages_skipped counter, waits for a few
 * iterations and captures the pages_skipped counter again. The expectation
 * is that over 50% of the page scans are skipped (There is only one page
 * that has KSM enabled and it gets scanned during each iteration and it
 * cannot be de-duplicated).
 *
 * Prerequisites:
 *
 * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
 *    distrub the testing as they also change some ksm tunables depends
 *    on current workloads.
 */

> +
> +#include <sys/types.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include "../include/mem.h"
> +#include "ksm_common.h"
> +
> +static void verify_ksm(void)
> +{
> +	create_memory_for_smartscan();
I wonder, if we reusable create_memory_for_smartscan() later. Maybe it should be
put into ksm07 for the start.

Also, the test needs to run on older kernel - exit with TCONF (which is not
considered as a failure) instead of TBROK which does now:
mem.c:488: TBROK: Failed to open FILE '/sys/kernel/mm/ksm/pages_skipped' for reading: ENOENT (2)

If the testing code stays in testcases/kernel/mem/lib/mem.c, you will have to
stat() it. But if it's really just this test specific and you move it to
ksm07.c, then you can simply add the handling via .save_restore.

> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.options = (struct tst_option[]) {
> +		{}
> +	},
> +	.save_restore = (const struct tst_path_val[]) {
> +		{"/sys/kernel/mm/ksm/run", NULL, TST_SR_TBROK},
> +		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TBROK},
> +		{"/sys/kernel/mm/ksm/smart_scan", "1",
> +			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
> +		{}
> +	},
> +	.needs_kconfigs = (const char *const[]){
> +		"CONFIG_KSM=y",
> +		NULL
> +	},
> +	.test_all = verify_ksm,
> +};

Also, there are missing static:
$ cd testcases/kernel/mem/ksm/; make check-ksm07
CHECK testcases/kernel/mem/ksm/ksm07.c
ksm07.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
make: [../../../../include/mk/rules.mk:56: check-ksm07] Error 1 (ignored)
ksm07.c: note: in included file:
ksm_common.h:14:5: warning: Symbol 'size' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:14:29: warning: Symbol 'num' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:14:38: warning: Symbol 'unit' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:6: warning: Symbol 'opt_sizestr' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:20: warning: Symbol 'opt_numstr' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:33: warning: Symbol 'opt_unitstr' has no prototype or library ('tst_') prefix. Should it be static?

Kind regards,
Petr


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests
  2023-12-01 21:09 ` [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests Stefan Roesch
@ 2023-12-04 10:45   ` Petr Vorel
  2023-12-04 18:38     ` Stefan Roesch
  2023-12-04 18:38     ` Stefan Roesch
  0 siblings, 2 replies; 10+ messages in thread
From: Petr Vorel @ 2023-12-04 10:45 UTC (permalink / raw)
  To: Stefan Roesch; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang

Hi Stefan,

> This disables the "smart scan" KSM feature to make sure that the volatile
> count remains at 0.

> Signed-off-by: Stefan Roesch <shr@devkernel.io>

> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202311161132.13d8ce5a-oliver.sang@intel.com
> ---
>  testcases/kernel/mem/ksm/ksm01.c | 2 ++
>  testcases/kernel/mem/ksm/ksm02.c | 2 ++
>  testcases/kernel/mem/ksm/ksm03.c | 2 ++
>  testcases/kernel/mem/ksm/ksm04.c | 2 ++
Li suggested in v1, that also ksm0[56].c should disable smart_scan (ksm05.c is
questionable, but since you prepared ksm07 it can IMHO be disable in both ksm0[56].c).

https://lore.kernel.org/ltp/CAEemH2fqamX720diM1N+iN9a8HM30_5sHg8V0EMHgHdrh3iZPw@mail.gmail.com/

With that, you can add:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests
  2023-12-04 10:45   ` Petr Vorel
  2023-12-04 18:38     ` Stefan Roesch
@ 2023-12-04 18:38     ` Stefan Roesch
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-04 18:38 UTC (permalink / raw)
  To: Petr Vorel; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang


Petr Vorel <pvorel@suse.cz> writes:

> Hi Stefan,
>
>> This disables the "smart scan" KSM feature to make sure that the volatile
>> count remains at 0.
>
>> Signed-off-by: Stefan Roesch <shr@devkernel.io>
>
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202311161132.13d8ce5a-oliver.sang@intel.com
>> ---
>>  testcases/kernel/mem/ksm/ksm01.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm02.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm03.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm04.c | 2 ++
> Li suggested in v1, that also ksm0[56].c should disable smart_scan (ksm05.c is
> questionable, but since you prepared ksm07 it can IMHO be disable in both ksm0[56].c).
>
> https://lore.kernel.org/ltp/CAEemH2fqamX720diM1N+iN9a8HM30_5sHg8V0EMHgHdrh3iZPw@mail.gmail.com/
>
> With that, you can add:
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr

The next version will also disable smart scan for ksm05 and ksm06


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests
  2023-12-04 10:45   ` Petr Vorel
@ 2023-12-04 18:38     ` Stefan Roesch
  2023-12-04 18:38     ` Stefan Roesch
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-04 18:38 UTC (permalink / raw)
  To: Petr Vorel; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang


Petr Vorel <pvorel@suse.cz> writes:

> Hi Stefan,
>
>> This disables the "smart scan" KSM feature to make sure that the volatile
>> count remains at 0.
>
>> Signed-off-by: Stefan Roesch <shr@devkernel.io>
>
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202311161132.13d8ce5a-oliver.sang@intel.com
>> ---
>>  testcases/kernel/mem/ksm/ksm01.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm02.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm03.c | 2 ++
>>  testcases/kernel/mem/ksm/ksm04.c | 2 ++
> Li suggested in v1, that also ksm0[56].c should disable smart_scan (ksm05.c is
> questionable, but since you prepared ksm07 it can IMHO be disable in both ksm0[56].c).
>
> https://lore.kernel.org/ltp/CAEemH2fqamX720diM1N+iN9a8HM30_5sHg8V0EMHgHdrh3iZPw@mail.gmail.com/
>
> With that, you can add:
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr

The next version will also disable smart scan for ksm05 and ksm06


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] add ksm test for smart-scan feature
  2023-12-04  9:21   ` Petr Vorel
@ 2023-12-04 18:41     ` Stefan Roesch
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-04 18:41 UTC (permalink / raw)
  To: Petr Vorel; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang


Petr Vorel <pvorel@suse.cz> writes:

> Hi Stefan,
>
>>  testcases/kernel/mem/.gitignore    |  1 +
>>  testcases/kernel/mem/include/mem.h |  1 +
>>  testcases/kernel/mem/ksm/ksm07.c   | 69 +++++++++++++++++++++++++++++
>>  testcases/kernel/mem/lib/mem.c     | 70 ++++++++++++++++++++++++++++++
> You need to add 'ksm07 ksm07' line to runtest/mm.
> If it's the only error, we can fix this before merge.
>
> Kind regards,
> Petr
>

The next version will also update runtest/mm.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] add ksm test for smart-scan feature
  2023-12-04 10:39   ` Petr Vorel
@ 2023-12-04 18:42     ` Stefan Roesch
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roesch @ 2023-12-04 18:42 UTC (permalink / raw)
  To: Petr Vorel; +Cc: kernel-team, david, oliver.sang, linux-mm, ltp, liwang


Petr Vorel <pvorel@suse.cz> writes:

> Hi Stefan,
>
> ...
>> +++ b/testcases/kernel/mem/ksm/ksm07.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Copyright (C) 2010-2017  Red Hat, Inc.
>> + *
>
>> + * This program is free software;  you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY;  without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
>> + * the GNU General Public License for more details.
> NOTE: we use SPDX instead of verbose GPL (see ksm06.c).
>> + *
>
> NOTE: we have special doc format which starts like this (see ksm06.c):
> /*\
>  * [Description]
>  *
>  * ...
>> + * Kernel Samepage Merging (KSM)
>> + *
>> + * This adds a new ksm (kernel samepage merging) test to evaluate the new
>> + * smart scan feature. It allocates a page and fills it with 'a'
>> + * characters. It captures the pages_skipped counter, waits for a few
>> + * iterations and captures the pages_skipped counter again. The expectation
>> + * is that over 50% of the page scans are skipped (There is only one page
>> + * that has KSM enabled and it gets scanned during each iteration and it
>> + * cannot be de-duplicated).
>> + *
>> + * Prerequisites:
>> + *
>> + * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
>> + *    distrub the testing as they also change some ksm tunables depends
>> + *    on current workloads.
> Hm, we don't have to any helper in LTP API to detect this, so at least we
> document this.
>> + *
>> + */
>
> The result is then uploaded:
> https://github.com/linux-test-project/ltp/releases/download/20230929/metadata.20230929.html
>
> Therefore please use:
>
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
>  * Copyright (C) 2010-2017  Red Hat, Inc.
>  */
> /*\
>  * [Description]
>  *
>  * Kernel Samepage Merging (KSM)
>  *
>  * This adds a new ksm (kernel samepage merging) test to evaluate the new
>  * smart scan feature. It allocates a page and fills it with 'a'
>  * characters. It captures the pages_skipped counter, waits for a few
>  * iterations and captures the pages_skipped counter again. The expectation
>  * is that over 50% of the page scans are skipped (There is only one page
>  * that has KSM enabled and it gets scanned during each iteration and it
>  * cannot be de-duplicated).
>  *
>  * Prerequisites:
>  *
>  * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
>  *    distrub the testing as they also change some ksm tunables depends
>  *    on current workloads.
>  */
>

The next version will use the above comment as documentation.

>> +
>> +#include <sys/types.h>
>> +#include <sys/mman.h>
>> +#include <sys/stat.h>
>> +#include <sys/wait.h>
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <signal.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include "../include/mem.h"
>> +#include "ksm_common.h"
>> +
>> +static void verify_ksm(void)
>> +{
>> +	create_memory_for_smartscan();
> I wonder, if we reusable create_memory_for_smartscan() later. Maybe it should be
> put into ksm07 for the start.
>

I moved it over to ksm07.c and renamed it to create_memory.

> Also, the test needs to run on older kernel - exit with TCONF (which is not
> considered as a failure) instead of TBROK which does now:
> mem.c:488: TBROK: Failed to open FILE '/sys/kernel/mm/ksm/pages_skipped' for reading: ENOENT (2)
>

Changed it to TCONF.

> If the testing code stays in testcases/kernel/mem/lib/mem.c, you will have to
> stat() it. But if it's really just this test specific and you move it to
> ksm07.c, then you can simply add the handling via .save_restore.
>
>> +}
>> +
>> +static struct tst_test test = {
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.options = (struct tst_option[]) {
>> +		{}
>> +	},
>> +	.save_restore = (const struct tst_path_val[]) {
>> +		{"/sys/kernel/mm/ksm/run", NULL, TST_SR_TBROK},
>> +		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TBROK},
>> +		{"/sys/kernel/mm/ksm/smart_scan", "1",
>> +			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
>> +		{}
>> +	},
>> +	.needs_kconfigs = (const char *const[]){
>> +		"CONFIG_KSM=y",
>> +		NULL
>> +	},
>> +	.test_all = verify_ksm,
>> +};
>
> Also, there are missing static:
>

These are declared as non-static in ksm_common.h.

> $ cd testcases/kernel/mem/ksm/; make check-ksm07
> CHECK testcases/kernel/mem/ksm/ksm07.c
> ksm07.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> make: [../../../../include/mk/rules.mk:56: check-ksm07] Error 1 (ignored)
> ksm07.c: note: in included file:
> ksm_common.h:14:5: warning: Symbol 'size' has no prototype or library ('tst_') prefix. Should it be static?
> ksm_common.h:14:29: warning: Symbol 'num' has no prototype or library ('tst_') prefix. Should it be static?
> ksm_common.h:14:38: warning: Symbol 'unit' has no prototype or library ('tst_') prefix. Should it be static?
> ksm_common.h:15:6: warning: Symbol 'opt_sizestr' has no prototype or library
> ('tst_') prefix. Should it be static?
> ksm_common.h:15:20: warning: Symbol 'opt_numstr' has no prototype or library
> ('tst_') prefix. Should it be static?
> ksm_common.h:15:33: warning: Symbol 'opt_unitstr' has no prototype or library
> ('tst_') prefix. Should it be static?
>
> Kind regards,
> Petr


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-12-04 18:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-01 21:09 [PATCH v2 0/2] KSM: support smart-scan feature Stefan Roesch
2023-12-01 21:09 ` [PATCH v2 1/2] mem: disable KSM smart scan for ksm tests Stefan Roesch
2023-12-04 10:45   ` Petr Vorel
2023-12-04 18:38     ` Stefan Roesch
2023-12-04 18:38     ` Stefan Roesch
2023-12-01 21:09 ` [PATCH v2 2/2] add ksm test for smart-scan feature Stefan Roesch
2023-12-04  9:21   ` Petr Vorel
2023-12-04 18:41     ` Stefan Roesch
2023-12-04 10:39   ` Petr Vorel
2023-12-04 18:42     ` Stefan Roesch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).