[LTP] [PATCH 1/3] swapping01: skip test if zram-swap is being used

[LTP] [PATCH 1/3] swapping01: skip test if zram-swap is being used

[Yang Xu]

For the swapping test we attempt to allocate 130% of the available RAM and
we make sure that the overflow would fit the swap, but as long as swap is
backed by RAM this obviously false. So skip it if zram-swap is being used.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/mem/swapping/swapping01.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
index 392b79d65..29c1e0443 100644
--- a/testcases/kernel/mem/swapping/swapping01.c
+++ b/testcases/kernel/mem/swapping/swapping01.c
@@ -67,6 +67,11 @@ static void test_swapping(void)
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
+	int ret;
+
+	ret = tst_system("zramctl | grep SWAP");
+	if (!ret)
+		tst_brk(TCONF, "zram-swap is being used!");
 
 	init_meminfo();
 
@@ -155,4 +160,8 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
 	.test_all = test_swapping,
+	.needs_cmds = (const char *[]) {
+		"zramctl",
+		NULL
+	}
 };
-- 
2.23.0


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

[LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Yang Xu]

If zram-generator package is installed and works, then we can not
remove zram module because zram swap is being used. We can also use zramstart
command or similar shell script(use zramctl,mkswap,swapon) to do this.

Since zram01.sh and zram02.sh needs to rmmod and modprobe, they can't work well
if zram module can't be removed. So skip it.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index fe9c915c3..ad5b6e216 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -49,6 +49,8 @@ zram_load()
 
 	tst_set_timeout $((dev_num*450))
 
+	rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
+
 	tst_res TINFO "create '$dev_num' zram device(s)"
 
 	modprobe zram num_devices=$dev_num || \
@@ -57,7 +59,8 @@ zram_load()
 	dev_num_created=$(ls /dev/zram* | wc -w)
 
 	if [ "$dev_num_created" -ne "$dev_num" ]; then
-		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
+		tst_brk FAIL "num of devices expected $dev_num, but created"\
+				"$dev_num_created"
 	fi
 
 	tst_res TPASS "all zram devices successfully created"
-- 
2.23.0


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

[LTP] [PATCH 3/3] zram/zram03: Convert into new api

[Yang Xu]

Also add removing zram module step in setup, so we can avoid the situation that
zram module is being used by zram-generator.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/device-drivers/zram/zram03.c | 221 +++++++-----------
 1 file changed, 90 insertions(+), 131 deletions(-)

diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
index 06995fd56..0cc6200a1 100644
--- a/testcases/kernel/device-drivers/zram/zram03.c
+++ b/testcases/kernel/device-drivers/zram/zram03.c
@@ -1,29 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (C) 2010  Red Hat, Inc.
+ */
+
+/*\
+ * [Description]
+ * 
  * zram: generic RAM based compressed R/W block devices
  * http://lkml.org/lkml/2010/8/9/227
- *
- * Copyright (C) 2010  Red Hat, Inc.
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
+ * 
+ * This case check whether data read from zram device is consistent with 
+ * thoese are written.
  */
 
+
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
@@ -32,12 +22,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
-
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "zram03";
-int TST_TOTAL = 1;
+#include "tst_test.h"
 
 #define PATH_ZRAM	"/sys/block/zram0"
 #define OBSOLETE_ZRAM_FILE	"/sys/block/zram0/num_reads"
@@ -47,44 +32,12 @@ int TST_TOTAL = 1;
 #define DEVICE		"/dev/zram0"
 
 static int modprobe;
-
-static void set_disksize(void);
-static void write_device(void);
-static void verify_device(void);
-static void reset(void);
-static void setup(void);
-static void cleanup(void);
-static void print(char *string);
-static void dump_info(void);
-
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		set_disksize();
-
-		write_device();
-		dump_info();
-		verify_device();
-
-		reset();
-		dump_info();
-	}
-	cleanup();
-	tst_exit();
-}
+static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
 
 static void set_disksize(void)
 {
-	tst_resm(TINFO, "create a zram device with %ld bytes in size.", SIZE);
-	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/disksize", "%ld", SIZE);
+	tst_res(TINFO, "create a zram device with %ld bytes in size.", SIZE);
+	SAFE_FILE_PRINTF(PATH_ZRAM "/disksize", "%ld", SIZE);
 }
 
 static void write_device(void)
@@ -92,17 +45,16 @@ static void write_device(void)
 	int fd;
 	char *s;
 
-	tst_resm(TINFO, "map this zram device into memory.");
-	fd = SAFE_OPEN(cleanup, DEVICE, O_RDWR);
-	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ | PROT_WRITE,
-		      MAP_SHARED, fd, 0);
+	tst_res(TINFO, "map this zram device into memory.");
+	fd = SAFE_OPEN(DEVICE, O_RDWR);
+	s = SAFE_MMAP(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 
-	tst_resm(TINFO, "write all the memory.");
+	tst_res(TINFO, "write all the memory.");
 	memset(s, 'a', SIZE - 1);
 	s[SIZE - 1] = '\0';
 
-	SAFE_MUNMAP(cleanup, s, SIZE);
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_MUNMAP(s, SIZE);
+	SAFE_CLOSE(fd);
 }
 
 static void verify_device(void)
@@ -111,9 +63,9 @@ static void verify_device(void)
 	long i = 0, fail = 0;
 	char *s;
 
-	tst_resm(TINFO, "verify contents from device.");
-	fd = SAFE_OPEN(cleanup, DEVICE, O_RDONLY);
-	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ, MAP_PRIVATE, fd, 0);
+	tst_res(TINFO, "verify contents from device.");
+	fd = SAFE_OPEN(DEVICE, O_RDONLY);
+	s = SAFE_MMAP(NULL, SIZE, PROT_READ, MAP_PRIVATE, fd, 0);
 
 	while (s[i] && i < SIZE - 1) {
 		if (s[i] != 'a')
@@ -121,59 +73,25 @@ static void verify_device(void)
 		i++;
 	}
 	if (i != SIZE - 1) {
-		tst_resm(TFAIL, "expect size: %ld, actual size: %ld.",
+		tst_res(TFAIL, "expect size: %ld, actual size: %ld.",
 			 SIZE - 1, i);
 	} else if (s[i] != '\0') {
-		tst_resm(TFAIL, "zram device seems not null terminated");
+		tst_res(TFAIL, "zram device seems not null terminated");
 	} else if (fail) {
-		tst_resm(TFAIL, "%ld failed bytes found.", fail);
+		tst_res(TFAIL, "%ld failed bytes found.", fail);
 	} else {
-		tst_resm(TPASS, "data read from zram device is consistent "
+		tst_res(TPASS, "data read from zram device is consistent "
 			 "with those are written");
 	}
 
-	SAFE_MUNMAP(cleanup, s, SIZE);
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_MUNMAP(s, SIZE);
+	SAFE_CLOSE(fd);
 }
 
 static void reset(void)
 {
-	tst_resm(TINFO, "reset it.");
-	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/reset", "1");
-}
-
-static void setup(void)
-{
-	int retried = 0;
-
-	tst_require_root();
-
-retry:
-	if (access(PATH_ZRAM, F_OK) == -1) {
-		if (errno == ENOENT) {
-			if (retried) {
-				tst_brkm(TCONF, NULL,
-					 "system has no zram device.");
-			}
-			if (system("modprobe zram") == -1) {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					 "system(modprobe zram) failed");
-			}
-			modprobe = 1;
-			retried = 1;
-			goto retry;
-		} else
-			tst_brkm(TBROK | TERRNO, NULL, "access");
-	}
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
-}
-
-static void cleanup(void)
-{
-	if (modprobe == 1 && system("rmmod zram") == -1)
-		tst_resm(TWARN | TERRNO, "system(rmmod zram) failed");
+	tst_res(TINFO, "%s...", __func__);
+	SAFE_FILE_PRINTF(PATH_ZRAM "/reset", "1");
 }
 
 static void print(char *string)
@@ -181,20 +99,17 @@ static void print(char *string)
 	char filename[BUFSIZ], value[BUFSIZ];
 
 	sprintf(filename, "%s/%s", PATH_ZRAM, string);
-	SAFE_FILE_SCANF(cleanup, filename, "%s", value);
-	tst_resm(TINFO, "%s is %s", filename, value);
+	SAFE_FILE_SCANF(filename, "%s", value);
+	tst_res(TINFO, "%s is %s", filename, value);
 }
 
 static void print_stat(char *nread, char *nwrite)
 {
 	char nread_val[BUFSIZ], nwrite_val[BUFSIZ];
 
-	SAFE_FILE_SCANF(cleanup, PATH_ZRAM_STAT, "%s %*s %*s %*s %s",
-			nread_val, nwrite_val);
-	tst_resm(TINFO, "%s from %s is %s", nread, PATH_ZRAM_STAT,
-		 nread_val);
-	tst_resm(TINFO, "%s from %s is %s", nwrite, PATH_ZRAM_STAT,
-		 nwrite_val);
+	SAFE_FILE_SCANF(PATH_ZRAM_STAT, "%s %*s %*s %*s %s", nread_val, nwrite_val);
+	tst_res(TINFO, "%s from %s is %s", nread, PATH_ZRAM_STAT, nread_val);
+	tst_res(TINFO, "%s from %s is %s", nwrite, PATH_ZRAM_STAT, nwrite_val);
 }
 
 static void print_mm_stat(char *orig, char *compr, char *mem, char *zero)
@@ -202,16 +117,12 @@ static void print_mm_stat(char *orig, char *compr, char *mem, char *zero)
 	char orig_val[BUFSIZ], compr_val[BUFSIZ];
 	char mem_val[BUFSIZ], zero_val[BUFSIZ];
 
-	SAFE_FILE_SCANF(cleanup, PATH_ZRAM_MM_STAT, "%s %s %s %*s %*s %s",
+	SAFE_FILE_SCANF(PATH_ZRAM_MM_STAT, "%s %s %s %*s %*s %s",
 			orig_val, compr_val, mem_val, zero_val);
-	tst_resm(TINFO, "%s from %s is %s", orig, PATH_ZRAM_MM_STAT,
-		 orig_val);
-	tst_resm(TINFO, "%s from %s is %s", compr, PATH_ZRAM_MM_STAT,
-		compr_val);
-	tst_resm(TINFO, "%s from %s is %s", mem, PATH_ZRAM_MM_STAT,
-		 mem_val);
-	tst_resm(TINFO, "%s from %s is %s", zero, PATH_ZRAM_MM_STAT,
-		 zero_val);
+	tst_res(TINFO, "%s from %s is %s", orig, PATH_ZRAM_MM_STAT, orig_val);
+	tst_res(TINFO, "%s from %s is %s", compr, PATH_ZRAM_MM_STAT, compr_val);
+	tst_res(TINFO, "%s from %s is %s", mem, PATH_ZRAM_MM_STAT, mem_val);
+	tst_res(TINFO, "%s from %s is %s", zero, PATH_ZRAM_MM_STAT, zero_val);
 }
 
 static void dump_info(void)
@@ -231,3 +142,51 @@ static void dump_info(void)
 		print_stat("num_reads", "num_writes");
 	}
 }
+
+static void run(void)
+{
+	set_disksize();
+
+	write_device();
+	dump_info();
+	verify_device();
+
+	reset();
+	dump_info();
+}
+
+static void setup(void)
+{
+	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
+
+	if (tst_cmd(cmd_rmmod, NULL, NULL, TST_CMD_PASS_RETVAL)) {
+		if (errno == EBUSY)
+			tst_brk(TCONF, "zram module may being used!");
+	}
+	if (errno == ENOENT)
+		SAFE_CMD(cmd_modprobe, NULL, NULL);
+
+	modprobe = 1;
+}
+
+static void cleanup(void)
+{
+	if (modprobe == 1)
+		SAFE_CMD(cmd_rmmod, NULL, NULL);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_drivers = (const char *const []) {
+		"zram",
+		NULL
+	},
+	.needs_cmds = (const char *[]) {
+		"modprobe",
+		"rmmod",
+		NULL
+	}
+};
-- 
2.23.0


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

Re: [LTP] [PATCH 1/3] swapping01: skip test if zram-swap is being used

[Cyril Hrubis]

Hi!
> ---
>  testcases/kernel/mem/swapping/swapping01.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
> index 392b79d65..29c1e0443 100644
> --- a/testcases/kernel/mem/swapping/swapping01.c
> +++ b/testcases/kernel/mem/swapping/swapping01.c
> @@ -67,6 +67,11 @@ static void test_swapping(void)
>  #ifdef TST_ABI32
>  	tst_brk(TCONF, "test is not designed for 32-bit system.");
>  #endif
> +	int ret;
> +
> +	ret = tst_system("zramctl | grep SWAP");
> +	if (!ret)
> +		tst_brk(TCONF, "zram-swap is being used!");

Actually what the zramctl does is to open the /proc/swaps file and looks
for the /dev/zram* in there. I guess that we can as well read the the
file ourselves...

>  	init_meminfo();
>  
> @@ -155,4 +160,8 @@ static struct tst_test test = {
>  	.needs_root = 1,
>  	.forks_child = 1,
>  	.test_all = test_swapping,
> +	.needs_cmds = (const char *[]) {
> +		"zramctl",
> +		NULL
> +	}
>  };
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Cyril Hrubis]

Hi!
>  	tst_set_timeout $((dev_num*450))
>  
> +	rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> +
>  	tst_res TINFO "create '$dev_num' zram device(s)"
>  
>  	modprobe zram num_devices=$dev_num || \
> @@ -57,7 +59,8 @@ zram_load()
>  	dev_num_created=$(ls /dev/zram* | wc -w)
>  
>  	if [ "$dev_num_created" -ne "$dev_num" ]; then
> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
> +		tst_brk FAIL "num of devices expected $dev_num, but created"\
                        ^
			This should have stayed TFAIL right?
> +				"$dev_num_created"
>  	fi
>  
>  	tst_res TPASS "all zram devices successfully created"
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 1/3] swapping01: skip test if zram-swap is being used

[Petr Vorel]

Hi Xu, Cyril,

> Hi!
> > ---
> >  testcases/kernel/mem/swapping/swapping01.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)

> > diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
> > index 392b79d65..29c1e0443 100644
> > --- a/testcases/kernel/mem/swapping/swapping01.c
> > +++ b/testcases/kernel/mem/swapping/swapping01.c
> > @@ -67,6 +67,11 @@ static void test_swapping(void)
> >  #ifdef TST_ABI32
> >  	tst_brk(TCONF, "test is not designed for 32-bit system.");
> >  #endif
> > +	int ret;
> > +
> > +	ret = tst_system("zramctl | grep SWAP");
> > +	if (!ret)
> > +		tst_brk(TCONF, "zram-swap is being used!");

> Actually what the zramctl does is to open the /proc/swaps file and looks
> for the /dev/zram* in there. I guess that we can as well read the the
> file ourselves...

+1
Because requiring zramctl disables this test for all systems without it.


Kind regards,
Petr

> >  	init_meminfo();

> > @@ -155,4 +160,8 @@ static struct tst_test test = {
> >  	.needs_root = 1,
> >  	.forks_child = 1,
> >  	.test_all = test_swapping,
> > +	.needs_cmds = (const char *[]) {
> > +		"zramctl",
> > +		NULL
> > +	}
> >  };

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Petr Vorel]

Hi Xu, Cyril,

> Hi!
> >  	tst_set_timeout $((dev_num*450))

> > +	rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
This is wrong:

$ lsmod | grep zram
# rmmod zram; echo $?
rmmod: ERROR: Module zram is not currently loaded
1

Why not detect with lsmod? i.e.:

	if lsmod | grep zram; then
		rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
	fi

Although I'm not sure if we should attempt to rmmod, maybe quit when lsmod
detects zram would be better.

Also it wouldn't harm to add at least rmmod into TST_NEEDS_CMDS (in the
zram_lib.sh + take into account TST_NEEDS_CMDS from tests), but that's another
story.

Kind regards,
Petr

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

Re: [LTP] [PATCH 3/3] zram/zram03: Convert into new api

[Petr Vorel]

Hi Xu,

> Also add removing zram module step in setup, so we can avoid the situation that
> zram module is being used by zram-generator.

...
> +++ b/testcases/kernel/device-drivers/zram/zram03.c
...
> +static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};

Again, here should be lsmod used.

Except this LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>


>  static void set_disksize(void)
>  {
> -	tst_resm(TINFO, "create a zram device with %ld bytes in size.", SIZE);
> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/disksize", "%ld", SIZE);
> +	tst_res(TINFO, "create a zram device with %ld bytes in size.", SIZE);
> +	SAFE_FILE_PRINTF(PATH_ZRAM "/disksize", "%ld", SIZE);
>  }

>  static void write_device(void)
> @@ -92,17 +45,16 @@ static void write_device(void)
>  	int fd;
>  	char *s;

> -	tst_resm(TINFO, "map this zram device into memory.");
> -	fd = SAFE_OPEN(cleanup, DEVICE, O_RDWR);
> -	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ | PROT_WRITE,
> -		      MAP_SHARED, fd, 0);
> +	tst_res(TINFO, "map this zram device into memory.");
> +	fd = SAFE_OPEN(DEVICE, O_RDWR);
> +	s = SAFE_MMAP(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);

> -	tst_resm(TINFO, "write all the memory.");
> +	tst_res(TINFO, "write all the memory.");
very nit: while at it, could you please remove unnecessary dot at the end?

...

>  static void reset(void)
>  {
> -	tst_resm(TINFO, "reset it.");
> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/reset", "1");
> -}
...
> +	tst_res(TINFO, "%s...", __func__);
> +	SAFE_FILE_PRINTF(PATH_ZRAM "/reset", "1");
>  }

checkpatch.pl complains:

$ make check-zram03
zram03.c:93: WARNING: Unnecessary ftrace-like logging - prefer using ftrace
=> nit, but checkpatch.pl is correct, previous info was better than tracing
function name. How about keep info instead of function name, e.g.:
tst_res(TINFO, "reset zram");

zram03.c:94: WARNING: Prefer using '"%s...", __func__' to using 'reset', this function's name, in a string
false positive (not sure if we should bother with renaming function to keep it quiet).

...
> +static void setup(void)
> +{
> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
> +
> +	if (tst_cmd(cmd_rmmod, NULL, NULL, TST_CMD_PASS_RETVAL)) {
> +		if (errno == EBUSY)
> +			tst_brk(TCONF, "zram module may being used!");
> +	}
nit: I'd add space here for a readability (it's on more places).
> +	if (errno == ENOENT)
> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
> +
> +	modprobe = 1;
> +}

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/3] swapping01: skip test if zram-swap is being used

[xuyang2018.jy]

Hi Cyril, Petr
> Hi Xu, Cyril,
>
>> Hi!
>>> ---
>>>   testcases/kernel/mem/swapping/swapping01.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>
>>> diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
>>> index 392b79d65..29c1e0443 100644
>>> --- a/testcases/kernel/mem/swapping/swapping01.c
>>> +++ b/testcases/kernel/mem/swapping/swapping01.c
>>> @@ -67,6 +67,11 @@ static void test_swapping(void)
>>>   #ifdef TST_ABI32
>>>   	tst_brk(TCONF, "test is not designed for 32-bit system.");
>>>   #endif
>>> +	int ret;
>>> +
>>> +	ret = tst_system("zramctl | grep SWAP");
>>> +	if (!ret)
>>> +		tst_brk(TCONF, "zram-swap is being used!");
>
>> Actually what the zramctl does is to open the /proc/swaps file and looks
>> for the /dev/zram* in there. I guess that we can as well read the the
>> file ourselves...
>
> +1
> Because requiring zramctl disables this test for all systems without it.
Although I seen zramctl source code before, I did't realize that get 
info from /proc/swap. Will do it on v2.

Best Regards
Yang Xu
>
>
> Kind regards,
> Petr
>
>>>   	init_meminfo();
>
>>> @@ -155,4 +160,8 @@ static struct tst_test test = {
>>>   	.needs_root = 1,
>>>   	.forks_child = 1,
>>>   	.test_all = test_swapping,
>>> +	.needs_cmds = (const char *[]) {
>>> +		"zramctl",
>>> +		NULL
>>> +	}
>>>   };

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Cyril
> Hi!
>>   	tst_set_timeout $((dev_num*450))
>>
>> +	rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
>> +
>>   	tst_res TINFO "create '$dev_num' zram device(s)"
>>
>>   	modprobe zram num_devices=$dev_num || \
>> @@ -57,7 +59,8 @@ zram_load()
>>   	dev_num_created=$(ls /dev/zram* | wc -w)
>>
>>   	if [ "$dev_num_created" -ne "$dev_num" ]; then
>> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
>> +		tst_brk FAIL "num of devices expected $dev_num, but created"\
>                          ^
> 			This should have stayed TFAIL right?
Yes, sorry for this typo.

Best Regards
Yang Xu
>> +				"$dev_num_created"
>>   	fi
>>
>>   	tst_res TPASS "all zram devices successfully created"
>> --
>> 2.23.0
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Petr
> Hi Xu, Cyril,
>
>> Hi!
>>>   	tst_set_timeout $((dev_num*450))
>
>>> +	rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> This is wrong:
>
> $ lsmod | grep zram
> # rmmod zram; echo $?
> rmmod: ERROR: Module zram is not currently loaded
> 1
>
> Why not detect with lsmod? i.e.:
>
> 	if lsmod | grep zram; then
> 		rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> 	fi
>
Yes
> Although I'm not sure if we should attempt to rmmod, maybe quit when lsmod
> detects zram would be better.
If zram module was loaded but not used, then we can rmmod and do 
modprobe(case needs a clean zram envrionment). IMO, it doesn't affect 
any things.
If zram module was loaded but used, then rmmod failed and case skips.

ps: zram01 need to rmmod and modprobe zram. So if kernel built with 
CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
>
> Also it wouldn't harm to add at least rmmod into TST_NEEDS_CMDS (in the
> zram_lib.sh + take into account TST_NEEDS_CMDS from tests), but that's another
> story.
Agree.

Best Regards
Yang Xu
>
> Kind regards,
> Petr

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Petr Vorel]

Hi Xu,

> Hi Petr
> > Hi Xu, Cyril,

> >> Hi!
> >>>   	tst_set_timeout $((dev_num*450))

> >>> +	rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> > This is wrong:

> > $ lsmod | grep zram
> > # rmmod zram; echo $?
> > rmmod: ERROR: Module zram is not currently loaded
> > 1

> > Why not detect with lsmod? i.e.:

> > 	if lsmod | grep zram; then
> > 		rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> > 	fi

> Yes
> > Although I'm not sure if we should attempt to rmmod, maybe quit when lsmod
> > detects zram would be better.
> If zram module was loaded but not used, then we can rmmod and do 
> modprobe(case needs a clean zram envrionment). IMO, it doesn't affect 
> any things.
> If zram module was loaded but used, then rmmod failed and case skips.
OK, it's safe, ack.

> ps: zram01 need to rmmod and modprobe zram. So if kernel built with 
> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
+1

Kind regards,
Petr

> > Also it wouldn't harm to add at least rmmod into TST_NEEDS_CMDS (in the
> > zram_lib.sh + take into account TST_NEEDS_CMDS from tests), but that's another
> > story.
> Agree.

> Best Regards
> Yang Xu

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Petr
> Hi Xu,
>
>> Hi Petr
>>> Hi Xu, Cyril,
>
>>>> Hi!
>>>>>    	tst_set_timeout $((dev_num*450))
>
>>>>> +	rmmod zram>   /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
>>> This is wrong:
>
>>> $ lsmod | grep zram
>>> # rmmod zram; echo $?
>>> rmmod: ERROR: Module zram is not currently loaded
>>> 1
>
>>> Why not detect with lsmod? i.e.:
>
>>> 	if lsmod | grep zram; then
>>> 		rmmod zram>   /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
>>> 	fi
>
>> Yes
>>> Although I'm not sure if we should attempt to rmmod, maybe quit when lsmod
>>> detects zram would be better.
>> If zram module was loaded but not used, then we can rmmod and do
>> modprobe(case needs a clean zram envrionment). IMO, it doesn't affect
>> any things.
>> If zram module was loaded but used, then rmmod failed and case skips.
> OK, it's safe, ack.
>
>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
> +1
>
Since zram_lib.sh uses tst_needs_driver to search module in 
modules.dep/modules.builtin. Maybe we can introduce a new macro 
TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?

The difference between TST_NEEDS_MODULES and TST_NEEDS_DIRVERS is that 
the former needs module, but the latter needs module or builtin.

But I am not sure whether have many shell case needs this(Like network.)
Of course, kconfig shell api also can do this.

Ok, I am back to this case and write my v2 patch.

Best Regards
Yang Xu
> Kind regards,
> Petr
>
>>> Also it wouldn't harm to add at least rmmod into TST_NEEDS_CMDS (in the
>>> zram_lib.sh + take into account TST_NEEDS_CMDS from tests), but that's another
>>> story.
>> Agree.
>
>> Best Regards
>> Yang Xu

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

Re: [LTP] [PATCH 3/3] zram/zram03: Convert into new api

[xuyang2018.jy]

Hi Petr
> Hi Xu,
>
>> Also add removing zram module step in setup, so we can avoid the situation that
>> zram module is being used by zram-generator.
>
> ...
>> +++ b/testcases/kernel/device-drivers/zram/zram03.c
> ...
>> +static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
>
> Again, here should be lsmod used.
I prefer to detect it in /proc/modules.
What do you think about it?
>
> Except this LGTM.
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
>
>>   static void set_disksize(void)
>>   {
>> -	tst_resm(TINFO, "create a zram device with %ld bytes in size.", SIZE);
>> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/disksize", "%ld", SIZE);
>> +	tst_res(TINFO, "create a zram device with %ld bytes in size.", SIZE);
>> +	SAFE_FILE_PRINTF(PATH_ZRAM "/disksize", "%ld", SIZE);
>>   }
>
>>   static void write_device(void)
>> @@ -92,17 +45,16 @@ static void write_device(void)
>>   	int fd;
>>   	char *s;
>
>> -	tst_resm(TINFO, "map this zram device into memory.");
>> -	fd = SAFE_OPEN(cleanup, DEVICE, O_RDWR);
>> -	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ | PROT_WRITE,
>> -		      MAP_SHARED, fd, 0);
>> +	tst_res(TINFO, "map this zram device into memory.");
>> +	fd = SAFE_OPEN(DEVICE, O_RDWR);
>> +	s = SAFE_MMAP(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>
>> -	tst_resm(TINFO, "write all the memory.");
>> +	tst_res(TINFO, "write all the memory.");
> very nit: while at it, could you please remove unnecessary dot at the end?
OK.
>
> ...
>
>>   static void reset(void)
>>   {
>> -	tst_resm(TINFO, "reset it.");
>> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/reset", "1");
>> -}
> ...
>> +	tst_res(TINFO, "%s...", __func__);
>> +	SAFE_FILE_PRINTF(PATH_ZRAM "/reset", "1");
>>   }
>
> checkpatch.pl complains:
>
> $ make check-zram03
> zram03.c:93: WARNING: Unnecessary ftrace-like logging - prefer using ftrace
> =>  nit, but checkpatch.pl is correct, previous info was better than tracing
> function name. How about keep info instead of function name, e.g.:
> tst_res(TINFO, "reset zram");
of course.
>
> zram03.c:94: WARNING: Prefer using '"%s...", __func__' to using 'reset', this function's name, in a string
> false positive (not sure if we should bother with renaming function to keep it quiet).
I prefer to ignore it.
>
> ...
>> +static void setup(void)
>> +{
>> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
>> +
>> +	if (tst_cmd(cmd_rmmod, NULL, NULL, TST_CMD_PASS_RETVAL)) {
>> +		if (errno == EBUSY)
>> +			tst_brk(TCONF, "zram module may being used!");
>> +	}
> nit: I'd add space here for a readability (it's on more places).
OK.
>> +	if (errno == ENOENT)
>> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
>> +
>> +	modprobe = 1;
>> +}
>
> Kind regards,
> Petr

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

Re: [LTP] [PATCH 3/3] zram/zram03: Convert into new api

[xuyang2018.jy]

Hi Petr
> Hi Petr
>> Hi Xu,
>>
>>> Also add removing zram module step in setup, so we can avoid the situation that
>>> zram module is being used by zram-generator.
>>
>> ...
>>> +++ b/testcases/kernel/device-drivers/zram/zram03.c
>> ...
>>> +static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
>>
>> Again, here should be lsmod used.
> I prefer to detect it in /proc/modules.
> What do you think about it?
Since we have require modprobe/rmmod commands, using lsmod is better. I 
will use lsmod as you suggested.
.>>
>> Except this LGTM.
>> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>>
>>
>>>    static void set_disksize(void)
>>>    {
>>> -	tst_resm(TINFO, "create a zram device with %ld bytes in size.", SIZE);
>>> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/disksize", "%ld", SIZE);
>>> +	tst_res(TINFO, "create a zram device with %ld bytes in size.", SIZE);
>>> +	SAFE_FILE_PRINTF(PATH_ZRAM "/disksize", "%ld", SIZE);
>>>    }
>>
>>>    static void write_device(void)
>>> @@ -92,17 +45,16 @@ static void write_device(void)
>>>    	int fd;
>>>    	char *s;
>>
>>> -	tst_resm(TINFO, "map this zram device into memory.");
>>> -	fd = SAFE_OPEN(cleanup, DEVICE, O_RDWR);
>>> -	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ | PROT_WRITE,
>>> -		      MAP_SHARED, fd, 0);
>>> +	tst_res(TINFO, "map this zram device into memory.");
>>> +	fd = SAFE_OPEN(DEVICE, O_RDWR);
>>> +	s = SAFE_MMAP(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>>
>>> -	tst_resm(TINFO, "write all the memory.");
>>> +	tst_res(TINFO, "write all the memory.");
>> very nit: while at it, could you please remove unnecessary dot at the end?
> OK.
>>
>> ...
>>
>>>    static void reset(void)
>>>    {
>>> -	tst_resm(TINFO, "reset it.");
>>> -	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/reset", "1");
>>> -}
>> ...
>>> +	tst_res(TINFO, "%s...", __func__);
>>> +	SAFE_FILE_PRINTF(PATH_ZRAM "/reset", "1");
>>>    }
>>
>> checkpatch.pl complains:
>>
>> $ make check-zram03
>> zram03.c:93: WARNING: Unnecessary ftrace-like logging - prefer using ftrace
>> =>   nit, but checkpatch.pl is correct, previous info was better than tracing
>> function name. How about keep info instead of function name, e.g.:
>> tst_res(TINFO, "reset zram");
> of course.
>>
>> zram03.c:94: WARNING: Prefer using '"%s...", __func__' to using 'reset', this function's name, in a string
>> false positive (not sure if we should bother with renaming function to keep it quiet).
> I prefer to ignore it.
>>
>> ...
>>> +static void setup(void)
>>> +{
>>> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
>>> +
>>> +	if (tst_cmd(cmd_rmmod, NULL, NULL, TST_CMD_PASS_RETVAL)) {
>>> +		if (errno == EBUSY)
>>> +			tst_brk(TCONF, "zram module may being used!");
>>> +	}
>> nit: I'd add space here for a readability (it's on more places).
> OK.
>>> +	if (errno == ENOENT)
>>> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
>>> +
>>> +	modprobe = 1;
>>> +}
>>
>> Kind regards,
>> Petr
>

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

[LTP] [PATCH v2 1/3] swapping01: skip test if zram-swap is being used

[Yang Xu]

For the swapping test we attempt to allocate 130% of the available RAM and
we make sure that the overflow would fit the swap, but as long as swap is
backed by RAM this obviously false. So skip it if zram-swap is being used.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/mem/swapping/swapping01.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
index 392b79d65..aef47e9a4 100644
--- a/testcases/kernel/mem/swapping/swapping01.c
+++ b/testcases/kernel/mem/swapping/swapping01.c
@@ -40,10 +40,10 @@
 
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "tst_safe_stdio.h"
 #include "lapi/abisize.h"
 #include "mem.h"
 
@@ -67,6 +67,14 @@ static void test_swapping(void)
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
+	FILE *file;
+	char line[PATH_MAX];
+
+	file = SAFE_FOPEN("/proc/swaps", "r");
+	while (fgets(line, sizeof(line), file)) {
+		if (strstr(line, "/dev/zram"))
+			tst_brk(TCONF, "zram-swap is being used!");
+	}
 
 	init_meminfo();
 
-- 
2.23.0


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

[LTP] [PATCH v2 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Yang Xu]

If zram-generator package is installed and works, then we can not
remove zram module because zram swap is being used. We can also use zramstart
command or similar shell script(use zramctl,mkswap,swapon) to do this.

Since zram01.sh and zram02.sh needs to rmmod and modprobe, they can't work well
if zram module can't be removed. So skip it.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index fe9c915c3..4fed1f17c 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -5,6 +5,7 @@
 
 dev_makeswap=-1
 dev_mounted=-1
+dev_zram_load=-1
 
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
@@ -17,6 +18,10 @@ zram_cleanup()
 {
 	local i
 
+	if [ $dev_zram_load -le 0 ]; then
+		return
+	fi
+
 	for i in $(seq 0 $dev_makeswap); do
 		swapoff /dev/zram$i
 	done
@@ -47,6 +52,11 @@ zram_load()
 		tst_brk TBROK "dev_num must be > 0"
 	fi
 
+	lsmod | grep -q '^zram'
+	if [ $? -eq 0 ]; then
+		 rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
+	fi
+
 	tst_set_timeout $((dev_num*450))
 
 	tst_res TINFO "create '$dev_num' zram device(s)"
@@ -54,10 +64,12 @@ zram_load()
 	modprobe zram num_devices=$dev_num || \
 		tst_brk TBROK "failed to insert zram module"
 
+	dev_zram_load=1
 	dev_num_created=$(ls /dev/zram* | wc -w)
 
 	if [ "$dev_num_created" -ne "$dev_num" ]; then
-		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
+		tst_brk TFAIL "expected num of devices $dev_num, but created"\
+				"$dev_num_created"
 	fi
 
 	tst_res TPASS "all zram devices successfully created"
-- 
2.23.0


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

[LTP] [PATCH v2 3/3] zram/zram03: Convert into new api

[Yang Xu]

Also add removing zram module step in setup, so we can avoid the situation that
zram module is being used by zram-generator.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
Not use lsmod to detect because tst_cmd doesn't support "lsmod |grep zram". Or
I miss something?
 testcases/kernel/device-drivers/zram/zram03.c | 233 ++++++++----------
 1 file changed, 102 insertions(+), 131 deletions(-)

diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
index 06995fd56..4f8fb994f 100644
--- a/testcases/kernel/device-drivers/zram/zram03.c
+++ b/testcases/kernel/device-drivers/zram/zram03.c
@@ -1,43 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * zram: generic RAM based compressed R/W block devices
- * http://lkml.org/lkml/2010/8/9/227
- *
  * Copyright (C) 2010  Red Hat, Inc.
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+/*\
+ * [Description]
  *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
+ * zram: generic RAM based compressed R/W block devices
+ * http://lkml.org/lkml/2010/8/9/227
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
+ * This case check whether data read from zram device is consistent with
+ * thoese are written.
  */
 
+
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <stdio.h>
 #include <string.h>
 #include <unistd.h>
-
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "zram03";
-int TST_TOTAL = 1;
+#include "tst_safe_stdio.h"
+#include "tst_test.h"
 
 #define PATH_ZRAM	"/sys/block/zram0"
 #define OBSOLETE_ZRAM_FILE	"/sys/block/zram0/num_reads"
@@ -47,44 +32,12 @@ int TST_TOTAL = 1;
 #define DEVICE		"/dev/zram0"
 
 static int modprobe;
-
-static void set_disksize(void);
-static void write_device(void);
-static void verify_device(void);
-static void reset(void);
-static void setup(void);
-static void cleanup(void);
-static void print(char *string);
-static void dump_info(void);
-
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		set_disksize();
-
-		write_device();
-		dump_info();
-		verify_device();
-
-		reset();
-		dump_info();
-	}
-	cleanup();
-	tst_exit();
-}
+static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
 
 static void set_disksize(void)
 {
-	tst_resm(TINFO, "create a zram device with %ld bytes in size.", SIZE);
-	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/disksize", "%ld", SIZE);
+	tst_res(TINFO, "create a zram device with %ld bytes in size", SIZE);
+	SAFE_FILE_PRINTF(PATH_ZRAM "/disksize", "%ld", SIZE);
 }
 
 static void write_device(void)
@@ -92,17 +45,16 @@ static void write_device(void)
 	int fd;
 	char *s;
 
-	tst_resm(TINFO, "map this zram device into memory.");
-	fd = SAFE_OPEN(cleanup, DEVICE, O_RDWR);
-	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ | PROT_WRITE,
-		      MAP_SHARED, fd, 0);
+	tst_res(TINFO, "map this zram device into memory");
+	fd = SAFE_OPEN(DEVICE, O_RDWR);
+	s = SAFE_MMAP(NULL, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 
-	tst_resm(TINFO, "write all the memory.");
+	tst_res(TINFO, "write all the memory");
 	memset(s, 'a', SIZE - 1);
 	s[SIZE - 1] = '\0';
 
-	SAFE_MUNMAP(cleanup, s, SIZE);
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_MUNMAP(s, SIZE);
+	SAFE_CLOSE(fd);
 }
 
 static void verify_device(void)
@@ -111,9 +63,9 @@ static void verify_device(void)
 	long i = 0, fail = 0;
 	char *s;
 
-	tst_resm(TINFO, "verify contents from device.");
-	fd = SAFE_OPEN(cleanup, DEVICE, O_RDONLY);
-	s = SAFE_MMAP(cleanup, NULL, SIZE, PROT_READ, MAP_PRIVATE, fd, 0);
+	tst_res(TINFO, "verify contents from device");
+	fd = SAFE_OPEN(DEVICE, O_RDONLY);
+	s = SAFE_MMAP(NULL, SIZE, PROT_READ, MAP_PRIVATE, fd, 0);
 
 	while (s[i] && i < SIZE - 1) {
 		if (s[i] != 'a')
@@ -121,59 +73,25 @@ static void verify_device(void)
 		i++;
 	}
 	if (i != SIZE - 1) {
-		tst_resm(TFAIL, "expect size: %ld, actual size: %ld.",
+		tst_res(TFAIL, "expect size: %ld, actual size: %ld.",
 			 SIZE - 1, i);
 	} else if (s[i] != '\0') {
-		tst_resm(TFAIL, "zram device seems not null terminated");
+		tst_res(TFAIL, "zram device seems not null terminated");
 	} else if (fail) {
-		tst_resm(TFAIL, "%ld failed bytes found.", fail);
+		tst_res(TFAIL, "%ld failed bytes found", fail);
 	} else {
-		tst_resm(TPASS, "data read from zram device is consistent "
+		tst_res(TPASS, "data read from zram device is consistent "
 			 "with those are written");
 	}
 
-	SAFE_MUNMAP(cleanup, s, SIZE);
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_MUNMAP(s, SIZE);
+	SAFE_CLOSE(fd);
 }
 
 static void reset(void)
 {
-	tst_resm(TINFO, "reset it.");
-	SAFE_FILE_PRINTF(cleanup, PATH_ZRAM "/reset", "1");
-}
-
-static void setup(void)
-{
-	int retried = 0;
-
-	tst_require_root();
-
-retry:
-	if (access(PATH_ZRAM, F_OK) == -1) {
-		if (errno == ENOENT) {
-			if (retried) {
-				tst_brkm(TCONF, NULL,
-					 "system has no zram device.");
-			}
-			if (system("modprobe zram") == -1) {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					 "system(modprobe zram) failed");
-			}
-			modprobe = 1;
-			retried = 1;
-			goto retry;
-		} else
-			tst_brkm(TBROK | TERRNO, NULL, "access");
-	}
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
-}
-
-static void cleanup(void)
-{
-	if (modprobe == 1 && system("rmmod zram") == -1)
-		tst_resm(TWARN | TERRNO, "system(rmmod zram) failed");
+	tst_res(TINFO, "Reset zram");
+	SAFE_FILE_PRINTF(PATH_ZRAM "/reset", "1");
 }
 
 static void print(char *string)
@@ -181,20 +99,17 @@ static void print(char *string)
 	char filename[BUFSIZ], value[BUFSIZ];
 
 	sprintf(filename, "%s/%s", PATH_ZRAM, string);
-	SAFE_FILE_SCANF(cleanup, filename, "%s", value);
-	tst_resm(TINFO, "%s is %s", filename, value);
+	SAFE_FILE_SCANF(filename, "%s", value);
+	tst_res(TINFO, "%s is %s", filename, value);
 }
 
 static void print_stat(char *nread, char *nwrite)
 {
 	char nread_val[BUFSIZ], nwrite_val[BUFSIZ];
 
-	SAFE_FILE_SCANF(cleanup, PATH_ZRAM_STAT, "%s %*s %*s %*s %s",
-			nread_val, nwrite_val);
-	tst_resm(TINFO, "%s from %s is %s", nread, PATH_ZRAM_STAT,
-		 nread_val);
-	tst_resm(TINFO, "%s from %s is %s", nwrite, PATH_ZRAM_STAT,
-		 nwrite_val);
+	SAFE_FILE_SCANF(PATH_ZRAM_STAT, "%s %*s %*s %*s %s", nread_val, nwrite_val);
+	tst_res(TINFO, "%s from %s is %s", nread, PATH_ZRAM_STAT, nread_val);
+	tst_res(TINFO, "%s from %s is %s", nwrite, PATH_ZRAM_STAT, nwrite_val);
 }
 
 static void print_mm_stat(char *orig, char *compr, char *mem, char *zero)
@@ -202,16 +117,12 @@ static void print_mm_stat(char *orig, char *compr, char *mem, char *zero)
 	char orig_val[BUFSIZ], compr_val[BUFSIZ];
 	char mem_val[BUFSIZ], zero_val[BUFSIZ];
 
-	SAFE_FILE_SCANF(cleanup, PATH_ZRAM_MM_STAT, "%s %s %s %*s %*s %s",
+	SAFE_FILE_SCANF(PATH_ZRAM_MM_STAT, "%s %s %s %*s %*s %s",
 			orig_val, compr_val, mem_val, zero_val);
-	tst_resm(TINFO, "%s from %s is %s", orig, PATH_ZRAM_MM_STAT,
-		 orig_val);
-	tst_resm(TINFO, "%s from %s is %s", compr, PATH_ZRAM_MM_STAT,
-		compr_val);
-	tst_resm(TINFO, "%s from %s is %s", mem, PATH_ZRAM_MM_STAT,
-		 mem_val);
-	tst_resm(TINFO, "%s from %s is %s", zero, PATH_ZRAM_MM_STAT,
-		 zero_val);
+	tst_res(TINFO, "%s from %s is %s", orig, PATH_ZRAM_MM_STAT, orig_val);
+	tst_res(TINFO, "%s from %s is %s", compr, PATH_ZRAM_MM_STAT, compr_val);
+	tst_res(TINFO, "%s from %s is %s", mem, PATH_ZRAM_MM_STAT, mem_val);
+	tst_res(TINFO, "%s from %s is %s", zero, PATH_ZRAM_MM_STAT, zero_val);
 }
 
 static void dump_info(void)
@@ -231,3 +142,63 @@ static void dump_info(void)
 		print_stat("num_reads", "num_writes");
 	}
 }
+
+static void run(void)
+{
+	set_disksize();
+
+	write_device();
+	dump_info();
+	verify_device();
+
+	reset();
+	dump_info();
+}
+
+static void setup(void)
+{
+	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
+	FILE *file;
+	char line[PATH_MAX];
+
+	file = SAFE_FOPEN("/proc/modules", "r");
+	while (fgets(line, sizeof(line), file)) {
+		if (strstr(line, "zram")) {
+			modprobe = 1;
+			break;
+		}
+	}
+
+	if (modprobe) {
+		tst_res(TINFO, "rmmod zram before test");
+		if (tst_cmd(cmd_rmmod, NULL, NULL, TST_CMD_PASS_RETVAL))
+			tst_brk(TCONF, "zram module may being used!");
+		else
+			modprobe = 0;
+	}
+
+	SAFE_CMD(cmd_modprobe, NULL, NULL);
+	modprobe = 1;
+}
+
+static void cleanup(void)
+{
+	if (modprobe == 1)
+		SAFE_CMD(cmd_rmmod, NULL, NULL);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_drivers = (const char *const []) {
+		"zram",
+		NULL
+	},
+	.needs_cmds = (const char *[]) {
+		"modprobe",
+		"rmmod",
+		NULL
+	}
+};
-- 
2.23.0


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

Re: [LTP] [PATCH v2 1/3] swapping01: skip test if zram-swap is being used

[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 1/3] swapping01: skip test if zram-swap is being used

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
> index 392b79d65..aef47e9a4 100644
> --- a/testcases/kernel/mem/swapping/swapping01.c
> +++ b/testcases/kernel/mem/swapping/swapping01.c
> @@ -40,10 +40,10 @@
>  
>  #include <sys/types.h>
>  #include <sys/wait.h>
> -#include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include "tst_safe_stdio.h"
>  #include "lapi/abisize.h"
>  #include "mem.h"
>  
> @@ -67,6 +67,14 @@ static void test_swapping(void)
>  #ifdef TST_ABI32
>  	tst_brk(TCONF, "test is not designed for 32-bit system.");
>  #endif
> +	FILE *file;
> +	char line[PATH_MAX];
> +
> +	file = SAFE_FOPEN("/proc/swaps", "r");
> +	while (fgets(line, sizeof(line), file)) {
> +		if (strstr(line, "/dev/zram"))
> +			tst_brk(TCONF, "zram-swap is being used!");
> +	}

Actually please add SAFE_FCLOSE(file) here, but otherwise Reviewed...

>  	init_meminfo();
>  
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Cyril Hrubis]

Hi!
> +	lsmod | grep -q '^zram'
> +	if [ $? -eq 0 ]; then

Why not just:
	if grep -q '^zram' /proc/modules; then
		...

Here as well what the lsmod does it to open /proc/modules and for each
entry it also looks at /sys/module/$name/refcnt and
/sys/module/$name/hodlers. But we are not interested in the second part
at all...

> +		 rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> +	fi
> +
>  	tst_set_timeout $((dev_num*450))
>  
>  	tst_res TINFO "create '$dev_num' zram device(s)"
> @@ -54,10 +64,12 @@ zram_load()
>  	modprobe zram num_devices=$dev_num || \
>  		tst_brk TBROK "failed to insert zram module"
>  
> +	dev_zram_load=1
>  	dev_num_created=$(ls /dev/zram* | wc -w)
>  
>  	if [ "$dev_num_created" -ne "$dev_num" ]; then
> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
> +		tst_brk TFAIL "expected num of devices $dev_num, but created"\
> +				"$dev_num_created"
>  	fi
>  
>  	tst_res TPASS "all zram devices successfully created"
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/3] zram/zram03: Convert into new api

[Cyril Hrubis]

Hi!
> Also add removing zram module step in setup, so we can avoid the situation that
> zram module is being used by zram-generator.
> 
> Fixes: #888
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
> Not use lsmod to detect because tst_cmd doesn't support "lsmod |grep zram". Or
> I miss something?

We can do the same here as we do for the swap detection here.

Just SAFE_FOPEN("/proc/modules") and loop over the lines and do strstr() for zram.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v3 1/3] swapping01: skip test if zram-swap is being used

[Yang Xu]

For the swapping test we attempt to allocate 130% of the available RAM and
we make sure that the overflow would fit the swap, but as long as swap is
backed by RAM this obviously false. So skip it if zram-swap is being used.

Fixes: #888
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/mem/swapping/swapping01.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/swapping/swapping01.c b/testcases/kernel/mem/swapping/swapping01.c
index 392b79d65..925790186 100644
--- a/testcases/kernel/mem/swapping/swapping01.c
+++ b/testcases/kernel/mem/swapping/swapping01.c
@@ -40,10 +40,10 @@
 
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "tst_safe_stdio.h"
 #include "lapi/abisize.h"
 #include "mem.h"
 
@@ -67,6 +67,17 @@ static void test_swapping(void)
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
+	FILE *file;
+	char line[PATH_MAX];
+
+	file = SAFE_FOPEN("/proc/swaps", "r");
+	while (fgets(line, sizeof(line), file)) {
+		if (strstr(line, "/dev/zram")) {
+			SAFE_FCLOSE(file);
+			tst_brk(TCONF, "zram-swap is being used!");
+		}
+	}
+	SAFE_FCLOSE(file);
 
 	init_meminfo();
 
-- 
2.23.0


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

[LTP] [PATCH v3 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Yang Xu]

If zram-generator package is installed and works, then we can not
remove zram module because zram swap is being used. We can also use zramstart
command or similar shell script(use zramctl,mkswap,swapon) to do this.

Since zram01.sh and zram02.sh needs to rmmod and modprobe, they can't work well
if zram module can't be removed. So skip it.

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index fe9c915c3..26f19ec02 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -5,6 +5,7 @@
 
 dev_makeswap=-1
 dev_mounted=-1
+dev_zram_load=-1
 
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
@@ -17,6 +18,10 @@ zram_cleanup()
 {
 	local i
 
+	if [ $dev_zram_load -le 0 ]; then
+		return
+	fi
+
 	for i in $(seq 0 $dev_makeswap); do
 		swapoff /dev/zram$i
 	done
@@ -47,6 +52,10 @@ zram_load()
 		tst_brk TBROK "dev_num must be > 0"
 	fi
 
+	if grep -q '^zram' /proc/modules; then
+		 rmmod zram > /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
+	fi
+
 	tst_set_timeout $((dev_num*450))
 
 	tst_res TINFO "create '$dev_num' zram device(s)"
@@ -54,10 +63,12 @@ zram_load()
 	modprobe zram num_devices=$dev_num || \
 		tst_brk TBROK "failed to insert zram module"
 
+	dev_zram_load=1
 	dev_num_created=$(ls /dev/zram* | wc -w)
 
 	if [ "$dev_num_created" -ne "$dev_num" ]; then
-		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
+		tst_brk TFAIL "expected num of devices $dev_num, but created"\
+				"$dev_num_created"
 	fi
 
 	tst_res TPASS "all zram devices successfully created"
-- 
2.23.0


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

Re: [LTP] [PATCH 3/3] zram/zram03: Convert into new api

[Petr Vorel]

Hi all,
> Hi Petr
> > Hi Petr
> >> Hi Xu,

> >>> Also add removing zram module step in setup, so we can avoid the situation that
> >>> zram module is being used by zram-generator.

> >> ...
> >>> +++ b/testcases/kernel/device-drivers/zram/zram03.c
> >> ...
> >>> +static const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};

> >> Again, here should be lsmod used.
> > I prefer to detect it in /proc/modules.
Generally I'd vote for approaches like this to not depend on anything external
to cover more systems (Cyril suggested in v2 SAFE_FOPEN("/proc/modules") and
loop over the lines and do strstr() for zram.)

> > What do you think about it?
> Since we have require modprobe/rmmod commands, using lsmod is better. I 
> will use lsmod as you suggested.
OK. I was thinking a bit whether using finit_module would help to replace
modprobe and not require rmmod to cover more systems, but it'd be probably
overcomplicated just for very few systems without kmod installed.

(+ We already depend on all but AOSP on /lib/modules/$(uname
-r)/modules.{dep,builtin} for tst_check_driver().)

Thus ack keeping the current approach.

Kind regards,
Petr

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Petr Vorel]

Hi Xu,

...
> >> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
> >> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
> > +1

We already have v3, but I got confused here in v1, thus asking.

> Since zram_lib.sh uses tst_needs_driver to search module in 
> modules.dep/modules.builtin. Maybe we can introduce a new macro 
> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?

> The difference between TST_NEEDS_MODULES and TST_NEEDS_DIRVERS is that 
> the former needs module, but the latter needs module or builtin.
Yes, and TST_NEEDS_DRIVERS is already used in zram_lib.sh.

Kind regards,
Petr

> But I am not sure whether have many shell case needs this(Like network.)
> Of course, kconfig shell api also can do this.

> Ok, I am back to this case and write my v2 patch.

> Best Regards
> Yang Xu
> > Kind regards,
> > Petr

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Petr
> Hi Xu,
>
> ...
>>>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
>>>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
>>> +1
>
> We already have v3, but I got confused here in v1, thus asking.
>
>> Since zram_lib.sh uses tst_needs_driver to search module in
>> modules.dep/modules.builtin. Maybe we can introduce a new macro
>> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
> I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?
Sorry, I don't know ltp shell api already have this TST_NEEDS_MODULE 
macro and so suggest to add a new macro wrongly.

Yes, I want to use TST_NEEDS_MODULE instead of TST_NEEDS_DIRVERS because 
the former is more accurate.

But it seems  our TST_NEEDS_MODULE only can check custom kernel module 
but can't check system kernel module(like ext4.ko.xz, xfs.ext4.ko.xz).

ps: kconfig shell api can also do the thing by using "config_zram=m". So 
I may use kconfig shell api in here in the feature.

Best Regards
Yang Xu
>
>> The difference between TST_NEEDS_MODULES and TST_NEEDS_DIRVERS is that
>> the former needs module, but the latter needs module or builtin.
> Yes, and TST_NEEDS_DRIVERS is already used in zram_lib.sh.
>
> Kind regards,
> Petr
>
>> But I am not sure whether have many shell case needs this(Like network.)
>> Of course, kconfig shell api also can do this.
>
>> Ok, I am back to this case and write my v2 patch.
>
>> Best Regards
>> Yang Xu
>>> Kind regards,
>>> Petr

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Petr Vorel]

Hi Xu,

[ Cc Naresh Kamboju who added zram support to kconfig ]

> Hi Petr
> > Hi Xu,

> > ...
> >>>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
> >>>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
> >>> +1

> > We already have v3, but I got confused here in v1, thus asking.

> >> Since zram_lib.sh uses tst_needs_driver to search module in
> >> modules.dep/modules.builtin. Maybe we can introduce a new macro
> >> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
> > I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?
> Sorry, I don't know ltp shell api already have this TST_NEEDS_MODULE 
> macro and so suggest to add a new macro wrongly.

> Yes, I want to use TST_NEEDS_MODULE instead of TST_NEEDS_DIRVERS because 
> the former is more accurate.

> But it seems  our TST_NEEDS_MODULE only can check custom kernel module 
> but can't check system kernel module(like ext4.ko.xz, xfs.ext4.ko.xz).
Good catch, tst_require_module() in tst_test.sh should be fixed.
Also, if I look correctly, there is no TST_NEEDS_MODULE equivalent for C API
(probably not needed).

> ps: kconfig shell api can also do the thing by using "config_zram=m". So 
> I may use kconfig shell api in here in the feature.
FYI When Naresh Kamboju put LTP zram tests into kselftest back in 2015,
he modified it to support also zram built-in (CONFIG_ZRAM=y).

We don't support it, thus CONFIG_ZRAM=m is OK now, but we should consider to add
at least some minimal support for CONFIG_ZRAM=y.

Kind regards,
Petr

> Best Regards
> Yang Xu

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Petr
> Hi Xu,
>
> [ Cc Naresh Kamboju who added zram support to kconfig ]
Oh, I don't know kernel selftest also have these two zram tests.
>
>> Hi Petr
>>> Hi Xu,
>
>>> ...
>>>>>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
>>>>>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
>>>>> +1
>
>>> We already have v3, but I got confused here in v1, thus asking.
>
>>>> Since zram_lib.sh uses tst_needs_driver to search module in
>>>> modules.dep/modules.builtin. Maybe we can introduce a new macro
>>>> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
>>> I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?
>> Sorry, I don't know ltp shell api already have this TST_NEEDS_MODULE
>> macro and so suggest to add a new macro wrongly.
>
>> Yes, I want to use TST_NEEDS_MODULE instead of TST_NEEDS_DIRVERS because
>> the former is more accurate.
>
>> But it seems  our TST_NEEDS_MODULE only can check custom kernel module
>> but can't check system kernel module(like ext4.ko.xz, xfs.ext4.ko.xz).
> Good catch, tst_require_module() in tst_test.sh should be fixed.
> Also, if I look correctly, there is no TST_NEEDS_MODULE equivalent for C API
> (probably not needed).
Maybe add a issue for this, so we or others can do it in the future.
>
>> ps: kconfig shell api can also do the thing by using "config_zram=m". So
>> I may use kconfig shell api in here in the feature.
> FYI When Naresh Kamboju put LTP zram tests into kselftest back in 2015,
> he modified it to support also zram built-in (CONFIG_ZRAM=y).
>
> We don't support it, thus CONFIG_ZRAM=m is OK now, but we should consider to add
> at least some minimal support for CONFIG_ZRAM=y.
Yes, kernel selftest supports CONFIG_ZRAM=y. Because zram01.sh in kernel 
selftest only used one zram device(It only supports ext4 filesystem but 
ltp supports all possible filesystem).

Since most distribution should use CONFIG_ZRAM=m, I guess we don't need 
to support CONFIG_ZRAM=y and ltp can support more filesystems.
Maybe can add a note that user can use kernel selftest when his kernel 
config is CONFIG_ZRAM=y.

Also, It seems kernel selftest zram case is old
1) should remove zram_max_streams
2) should use mm_stat instead of free -m to calcaute ratio
3) should consider the situation that /dev/zram0 is being used

Best Regards
Yang Xu
>
> Kind regards,
> Petr
>
>> Best Regards
>> Yang Xu

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi Petr
> Hi Petr
>> Hi Xu,
>>
>> [ Cc Naresh Kamboju who added zram support to kconfig ]
> Oh, I don't know kernel selftest also have these two zram tests.
>>
>>> Hi Petr
>>>> Hi Xu,
>>
>>>> ...
>>>>>>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
>>>>>>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
>>>>>> +1
>>
>>>> We already have v3, but I got confused here in v1, thus asking.
>>
>>>>> Since zram_lib.sh uses tst_needs_driver to search module in
>>>>> modules.dep/modules.builtin. Maybe we can introduce a new macro
>>>>> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
>>>> I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?
>>> Sorry, I don't know ltp shell api already have this TST_NEEDS_MODULE
>>> macro and so suggest to add a new macro wrongly.
>>
>>> Yes, I want to use TST_NEEDS_MODULE instead of TST_NEEDS_DIRVERS because
>>> the former is more accurate.
>>
>>> But it seems  our TST_NEEDS_MODULE only can check custom kernel module
>>> but can't check system kernel module(like ext4.ko.xz, xfs.ext4.ko.xz).
>> Good catch, tst_require_module() in tst_test.sh should be fixed.
>> Also, if I look correctly, there is no TST_NEEDS_MODULE equivalent for C API
>> (probably not needed).
> Maybe add a issue for this, so we or others can do it in the future.
>>
>>> ps: kconfig shell api can also do the thing by using "config_zram=m". So
>>> I may use kconfig shell api in here in the feature.
>> FYI When Naresh Kamboju put LTP zram tests into kselftest back in 2015,
>> he modified it to support also zram built-in (CONFIG_ZRAM=y).
>>
>> We don't support it, thus CONFIG_ZRAM=m is OK now, but we should consider to add
>> at least some minimal support for CONFIG_ZRAM=y.
Maybe add sysfs interface, we can use hot_add in setup and hot_remove in 
cleanup even zram device is being used or  zram module is builtin.

Best Regards
Yang Xu
> Yes, kernel selftest supports CONFIG_ZRAM=y. Because zram01.sh in kernel
> selftest only used one zram device(It only supports ext4 filesystem but
> ltp supports all possible filesystem).
>
> Since most distribution should use CONFIG_ZRAM=m, I guess we don't need
> to support CONFIG_ZRAM=y and ltp can support more filesystems.
> Maybe can add a note that user can use kernel selftest when his kernel
> config is CONFIG_ZRAM=y.
>
> Also, It seems kernel selftest zram case is old
> 1) should remove zram_max_streams
> 2) should use mm_stat instead of free -m to calcaute ratio
> 3) should consider the situation that /dev/zram0 is being used
>
> Best Regards
> Yang Xu
>>
>> Kind regards,
>> Petr
>>
>>> Best Regards
>>> Yang Xu
>

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

Re: [LTP] [PATCH v3 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[xuyang2018.jy]

Hi All

I will send a v4 patch today by using hot_add/hot_remove interface
instead of skipping case.

Best Regards
Yang Xu
> If zram-generator package is installed and works, then we can not
> remove zram module because zram swap is being used. We can also use zramstart
> command or similar shell script(use zramctl,mkswap,swapon) to do this.
> 
> Since zram01.sh and zram02.sh needs to rmmod and modprobe, they can't work well
> if zram module can't be removed. So skip it.
> 
> Fixes: #888
> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> ---
>   testcases/kernel/device-drivers/zram/zram_lib.sh | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
> index fe9c915c3..26f19ec02 100755
> --- a/testcases/kernel/device-drivers/zram/zram_lib.sh
> +++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
> @@ -5,6 +5,7 @@
> 
>   dev_makeswap=-1
>   dev_mounted=-1
> +dev_zram_load=-1
> 
>   TST_NEEDS_TMPDIR=1
>   TST_NEEDS_ROOT=1
> @@ -17,6 +18,10 @@ zram_cleanup()
>   {
>   	local i
> 
> +	if [ $dev_zram_load -le 0 ]; then
> +		return
> +	fi
> +
>   	for i in $(seq 0 $dev_makeswap); do
>   		swapoff /dev/zram$i
>   	done
> @@ -47,6 +52,10 @@ zram_load()
>   		tst_brk TBROK "dev_num must be>  0"
>   	fi
> 
> +	if grep -q '^zram' /proc/modules; then
> +		 rmmod zram>  /dev/null 2>&1 || tst_brk TCONF "zram module is being used"
> +	fi
> +
>   	tst_set_timeout $((dev_num*450))
> 
>   	tst_res TINFO "create '$dev_num' zram device(s)"
> @@ -54,10 +63,12 @@ zram_load()
>   	modprobe zram num_devices=$dev_num || \
>   		tst_brk TBROK "failed to insert zram module"
> 
> +	dev_zram_load=1
>   	dev_num_created=$(ls /dev/zram* | wc -w)
> 
>   	if [ "$dev_num_created" -ne "$dev_num" ]; then
> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
> +		tst_brk TFAIL "expected num of devices $dev_num, but created"\
> +				"$dev_num_created"
>   	fi
> 
>   	tst_res TPASS "all zram devices successfully created"

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

Re: [LTP] [PATCH 2/3] zram/zram_lib.sh: Skip test if zram module can not be removed

[Petr Vorel]

Hi Xu,

> Hi Petr
> > Hi Xu,

> > [ Cc Naresh Kamboju who added zram support to kconfig ]
> Oh, I don't know kernel selftest also have these two zram tests.

> >> Hi Petr
> >>> Hi Xu,

> >>> ...
> >>>>>> ps: zram01 need to rmmod and modprobe zram. So if kernel built with
> >>>>>> CONFIG_ZRAM=y, we also need to TCONF. But it is another story.
> >>>>> +1

> >>> We already have v3, but I got confused here in v1, thus asking.

> >>>> Since zram_lib.sh uses tst_needs_driver to search module in
> >>>> modules.dep/modules.builtin. Maybe we can introduce a new macro
> >>>> TST_NEEDS_MODULES (like TST_NEEDS_DIRVERS)?
> >>> I got lost here, did you suggest zram_lib.sh to switch to use $TST_NEEDS_MODULE?
> >> Sorry, I don't know ltp shell api already have this TST_NEEDS_MODULE
> >> macro and so suggest to add a new macro wrongly.

> >> Yes, I want to use TST_NEEDS_MODULE instead of TST_NEEDS_DIRVERS because
> >> the former is more accurate.

> >> But it seems  our TST_NEEDS_MODULE only can check custom kernel module
> >> but can't check system kernel module(like ext4.ko.xz, xfs.ext4.ko.xz).
> > Good catch, tst_require_module() in tst_test.sh should be fixed.
> > Also, if I look correctly, there is no TST_NEEDS_MODULE equivalent for C API
> > (probably not needed).
> Maybe add a issue for this, so we or others can do it in the future.
And is it needed to be added to C API?

> >> ps: kconfig shell api can also do the thing by using "config_zram=m". So
> >> I may use kconfig shell api in here in the feature.
> > FYI When Naresh Kamboju put LTP zram tests into kselftest back in 2015,
> > he modified it to support also zram built-in (CONFIG_ZRAM=y).

> > We don't support it, thus CONFIG_ZRAM=m is OK now, but we should consider to add
> > at least some minimal support for CONFIG_ZRAM=y.
> Yes, kernel selftest supports CONFIG_ZRAM=y. Because zram01.sh in kernel 
> selftest only used one zram device(It only supports ext4 filesystem but 
> ltp supports all possible filesystem).

> Since most distribution should use CONFIG_ZRAM=m, I guess we don't need 
> to support CONFIG_ZRAM=y and ltp can support more filesystems.
> Maybe can add a note that user can use kernel selftest when his kernel 
> config is CONFIG_ZRAM=y.
Not sure.

> Also, It seems kernel selftest zram case is old
> 1) should remove zram_max_streams
> 2) should use mm_stat instead of free -m to calcaute ratio
> 3) should consider the situation that /dev/zram0 is being used
Yes it's old. As some people run only kselftest it might be worth to update it.

Kind regards,
Petr

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