[LTP] [PATCH 0/4] enhance .save_restore to support set value

[LTP] [PATCH 0/4] enhance .save_restore to support set value

[Li Wang]

This makes .save_restore to support set expected value after
saving the knob's original to be possible, the main change
is convert the NULL-terminated string to struct tst_path_val.

And the rest part is preparing and cleanup work.

Li Wang (4):
  lib: move struct tst_sys_conf to internal
  lib: enhance .save_restore to support set expected value
  testcase: switch to the new .save_restore
  ksm: cleanup work and make use of .save_restore

 doc/c-test-api.txt                            | 16 +++----
 include/tst_sys_conf.h                        |  8 ++--
 include/tst_test.h                            |  2 +-
 lib/newlib_tests/test19.c                     | 14 +++----
 lib/newlib_tests/test20.c                     | 10 ++---
 lib/tst_sys_conf.c                            | 19 +++++++++
 lib/tst_test.c                                |  9 ++--
 testcases/cve/icmp_rate_limit01.c             |  4 +-
 testcases/kernel/containers/userns/userns08.c |  4 +-
 testcases/kernel/mem/ksm/ksm01.c              | 42 +++++++------------
 testcases/kernel/mem/ksm/ksm02.c              | 29 ++++---------
 testcases/kernel/mem/ksm/ksm03.c              | 29 ++++---------
 testcases/kernel/mem/ksm/ksm04.c              | 28 ++++---------
 testcases/kernel/mem/ksm/ksm05.c              | 29 ++++---------
 testcases/kernel/mem/ksm/ksm06.c              | 10 ++---
 testcases/kernel/mem/ksm/ksm_common.h         |  2 -
 testcases/kernel/syscalls/add_key/add_key05.c | 16 ++-----
 testcases/kernel/syscalls/bind/bind06.c       |  4 +-
 testcases/kernel/syscalls/madvise/madvise08.c |  4 +-
 .../syscalls/migrate_pages/migrate_pages02.c  |  5 +--
 testcases/kernel/syscalls/sendto/sendto03.c   |  4 +-
 .../kernel/syscalls/setsockopt/setsockopt05.c |  4 +-
 .../kernel/syscalls/setsockopt/setsockopt06.c |  4 +-
 .../kernel/syscalls/setsockopt/setsockopt07.c |  4 +-
 .../kernel/syscalls/setsockopt/setsockopt08.c |  4 +-
 .../kernel/syscalls/setsockopt/setsockopt09.c |  4 +-
 26 files changed, 130 insertions(+), 178 deletions(-)

-- 
2.31.1


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

[LTP] [PATCH 1/4] lib: move struct tst_sys_conf to internal

[Li Wang]

As it only used in tst_sys_conf library interanlly, there is
unnecessary to put in header file.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/tst_sys_conf.h | 6 ------
 lib/tst_sys_conf.c     | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
index 323e29a27..507a552e8 100644
--- a/include/tst_sys_conf.h
+++ b/include/tst_sys_conf.h
@@ -5,12 +5,6 @@
 #ifndef TST_SYS_CONF_H__
 #define TST_SYS_CONF_H__
 
-struct tst_sys_conf {
-	char path[PATH_MAX];
-	char value[PATH_MAX];
-	struct tst_sys_conf *next;
-};
-
 int tst_sys_conf_save_str(const char *path, const char *value);
 int tst_sys_conf_save(const char *path);
 void tst_sys_conf_restore(int verbose);
diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
index 4ad9f8b9b..d7118f15f 100644
--- a/lib/tst_sys_conf.c
+++ b/lib/tst_sys_conf.c
@@ -12,6 +12,12 @@
 #include "tst_test.h"
 #include "tst_sys_conf.h"
 
+struct tst_sys_conf {
+	char path[PATH_MAX];
+	char value[PATH_MAX];
+	struct tst_sys_conf *next;
+};
+
 static struct tst_sys_conf *save_restore_data;
 
 void tst_sys_conf_dump(void)
-- 
2.31.1


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

Re: [LTP] [PATCH 1/4] lib: move struct tst_sys_conf to internal

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 2/4] lib: enhance .save_restore to support set expected value

[Li Wang]

This extends that .save_restore support set new expected value after
saving the knob's original, which also avoids additionally checking
before using the file at other places.

And, export function tst_sys_conf_set() can be singly used for setting
new value of knob in the whole LTP.

Reported-by: Bruno Goncalves <bgoncalv@redhat.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Jan Stancek <jstancek@redhat.com>
---
 doc/c-test-api.txt     | 16 +++++++++-------
 include/tst_sys_conf.h |  6 ++++++
 include/tst_test.h     |  2 +-
 lib/tst_sys_conf.c     | 13 +++++++++++++
 lib/tst_test.c         |  9 +++++----
 5 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 28383ccee..3e167a1d3 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -1559,12 +1559,14 @@ itself is not available on the system.
 1.27 Saving & restoring /proc|sys values
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-LTP library can be instructed to save and restore value of specified
+LTP library can be instructed to save and restore the value of specified
 (/proc|sys) files. This is achieved by initialized tst_test struct
-field 'save_restore'. It is a 'NULL' terminated array of strings where
-each string represents a file, whose value is saved at the beginning
-and restored at the end of the test. Only first line of a specified
-file is saved and restored.
+field 'save_restore'. It is a NULL-terminated array of struct
+'tst_path_val' where each tst_path_val.path represents a file, whose
+value is saved at the beginning and restored at the end of the test.
+If pass a value to tst_path_val.value that will be set to the knob
+correspondingly, NULL means do nothing else but save the original value.
+Only the first line of a specified file is saved and restored.
 
 Pathnames can be optionally prefixed to specify how strictly (during
 'store') are handled errors:
@@ -1578,8 +1580,8 @@ Pathnames can be optionally prefixed to specify how strictly (during
 
 [source,c]
 -------------------------------------------------------------------------------
-static const char *save_restore[] = {
-	"/proc/sys/kernel/core_pattern",
+static const struct tst_path_val save_restore[] = {
+	{"/proc/sys/kernel/core_pattern", NULL},
 	NULL,
 };
 
diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
index 507a552e8..b7bbe36fc 100644
--- a/include/tst_sys_conf.h
+++ b/include/tst_sys_conf.h
@@ -5,8 +5,14 @@
 #ifndef TST_SYS_CONF_H__
 #define TST_SYS_CONF_H__
 
+struct tst_path_val {
+        const char *path;
+        const char *val;
+};
+
 int tst_sys_conf_save_str(const char *path, const char *value);
 int tst_sys_conf_save(const char *path);
+void tst_sys_conf_set(const char *path, const char *value);
 void tst_sys_conf_restore(int verbose);
 void tst_sys_conf_dump(void);
 
diff --git a/include/tst_test.h b/include/tst_test.h
index 816fab4dd..e514efa76 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -259,7 +259,7 @@ struct tst_test {
 	 * NULL terminated array of (/proc, /sys) files to save
 	 * before setup and restore after cleanup
 	 */
-	const char * const *save_restore;
+	const struct tst_path_val const *save_restore;
 
 	/*
 	 * NULL terminated array of kernel config options required for the
diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
index d7118f15f..f9460d228 100644
--- a/lib/tst_sys_conf.c
+++ b/lib/tst_sys_conf.c
@@ -96,6 +96,19 @@ int tst_sys_conf_save(const char *path)
 	return tst_sys_conf_save_str(path, line);
 }
 
+void tst_sys_conf_set(const char *path, const char *value)
+{
+	char flag = path[0];
+	if (flag  == '?' || flag == '!')
+		path++;
+
+	if (access(path, F_OK) != 0)
+		tst_brk(TBROK | TERRNO, " The path %s is not exist", path);
+
+	if (value)
+		SAFE_FILE_PRINTF(path, "%s", value);
+}
+
 void tst_sys_conf_restore(int verbose)
 {
 	struct tst_sys_conf *i;
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 9e745c537..fe2e2bb6c 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1105,11 +1105,12 @@ static void do_setup(int argc, char *argv[])
 		tst_tmpdir();
 
 	if (tst_test->save_restore) {
-		const char * const *name = tst_test->save_restore;
+		const struct tst_path_val const *pvl = tst_test->save_restore;
 
-		while (*name) {
-			tst_sys_conf_save(*name);
-			name++;
+		while (pvl->path) {
+			if (!tst_sys_conf_save(pvl->path))
+				tst_sys_conf_set(pvl->path, pvl->val);
+			pvl++;
 		}
 	}
 
-- 
2.31.1


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

Re: [LTP] [PATCH 2/4] lib: enhance .save_restore to support set expected value

[Cyril Hrubis]

Hi!
> This extends that .save_restore support set new expected value after
> saving the knob's original, which also avoids additionally checking
> before using the file at other places.
> 
> And, export function tst_sys_conf_set() can be singly used for setting
> new value of knob in the whole LTP.
> 
> Reported-by: Bruno Goncalves <bgoncalv@redhat.com>
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Jan Stancek <jstancek@redhat.com>
> ---
>  doc/c-test-api.txt     | 16 +++++++++-------
>  include/tst_sys_conf.h |  6 ++++++
>  include/tst_test.h     |  2 +-
>  lib/tst_sys_conf.c     | 13 +++++++++++++
>  lib/tst_test.c         |  9 +++++----
>  5 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
> index 28383ccee..3e167a1d3 100644
> --- a/doc/c-test-api.txt
> +++ b/doc/c-test-api.txt
> @@ -1559,12 +1559,14 @@ itself is not available on the system.
>  1.27 Saving & restoring /proc|sys values
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> -LTP library can be instructed to save and restore value of specified
> +LTP library can be instructed to save and restore the value of specified
                                                      ^
						      This shouldn't be
						      here as we do not
						      talk about
						      specific proc/sys
						      files but about
						      any proc/sys file

>  (/proc|sys) files. This is achieved by initialized tst_test struct
> -field 'save_restore'. It is a 'NULL' terminated array of strings where
> -each string represents a file, whose value is saved at the beginning
> -and restored at the end of the test. Only first line of a specified
> -file is saved and restored.
> +field 'save_restore'. It is a NULL-terminated array of struct
> +'tst_path_val' where each tst_path_val.path represents a file, whose
> +value is saved at the beginning and restored at the end of the test.
> +If pass a value to tst_path_val.value that will be set to the knob
> +correspondingly, NULL means do nothing else but save the original value.

"If non-NULL value is passed it is written to the respective file at the
beginning of the test."

> +Only the first line of a specified file is saved and restored.
>  
>  Pathnames can be optionally prefixed to specify how strictly (during
>  'store') are handled errors:
> @@ -1578,8 +1580,8 @@ Pathnames can be optionally prefixed to specify how strictly (during
>  
>  [source,c]
>  -------------------------------------------------------------------------------
> -static const char *save_restore[] = {
> -	"/proc/sys/kernel/core_pattern",
> +static const struct tst_path_val save_restore[] = {
> +	{"/proc/sys/kernel/core_pattern", NULL},
>  	NULL,
>  };
>  
> diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
> index 507a552e8..b7bbe36fc 100644
> --- a/include/tst_sys_conf.h
> +++ b/include/tst_sys_conf.h
> @@ -5,8 +5,14 @@
>  #ifndef TST_SYS_CONF_H__
>  #define TST_SYS_CONF_H__
>  
> +struct tst_path_val {
> +        const char *path;
> +        const char *val;
> +};
> +
>  int tst_sys_conf_save_str(const char *path, const char *value);
>  int tst_sys_conf_save(const char *path);
> +void tst_sys_conf_set(const char *path, const char *value);
>  void tst_sys_conf_restore(int verbose);
>  void tst_sys_conf_dump(void);
>  
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 816fab4dd..e514efa76 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -259,7 +259,7 @@ struct tst_test {
>  	 * NULL terminated array of (/proc, /sys) files to save
>  	 * before setup and restore after cleanup
>  	 */
> -	const char * const *save_restore;
> +	const struct tst_path_val const *save_restore;
>  
>  	/*
>  	 * NULL terminated array of kernel config options required for the
> diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
> index d7118f15f..f9460d228 100644
> --- a/lib/tst_sys_conf.c
> +++ b/lib/tst_sys_conf.c
> @@ -96,6 +96,19 @@ int tst_sys_conf_save(const char *path)
>  	return tst_sys_conf_save_str(path, line);
>  }
>  
> +void tst_sys_conf_set(const char *path, const char *value)
> +{
> +	char flag = path[0];
> +	if (flag  == '?' || flag == '!')
> +		path++;
> +
> +	if (access(path, F_OK) != 0)
> +		tst_brk(TBROK | TERRNO, " The path %s is not exist", path);
                                         ^            ^
					 |           does
                                      useless space?

Also do we really have to check for the file existence here? The
SAFE_FILE_PRINTF() will TBROK if the fopen() fails anyways.


> +	if (value)
> +		SAFE_FILE_PRINTF(path, "%s", value);
> +}
> +
>  void tst_sys_conf_restore(int verbose)
>  {
>  	struct tst_sys_conf *i;
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 9e745c537..fe2e2bb6c 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1105,11 +1105,12 @@ static void do_setup(int argc, char *argv[])
>  		tst_tmpdir();
>  
>  	if (tst_test->save_restore) {
> -		const char * const *name = tst_test->save_restore;
> +		const struct tst_path_val const *pvl = tst_test->save_restore;
>  
> -		while (*name) {
> -			tst_sys_conf_save(*name);
> -			name++;
> +		while (pvl->path) {
> +			if (!tst_sys_conf_save(pvl->path))
> +				tst_sys_conf_set(pvl->path, pvl->val);

Maybe it would be cleaner if we added tst_sys_conf_save_set() function
instead of tst_sys_conf_set() that would do both, saved the value and
set new one if non-NULL.

> +			pvl++;
>  		}
>  	}
>  
> -- 
> 2.31.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/4] lib: enhance .save_restore to support set expected value

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1421 bytes --]

Cyril Hrubis <chrubis@suse.cz> wrote:


> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 9e745c537..fe2e2bb6c 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1105,11 +1105,12 @@ static void do_setup(int argc, char *argv[])
> >               tst_tmpdir();
> >
> >       if (tst_test->save_restore) {
> > -             const char * const *name = tst_test->save_restore;
> > +             const struct tst_path_val const *pvl =
> tst_test->save_restore;
> >
> > -             while (*name) {
> > -                     tst_sys_conf_save(*name);
> > -                     name++;
> > +             while (pvl->path) {
> > +                     if (!tst_sys_conf_save(pvl->path))
> > +                             tst_sys_conf_set(pvl->path, pvl->val);
>
> Maybe it would be cleaner if we added tst_sys_conf_save_set() function
> instead of tst_sys_conf_set() that would do both, saved the value and
> set new one if non-NULL.
>

Yes, it can be. Actually, I started writing like that at the beginning.

But I feel that tst_sys_conf_save_set() looks a bit longer and does more
things. To split the set into a separate function is to respect the UNIX
design
philosophy (one function does one thing). Another important reason
is to export tst_sys_conf_set() to global use instead of low-level macros
for knob settings.

The rest suggestions sound good to me, thanks for reviewing!

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2610 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

[LTP] [PATCH 3/4] testcase: switch to the new .save_restore

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 lib/newlib_tests/test19.c                        | 14 ++++++--------
 lib/newlib_tests/test20.c                        | 10 ++++------
 testcases/cve/icmp_rate_limit01.c                |  4 ++--
 testcases/kernel/containers/userns/userns08.c    |  4 ++--
 testcases/kernel/mem/ksm/ksm01.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm02.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm03.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm04.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm06.c                 | 10 +++++-----
 testcases/kernel/syscalls/add_key/add_key05.c    | 16 ++++------------
 testcases/kernel/syscalls/bind/bind06.c          |  4 ++--
 testcases/kernel/syscalls/madvise/madvise08.c    |  4 ++--
 .../syscalls/migrate_pages/migrate_pages02.c     |  5 ++---
 testcases/kernel/syscalls/sendto/sendto03.c      |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt05.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt06.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt07.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt08.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt09.c    |  4 ++--
 19 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/lib/newlib_tests/test19.c b/lib/newlib_tests/test19.c
index 78b5202d2..402a2ad30 100644
--- a/lib/newlib_tests/test19.c
+++ b/lib/newlib_tests/test19.c
@@ -8,13 +8,6 @@
 #include "tst_test.h"
 #include "tst_sys_conf.h"
 
-static const char * const save_restore[] = {
-	"?/proc/nonexistent",
-	"!/proc/sys/kernel/numa_balancing",
-	"/proc/sys/kernel/core_pattern",
-	NULL,
-};
-
 static void setup(void)
 {
 	SAFE_FILE_PRINTF("/proc/sys/kernel/core_pattern", "changed");
@@ -30,5 +23,10 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.test_all = run,
 	.setup = setup,
-	.save_restore = save_restore,
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/nonexistent", NULL},
+		{"!/proc/sys/kernel/numa_balancing", NULL},
+		{"/proc/sys/kernel/core_pattern", NULL},
+		NULL,
+	},
 };
diff --git a/lib/newlib_tests/test20.c b/lib/newlib_tests/test20.c
index 53317b669..5feb7c144 100644
--- a/lib/newlib_tests/test20.c
+++ b/lib/newlib_tests/test20.c
@@ -11,11 +11,6 @@
 #include "tst_hugepage.h"
 #include "tst_sys_conf.h"
 
-static const char * const save_restore[] = {
-	"!/proc/sys/kernel/numa_balancing",
-	NULL,
-};
-
 static void do_test(void) {
 
 	unsigned long val, hpages;
@@ -41,5 +36,8 @@ static void do_test(void) {
 static struct tst_test test = {
 	.test_all = do_test,
 	.request_hugepages = 2,
-	.save_restore = save_restore,
+	.save_restore = (const struct tst_path_val const[]) {
+		{"!/proc/sys/kernel/numa_balancing", "0"},
+		NULL,
+	},
 };
diff --git a/testcases/cve/icmp_rate_limit01.c b/testcases/cve/icmp_rate_limit01.c
index 3ada32675..81735ce18 100644
--- a/testcases/cve/icmp_rate_limit01.c
+++ b/testcases/cve/icmp_rate_limit01.c
@@ -268,8 +268,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/containers/userns/userns08.c b/testcases/kernel/containers/userns/userns08.c
index c141b1aca..33d0a9997 100644
--- a/testcases/kernel/containers/userns/userns08.c
+++ b/testcases/kernel/containers/userns/userns08.c
@@ -134,8 +134,8 @@ static struct tst_test test = {
 		"CONFIG_USER_NS",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index 7470d3143..0a81e2016 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -103,8 +103,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 76a87c6b1..6ba6ee868 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -115,8 +115,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 6a0566f43..71d2d8bd9 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -100,8 +100,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 77fafa641..8429f4843 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -116,8 +116,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm06.c b/testcases/kernel/mem/ksm/ksm06.c
index 6a74b7406..f28383cc0 100644
--- a/testcases/kernel/mem/ksm/ksm06.c
+++ b/testcases/kernel/mem/ksm/ksm06.c
@@ -136,11 +136,11 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
-		"!/sys/kernel/mm/ksm/run",
-		"!/sys/kernel/mm/ksm/sleep_millisecs",
-		"/sys/kernel/mm/ksm/merge_across_nodes",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
+		{"/sys/kernel/mm/ksm/merge_across_nodes", NULL},
 		NULL,
 	},
 	.needs_kconfigs = (const char *const[]){
diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
index 2be1b7014..bbe44e043 100644
--- a/testcases/kernel/syscalls/add_key/add_key05.c
+++ b/testcases/kernel/syscalls/add_key/add_key05.c
@@ -202,13 +202,6 @@ static void do_test(unsigned int n)
 	return;
 }
 
-static void setup(void)
-{
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/gc_delay", "1");
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxkeys", "200");
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxbytes", "20000");
-}
-
 static void cleanup(void)
 {
 	while (usern--)
@@ -220,12 +213,11 @@ static struct tst_test test = {
 	.tcnt = 2,
 	.needs_root = 1,
 	.forks_child = 1,
-	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/kernel/keys/gc_delay",
-		"?/proc/sys/kernel/keys/maxkeys",
-		"?/proc/sys/kernel/keys/maxbytes",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/kernel/keys/gc_delay", "1"},
+		{"?/proc/sys/kernel/keys/maxkeys", "200"},
+		{"?/proc/sys/kernel/keys/maxbytes", "20000"},
 		NULL,
 	},
 	.bufs = (struct tst_buffers []) {
diff --git a/testcases/kernel/syscalls/bind/bind06.c b/testcases/kernel/syscalls/bind/bind06.c
index 297311c04..7a84b19cd 100644
--- a/testcases/kernel/syscalls/bind/bind06.c
+++ b/testcases/kernel/syscalls/bind/bind06.c
@@ -109,8 +109,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index ff167dafe..e513d558a 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -213,8 +213,8 @@ static struct tst_test test = {
 	.needs_tmpdir = 1,
 	.needs_root = 1,
 	.forks_child = 1,
-	.save_restore = (const char * const[]) {
-		CORE_PATTERN,
+	.save_restore = (const struct tst_path_val const[]) {
+		{CORE_PATTERN, NULL},
 		NULL,
 	},
 };
diff --git a/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c b/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
index 485a1c5aa..162ff09a9 100644
--- a/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
+++ b/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
@@ -276,7 +276,6 @@ static void setup(void)
 	else if (tst_kvercmp(2, 6, 18) < 0)
 		tst_brk(TCONF, "2.6.18 or greater kernel required");
 
-	FILE_PRINTF("/proc/sys/kernel/numa_balancing", "0");
 	/*
 	 * find 2 nodes, which can hold NODE_MIN_FREEMEM bytes
 	 * The reason is that:
@@ -327,8 +326,8 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.test_all = run,
 	.setup = setup,
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/kernel/numa_balancing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/kernel/numa_balancing", "0"},
 		NULL,
 	},
 };
diff --git a/testcases/kernel/syscalls/sendto/sendto03.c b/testcases/kernel/syscalls/sendto/sendto03.c
index 217383993..4459f8849 100644
--- a/testcases/kernel/syscalls/sendto/sendto03.c
+++ b/testcases/kernel/syscalls/sendto/sendto03.c
@@ -217,8 +217,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt05.c b/testcases/kernel/syscalls/setsockopt/setsockopt05.c
index 4b8b3d22e..abc76627a 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt05.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt05.c
@@ -101,8 +101,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt06.c b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
index 644e61f3f..25dd04a15 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt06.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
@@ -129,8 +129,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt07.c b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
index 7385ce79e..b405c8ece 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt07.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
@@ -141,8 +141,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 5b648d754..49eb17b13 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -158,8 +158,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt09.c b/testcases/kernel/syscalls/setsockopt/setsockopt09.c
index 2fc66ebbc..53944247f 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt09.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt09.c
@@ -105,8 +105,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
-- 
2.31.1


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

Re: [LTP] [PATCH 3/4] testcase: switch to the new .save_restore

[Cyril Hrubis]

Hi!
Looks obviously correct, but should have been part of the previous
patch, otherwise we break bisectability.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 4/4] ksm: cleanup work and make use of .save_restore

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/mem/ksm/ksm01.c      | 38 ++++++++++-----------------
 testcases/kernel/mem/ksm/ksm02.c      | 25 +++++-------------
 testcases/kernel/mem/ksm/ksm03.c      | 25 +++++-------------
 testcases/kernel/mem/ksm/ksm04.c      | 24 +++++------------
 testcases/kernel/mem/ksm/ksm05.c      | 29 ++++++--------------
 testcases/kernel/mem/ksm/ksm_common.h |  2 --
 6 files changed, 43 insertions(+), 100 deletions(-)

diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index 0a81e2016..301c4d3c2 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -66,30 +66,7 @@ static void verify_ksm(void)
 
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
-
-	/*
-	 * kernel commit 90bd6fd introduced a new KSM sysfs knob
-	 * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
-	 * will prevent KSM pages being merged across numa nodes,
-	 * which will cause the case fail, so we need to make sure
-	 * it is enabled before testing.
-	 */
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-}
-
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
 }
 
 static struct tst_test test = {
@@ -102,11 +79,24 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		/*
+		 * kernel commit 90bd6fd introduced a new KSM sysfs knob
+		 * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
+		 * will prevent KSM pages being merged across numa nodes,
+		 * which will cause the case fail, so we need to make sure
+		 * it is enabled before testing.
+		 */
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 };
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 6ba6ee868..6ad7bcadb 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -82,26 +82,9 @@ static void verify_ksm(void)
 	SAFE_CG_PRINTF(tst_cg_drain, "cgroup.procs", "%d", getpid());
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-	}
-}
-
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
 }
 
 static struct tst_test test = {
@@ -114,11 +97,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){ "cpuset", NULL },
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 71d2d8bd9..94223e332 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -66,29 +66,12 @@ static void verify_ksm(void)
 
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-	}
-}
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
@@ -99,11 +82,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 8429f4843..3e8a77bec 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -82,24 +82,8 @@ static void verify_ksm(void)
 	create_same_memory(size, num, unit);
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-}
-
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
@@ -115,11 +99,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){
diff --git a/testcases/kernel/mem/ksm/ksm05.c b/testcases/kernel/mem/ksm/ksm05.c
index 380bb0202..33721d388 100644
--- a/testcases/kernel/mem/ksm/ksm05.c
+++ b/testcases/kernel/mem/ksm/ksm05.c
@@ -83,32 +83,19 @@ static void sighandler(int sig)
 	_exit((sig == SIGSEGV) ? 0 : sig);
 }
 
-static void setup(void)
-{
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	/* save original /sys/kernel/mm/ksm/run value */
-	SAFE_FILE_SCANF(PATH_KSM "run", "%d", &ksm_run_orig);
-
-	/* echo 1 > /sys/kernel/mm/ksm/run */
-	SAFE_FILE_PRINTF(PATH_KSM "run", "1");
-}
-
-static void cleanup(void)
-{
-	/* restore /sys/kernel/mm/ksm/run value */
-	if (ksm_run_orig > 0)
-		FILE_PRINTF(PATH_KSM "run", "%d", ksm_run_orig);
-}
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
-	.setup = setup,
-	.cleanup = cleanup,
 	.test_all = test_ksm,
 	.min_kver = "2.6.32",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", "1"},
+		NULL,
+	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 };
 
 #else
diff --git a/testcases/kernel/mem/ksm/ksm_common.h b/testcases/kernel/mem/ksm/ksm_common.h
index e6d5387c8..56faf01e4 100644
--- a/testcases/kernel/mem/ksm/ksm_common.h
+++ b/testcases/kernel/mem/ksm/ksm_common.h
@@ -9,8 +9,6 @@
 
 #include "tst_test.h"
 
-int merge_across_nodes;
-
 int size = 128, num = 3, unit = 1;
 char *opt_sizestr, *opt_numstr, *opt_unitstr;
 
-- 
2.31.1


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

Re: [LTP] [PATCH 4/4] ksm: cleanup work and make use of .save_restore

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
> index 0a81e2016..301c4d3c2 100644
> --- a/testcases/kernel/mem/ksm/ksm01.c
> +++ b/testcases/kernel/mem/ksm/ksm01.c
> @@ -66,30 +66,7 @@ static void verify_ksm(void)
>  
>  static void setup(void)
>  {
> -	if (access(PATH_KSM, F_OK) == -1)
> -		tst_brk(TCONF, "KSM configuration is not enabled");
> -
>  	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
> -
> -	/*
> -	 * kernel commit 90bd6fd introduced a new KSM sysfs knob
> -	 * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
> -	 * will prevent KSM pages being merged across numa nodes,
> -	 * which will cause the case fail, so we need to make sure
> -	 * it is enabled before testing.
> -	 */
> -	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
> -		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
> -				"%d", &merge_across_nodes);
> -		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
> -	}
> -}
> -
> -static void cleanup(void)
> -{
> -	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
> -		FILE_PRINTF(PATH_KSM "merge_across_nodes",
> -				 "%d", merge_across_nodes);
>  }
>  
>  static struct tst_test test = {
> @@ -102,11 +79,24 @@ static struct tst_test test = {
>  		{}
>  	},
>  	.setup = setup,
> -	.cleanup = cleanup,
>  	.save_restore = (const struct tst_path_val const[]) {
> +		{"!/sys/kernel/mm/ksm/run", NULL},
> +		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
>  		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
> +		/*
> +		 * kernel commit 90bd6fd introduced a new KSM sysfs knob
> +		 * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
> +		 * will prevent KSM pages being merged across numa nodes,
> +		 * which will cause the case fail, so we need to make sure
> +		 * it is enabled before testing.
> +		 */

I would have removed this comment, it's not in the rest of the tests
either way...

Otherwise:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2 1/3] lib: move struct tst_sys_conf to internal

[Li Wang]

As it only used in tst_sys_conf library internally, there is
unnecessary to put in header file.

Signed-off-by: Li Wang <liwang@redhat.com>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_sys_conf.h | 6 ------
 lib/tst_sys_conf.c     | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
index 323e29a27..507a552e8 100644
--- a/include/tst_sys_conf.h
+++ b/include/tst_sys_conf.h
@@ -5,12 +5,6 @@
 #ifndef TST_SYS_CONF_H__
 #define TST_SYS_CONF_H__
 
-struct tst_sys_conf {
-	char path[PATH_MAX];
-	char value[PATH_MAX];
-	struct tst_sys_conf *next;
-};
-
 int tst_sys_conf_save_str(const char *path, const char *value);
 int tst_sys_conf_save(const char *path);
 void tst_sys_conf_restore(int verbose);
diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
index 4ad9f8b9b..d7118f15f 100644
--- a/lib/tst_sys_conf.c
+++ b/lib/tst_sys_conf.c
@@ -12,6 +12,12 @@
 #include "tst_test.h"
 #include "tst_sys_conf.h"
 
+struct tst_sys_conf {
+	char path[PATH_MAX];
+	char value[PATH_MAX];
+	struct tst_sys_conf *next;
+};
+
 static struct tst_sys_conf *save_restore_data;
 
 void tst_sys_conf_dump(void)
-- 
2.31.1


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

[LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[Li Wang]

This extends that .save_restore support set new expected value after
saving the knob's original, which also avoids additionally checking
before using the file at other places.

And, export function tst_sys_conf_set() can be singly used for setting
new value of knob in the whole LTP.

Reported-by: Bruno Goncalves <bgoncalv@redhat.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Jan Stancek <jstancek@redhat.com>
---

Notes:
    v1 --> v2
        * refine the description in doc as suggested by Cyril
        * remove redundant file check in tst_sys_conf_set
        * merge patch 3/4 into 2/4 for bisectability reason

 doc/c-test-api.txt                               | 14 ++++++++------
 include/tst_sys_conf.h                           |  6 ++++++
 include/tst_test.h                               |  2 +-
 lib/newlib_tests/test19.c                        | 14 ++++++--------
 lib/newlib_tests/test20.c                        | 10 ++++------
 lib/tst_sys_conf.c                               | 10 ++++++++++
 lib/tst_test.c                                   |  9 +++++----
 testcases/cve/icmp_rate_limit01.c                |  4 ++--
 testcases/kernel/containers/userns/userns08.c    |  4 ++--
 testcases/kernel/mem/ksm/ksm01.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm02.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm03.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm04.c                 |  4 ++--
 testcases/kernel/mem/ksm/ksm06.c                 | 10 +++++-----
 testcases/kernel/syscalls/add_key/add_key05.c    | 16 ++++------------
 testcases/kernel/syscalls/bind/bind06.c          |  4 ++--
 testcases/kernel/syscalls/madvise/madvise08.c    |  4 ++--
 .../syscalls/migrate_pages/migrate_pages02.c     |  5 ++---
 testcases/kernel/syscalls/sendto/sendto03.c      |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt05.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt06.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt07.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt08.c    |  4 ++--
 .../kernel/syscalls/setsockopt/setsockopt09.c    |  4 ++--
 24 files changed, 79 insertions(+), 73 deletions(-)

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 28383ccee..915255176 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -1561,10 +1561,12 @@ itself is not available on the system.
 
 LTP library can be instructed to save and restore value of specified
 (/proc|sys) files. This is achieved by initialized tst_test struct
-field 'save_restore'. It is a 'NULL' terminated array of strings where
-each string represents a file, whose value is saved at the beginning
-and restored at the end of the test. Only first line of a specified
-file is saved and restored.
+field 'save_restore'. It is a NULL-terminated array of struct
+'tst_path_val' where each tst_path_val.path represents a file, whose
+value is saved at the beginning and restored at the end of the test.
+If non-NULL value is passed it is written to the respective file at
+the beginning of the test. Only the first line of a specified file
+is saved and restored.
 
 Pathnames can be optionally prefixed to specify how strictly (during
 'store') are handled errors:
@@ -1578,8 +1580,8 @@ Pathnames can be optionally prefixed to specify how strictly (during
 
 [source,c]
 -------------------------------------------------------------------------------
-static const char *save_restore[] = {
-	"/proc/sys/kernel/core_pattern",
+static const struct tst_path_val save_restore[] = {
+	{"/proc/sys/kernel/core_pattern", NULL},
 	NULL,
 };
 
diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
index 507a552e8..b7bbe36fc 100644
--- a/include/tst_sys_conf.h
+++ b/include/tst_sys_conf.h
@@ -5,8 +5,14 @@
 #ifndef TST_SYS_CONF_H__
 #define TST_SYS_CONF_H__
 
+struct tst_path_val {
+        const char *path;
+        const char *val;
+};
+
 int tst_sys_conf_save_str(const char *path, const char *value);
 int tst_sys_conf_save(const char *path);
+void tst_sys_conf_set(const char *path, const char *value);
 void tst_sys_conf_restore(int verbose);
 void tst_sys_conf_dump(void);
 
diff --git a/include/tst_test.h b/include/tst_test.h
index 816fab4dd..e514efa76 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -259,7 +259,7 @@ struct tst_test {
 	 * NULL terminated array of (/proc, /sys) files to save
 	 * before setup and restore after cleanup
 	 */
-	const char * const *save_restore;
+	const struct tst_path_val const *save_restore;
 
 	/*
 	 * NULL terminated array of kernel config options required for the
diff --git a/lib/newlib_tests/test19.c b/lib/newlib_tests/test19.c
index 78b5202d2..402a2ad30 100644
--- a/lib/newlib_tests/test19.c
+++ b/lib/newlib_tests/test19.c
@@ -8,13 +8,6 @@
 #include "tst_test.h"
 #include "tst_sys_conf.h"
 
-static const char * const save_restore[] = {
-	"?/proc/nonexistent",
-	"!/proc/sys/kernel/numa_balancing",
-	"/proc/sys/kernel/core_pattern",
-	NULL,
-};
-
 static void setup(void)
 {
 	SAFE_FILE_PRINTF("/proc/sys/kernel/core_pattern", "changed");
@@ -30,5 +23,10 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.test_all = run,
 	.setup = setup,
-	.save_restore = save_restore,
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/nonexistent", NULL},
+		{"!/proc/sys/kernel/numa_balancing", NULL},
+		{"/proc/sys/kernel/core_pattern", NULL},
+		NULL,
+	},
 };
diff --git a/lib/newlib_tests/test20.c b/lib/newlib_tests/test20.c
index 53317b669..5feb7c144 100644
--- a/lib/newlib_tests/test20.c
+++ b/lib/newlib_tests/test20.c
@@ -11,11 +11,6 @@
 #include "tst_hugepage.h"
 #include "tst_sys_conf.h"
 
-static const char * const save_restore[] = {
-	"!/proc/sys/kernel/numa_balancing",
-	NULL,
-};
-
 static void do_test(void) {
 
 	unsigned long val, hpages;
@@ -41,5 +36,8 @@ static void do_test(void) {
 static struct tst_test test = {
 	.test_all = do_test,
 	.request_hugepages = 2,
-	.save_restore = save_restore,
+	.save_restore = (const struct tst_path_val const[]) {
+		{"!/proc/sys/kernel/numa_balancing", "0"},
+		NULL,
+	},
 };
diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
index d7118f15f..003698825 100644
--- a/lib/tst_sys_conf.c
+++ b/lib/tst_sys_conf.c
@@ -96,6 +96,16 @@ int tst_sys_conf_save(const char *path)
 	return tst_sys_conf_save_str(path, line);
 }
 
+void tst_sys_conf_set(const char *path, const char *value)
+{
+	char flag = path[0];
+	if (flag  == '?' || flag == '!')
+		path++;
+
+	if (value)
+		SAFE_FILE_PRINTF(path, "%s", value);
+}
+
 void tst_sys_conf_restore(int verbose)
 {
 	struct tst_sys_conf *i;
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 9e745c537..fe2e2bb6c 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1105,11 +1105,12 @@ static void do_setup(int argc, char *argv[])
 		tst_tmpdir();
 
 	if (tst_test->save_restore) {
-		const char * const *name = tst_test->save_restore;
+		const struct tst_path_val const *pvl = tst_test->save_restore;
 
-		while (*name) {
-			tst_sys_conf_save(*name);
-			name++;
+		while (pvl->path) {
+			if (!tst_sys_conf_save(pvl->path))
+				tst_sys_conf_set(pvl->path, pvl->val);
+			pvl++;
 		}
 	}
 
diff --git a/testcases/cve/icmp_rate_limit01.c b/testcases/cve/icmp_rate_limit01.c
index 3ada32675..81735ce18 100644
--- a/testcases/cve/icmp_rate_limit01.c
+++ b/testcases/cve/icmp_rate_limit01.c
@@ -268,8 +268,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/containers/userns/userns08.c b/testcases/kernel/containers/userns/userns08.c
index c141b1aca..33d0a9997 100644
--- a/testcases/kernel/containers/userns/userns08.c
+++ b/testcases/kernel/containers/userns/userns08.c
@@ -134,8 +134,8 @@ static struct tst_test test = {
 		"CONFIG_USER_NS",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index 7470d3143..0a81e2016 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -103,8 +103,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 76a87c6b1..6ba6ee868 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -115,8 +115,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 6a0566f43..71d2d8bd9 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -100,8 +100,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 77fafa641..8429f4843 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -116,8 +116,8 @@ static struct tst_test test = {
 	},
 	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
 		NULL,
 	},
 	.test_all = verify_ksm,
diff --git a/testcases/kernel/mem/ksm/ksm06.c b/testcases/kernel/mem/ksm/ksm06.c
index 6a74b7406..f28383cc0 100644
--- a/testcases/kernel/mem/ksm/ksm06.c
+++ b/testcases/kernel/mem/ksm/ksm06.c
@@ -136,11 +136,11 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.save_restore = (const char * const[]) {
-		"?/sys/kernel/mm/ksm/max_page_sharing",
-		"!/sys/kernel/mm/ksm/run",
-		"!/sys/kernel/mm/ksm/sleep_millisecs",
-		"/sys/kernel/mm/ksm/merge_across_nodes",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
+		{"/sys/kernel/mm/ksm/merge_across_nodes", NULL},
 		NULL,
 	},
 	.needs_kconfigs = (const char *const[]){
diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
index 2be1b7014..bbe44e043 100644
--- a/testcases/kernel/syscalls/add_key/add_key05.c
+++ b/testcases/kernel/syscalls/add_key/add_key05.c
@@ -202,13 +202,6 @@ static void do_test(unsigned int n)
 	return;
 }
 
-static void setup(void)
-{
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/gc_delay", "1");
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxkeys", "200");
-	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxbytes", "20000");
-}
-
 static void cleanup(void)
 {
 	while (usern--)
@@ -220,12 +213,11 @@ static struct tst_test test = {
 	.tcnt = 2,
 	.needs_root = 1,
 	.forks_child = 1,
-	.setup = setup,
 	.cleanup = cleanup,
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/kernel/keys/gc_delay",
-		"?/proc/sys/kernel/keys/maxkeys",
-		"?/proc/sys/kernel/keys/maxbytes",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/kernel/keys/gc_delay", "1"},
+		{"?/proc/sys/kernel/keys/maxkeys", "200"},
+		{"?/proc/sys/kernel/keys/maxbytes", "20000"},
 		NULL,
 	},
 	.bufs = (struct tst_buffers []) {
diff --git a/testcases/kernel/syscalls/bind/bind06.c b/testcases/kernel/syscalls/bind/bind06.c
index 297311c04..7a84b19cd 100644
--- a/testcases/kernel/syscalls/bind/bind06.c
+++ b/testcases/kernel/syscalls/bind/bind06.c
@@ -109,8 +109,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index ff167dafe..e513d558a 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -213,8 +213,8 @@ static struct tst_test test = {
 	.needs_tmpdir = 1,
 	.needs_root = 1,
 	.forks_child = 1,
-	.save_restore = (const char * const[]) {
-		CORE_PATTERN,
+	.save_restore = (const struct tst_path_val const[]) {
+		{CORE_PATTERN, NULL},
 		NULL,
 	},
 };
diff --git a/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c b/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
index 485a1c5aa..162ff09a9 100644
--- a/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
+++ b/testcases/kernel/syscalls/migrate_pages/migrate_pages02.c
@@ -276,7 +276,6 @@ static void setup(void)
 	else if (tst_kvercmp(2, 6, 18) < 0)
 		tst_brk(TCONF, "2.6.18 or greater kernel required");
 
-	FILE_PRINTF("/proc/sys/kernel/numa_balancing", "0");
 	/*
 	 * find 2 nodes, which can hold NODE_MIN_FREEMEM bytes
 	 * The reason is that:
@@ -327,8 +326,8 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.test_all = run,
 	.setup = setup,
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/kernel/numa_balancing",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/kernel/numa_balancing", "0"},
 		NULL,
 	},
 };
diff --git a/testcases/kernel/syscalls/sendto/sendto03.c b/testcases/kernel/syscalls/sendto/sendto03.c
index 217383993..4459f8849 100644
--- a/testcases/kernel/syscalls/sendto/sendto03.c
+++ b/testcases/kernel/syscalls/sendto/sendto03.c
@@ -217,8 +217,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt05.c b/testcases/kernel/syscalls/setsockopt/setsockopt05.c
index 4b8b3d22e..abc76627a 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt05.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt05.c
@@ -101,8 +101,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt06.c b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
index 644e61f3f..25dd04a15 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt06.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt06.c
@@ -129,8 +129,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt07.c b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
index 7385ce79e..b405c8ece 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt07.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt07.c
@@ -141,8 +141,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 5b648d754..49eb17b13 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -158,8 +158,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt09.c b/testcases/kernel/syscalls/setsockopt/setsockopt09.c
index 2fc66ebbc..53944247f 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt09.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt09.c
@@ -105,8 +105,8 @@ static struct tst_test test = {
 		"CONFIG_NET_NS=y",
 		NULL
 	},
-	.save_restore = (const char * const[]) {
-		"?/proc/sys/user/max_user_namespaces",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"?/proc/sys/user/max_user_namespaces", NULL},
 		NULL,
 	},
 	.tags = (const struct tst_tag[]) {
-- 
2.31.1


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

Re: [LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 173 bytes --]

On Wed, Mar 9, 2022 at 5:58 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
>

Patchset applied, thanks!

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 877 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[xuyang2018.jy]

Hi Li
>
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 816fab4dd..e514efa76 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -259,7 +259,7 @@ struct tst_test {
>   	 * NULL terminated array of (/proc, /sys) files to save
>   	 * before setup and restore after cleanup
>   	 */
> -	const char * const *save_restore;
> +	const struct tst_path_val const *save_restore;
Ci complains about duplicate const see the following url
https://github.com/linux-test-project/ltp/runs/5491806027?check_suite_focus=true#step:10:19

How about using const struct tst_path_val *save_restore like
other struct pointer( struct tst_cap *caps, const struct tst_tag *tags)?

Best Regards
Yang Xu

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

Re: [LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 732 bytes --]

> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -259,7 +259,7 @@ struct tst_test {
> >        * NULL terminated array of (/proc, /sys) files to save
> >        * before setup and restore after cleanup
> >        */
> > -     const char * const *save_restore;
> > +     const struct tst_path_val const *save_restore;
> Ci complains about duplicate const see the following url
>
> https://github.com/linux-test-project/ltp/runs/5491806027?check_suite_focus=true#step:10:19
>
> How about using const struct tst_path_val *save_restore like
> other struct pointer( struct tst_cap *caps, const struct tst_tag *tags)?
>

Yes, good catch. Can you send a patch to fix the compiling issue or I do
that?

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1378 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 2/3] lib: enhance .save_restore to support set expected value

[xuyang2018.jy]

Hi Li
>      > --- a/include/tst_test.h
>      > +++ b/include/tst_test.h
>      > @@ -259,7 +259,7 @@ struct tst_test {
>      > * NULL terminated array of (/proc, /sys) files to save
>      > * before setup and restore after cleanup
>      > */
>      > - const char * const *save_restore;
>      > + const struct tst_path_val const *save_restore;
>     Ci complains about duplicate const see the following url
>     https://github.com/linux-test-project/ltp/runs/5491806027?check_suite_focus=true#step:10:19
>
>     How about using const struct tst_path_val *save_restore like
>     other struct pointer( struct tst_cap *caps, const struct tst_tag *tags)?
>
>
> Yes, good catch. Can you send a patch to fix the compiling issue or I do
> that?
Of course, you can do it.

Best Regards
Yang Xu
>
> --
> Regards,
> Li Wang

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

[LTP] [PATCH v2 3/3] ksm: cleanup work and make use of .save_restore

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
---

Notes:
    v1 --> v2
        remove the code comments from ksm01.c

 testcases/kernel/mem/ksm/ksm01.c      | 31 ++++++---------------------
 testcases/kernel/mem/ksm/ksm02.c      | 25 ++++++---------------
 testcases/kernel/mem/ksm/ksm03.c      | 25 ++++++---------------
 testcases/kernel/mem/ksm/ksm04.c      | 24 ++++++---------------
 testcases/kernel/mem/ksm/ksm05.c      | 29 +++++++------------------
 testcases/kernel/mem/ksm/ksm_common.h |  2 --
 6 files changed, 36 insertions(+), 100 deletions(-)

diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
index 0a81e2016..305bf30ea 100644
--- a/testcases/kernel/mem/ksm/ksm01.c
+++ b/testcases/kernel/mem/ksm/ksm01.c
@@ -66,30 +66,7 @@ static void verify_ksm(void)
 
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
-
-	/*
-	 * kernel commit 90bd6fd introduced a new KSM sysfs knob
-	 * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
-	 * will prevent KSM pages being merged across numa nodes,
-	 * which will cause the case fail, so we need to make sure
-	 * it is enabled before testing.
-	 */
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-}
-
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
 }
 
 static struct tst_test test = {
@@ -102,11 +79,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 };
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 6ba6ee868..6ad7bcadb 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -82,26 +82,9 @@ static void verify_ksm(void)
 	SAFE_CG_PRINTF(tst_cg_drain, "cgroup.procs", "%d", getpid());
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-	}
-}
-
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
 }
 
 static struct tst_test test = {
@@ -114,11 +97,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){ "cpuset", NULL },
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 71d2d8bd9..94223e332 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -66,29 +66,12 @@ static void verify_ksm(void)
 
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-	}
-}
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
@@ -99,11 +82,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index 8429f4843..3e8a77bec 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -82,24 +82,8 @@ static void verify_ksm(void)
 	create_same_memory(size, num, unit);
 }
 
-static void cleanup(void)
-{
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
-		FILE_PRINTF(PATH_KSM "merge_across_nodes",
-				 "%d", merge_across_nodes);
-}
-
 static void setup(void)
 {
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
-		SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
-				"%d", &merge_across_nodes);
-		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
-	}
-
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
@@ -115,11 +99,17 @@ static struct tst_test test = {
 		{}
 	},
 	.setup = setup,
-	.cleanup = cleanup,
 	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", NULL},
+		{"!/sys/kernel/mm/ksm/sleep_millisecs", NULL},
 		{"?/sys/kernel/mm/ksm/max_page_sharing", NULL},
+		{"?/sys/kernel/mm/ksm/merge_across_nodes", "1"},
 		NULL,
 	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 	.test_all = verify_ksm,
 	.min_kver = "2.6.32",
 	.needs_cgroup_ctrls = (const char *const []){
diff --git a/testcases/kernel/mem/ksm/ksm05.c b/testcases/kernel/mem/ksm/ksm05.c
index 380bb0202..33721d388 100644
--- a/testcases/kernel/mem/ksm/ksm05.c
+++ b/testcases/kernel/mem/ksm/ksm05.c
@@ -83,32 +83,19 @@ static void sighandler(int sig)
 	_exit((sig == SIGSEGV) ? 0 : sig);
 }
 
-static void setup(void)
-{
-	if (access(PATH_KSM, F_OK) == -1)
-		tst_brk(TCONF, "KSM configuration is not enabled");
-
-	/* save original /sys/kernel/mm/ksm/run value */
-	SAFE_FILE_SCANF(PATH_KSM "run", "%d", &ksm_run_orig);
-
-	/* echo 1 > /sys/kernel/mm/ksm/run */
-	SAFE_FILE_PRINTF(PATH_KSM "run", "1");
-}
-
-static void cleanup(void)
-{
-	/* restore /sys/kernel/mm/ksm/run value */
-	if (ksm_run_orig > 0)
-		FILE_PRINTF(PATH_KSM "run", "%d", ksm_run_orig);
-}
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
-	.setup = setup,
-	.cleanup = cleanup,
 	.test_all = test_ksm,
 	.min_kver = "2.6.32",
+	.save_restore = (const struct tst_path_val const[]) {
+		{"!/sys/kernel/mm/ksm/run", "1"},
+		NULL,
+	},
+	.needs_kconfigs = (const char *const[]){
+		"CONFIG_KSM=y",
+		NULL
+	},
 };
 
 #else
diff --git a/testcases/kernel/mem/ksm/ksm_common.h b/testcases/kernel/mem/ksm/ksm_common.h
index e6d5387c8..56faf01e4 100644
--- a/testcases/kernel/mem/ksm/ksm_common.h
+++ b/testcases/kernel/mem/ksm/ksm_common.h
@@ -9,8 +9,6 @@
 
 #include "tst_test.h"
 
-int merge_across_nodes;
-
 int size = 128, num = 3, unit = 1;
 char *opt_sizestr, *opt_numstr, *opt_unitstr;
 
-- 
2.31.1


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