All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used
@ 2021-12-15  7:20 Yang Xu
  2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Yang Xu @ 2021-12-15  7:20 UTC (permalink / raw)
  To: ltp

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

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

* [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-15  7:20 [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Yang Xu
@ 2021-12-15  7:20 ` Yang Xu
  2021-12-17  7:48   ` Petr Vorel
  2021-12-17  7:49   ` Petr Vorel
  2021-12-15  7:20 ` [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api Yang Xu
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 18+ messages in thread
From: Yang Xu @ 2021-12-15  7:20 UTC (permalink / raw)
  To: ltp

If zram-generator package is installed and works, then we can not remove zram module
because zram swap is being used. This case needs a clean zram environment, change this
test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
still can add zram device and remove them in cleanup.

Also, zram01,02 case are adjuested to adapt the situation that CONFIG_ZRAM=y.

1]https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices

Fixes: #888
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 .../kernel/device-drivers/zram/zram01.sh      |  6 +-
 .../kernel/device-drivers/zram/zram02.sh      |  4 +-
 .../kernel/device-drivers/zram/zram_lib.sh    | 60 ++++++++++++-------
 3 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index ad9a9a2be..5e13f387c 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -69,7 +69,7 @@ setup()
 
 zram_makefs()
 {
-	local i=0
+	local i=$dev_start
 	local fs
 
 	for fs in $zram_filesystems; do
@@ -90,7 +90,7 @@ zram_mount()
 {
 	local i=0
 
-	for i in $(seq 0 $(($dev_num - 1))); do
+	for i in $(seq $dev_start $dev_end); do
 		tst_res TINFO "mount /dev/zram$i"
 		mkdir zram$i
 		ROD mount /dev/zram$i zram$i
@@ -102,7 +102,7 @@ zram_mount()
 
 zram_fill_fs()
 {
-	for i in $(seq 0 $(($dev_num - 1))); do
+	for i in $(seq $dev_start $dev_end); do
 		tst_res TINFO "filling zram$i (it can take long time)"
 		local b=0
 		while true; do
diff --git a/testcases/kernel/device-drivers/zram/zram02.sh b/testcases/kernel/device-drivers/zram/zram02.sh
index f0421ce7f..c980fce76 100755
--- a/testcases/kernel/device-drivers/zram/zram02.sh
+++ b/testcases/kernel/device-drivers/zram/zram02.sh
@@ -29,7 +29,7 @@ zram_makeswap()
 	tst_require_cmds mkswap swapon swapoff
 	local i=0
 
-	for i in $(seq 0 $(($dev_num - 1))); do
+	for i in $(seq $dev_start $dev_end); do
 		ROD mkswap /dev/zram$i
 		ROD swapon /dev/zram$i
 		tst_res TINFO "done with /dev/zram$i"
@@ -44,7 +44,7 @@ zram_swapoff()
 	tst_require_cmds swapoff
 	local i
 
-	for i in $(seq 0 $dev_makeswap); do
+	for i in $(seq $dev_start $dev_end); do
 		ROD swapoff /dev/zram$i
 	done
 	dev_makeswap=-1
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index fe9c915c3..db9552501 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -5,6 +5,9 @@
 
 dev_makeswap=-1
 dev_mounted=-1
+dev_start=-1
+dev_end=-1
+module_load=-1
 
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
@@ -17,19 +20,25 @@ zram_cleanup()
 {
 	local i
 
-	for i in $(seq 0 $dev_makeswap); do
+	for i in $(seq $dev_start $dev_makeswap); do
 		swapoff /dev/zram$i
 	done
 
-	for i in $(seq 0 $dev_mounted); do
+	for i in $(seq $dev_start $dev_mounted); do
 		umount /dev/zram$i
 	done
 
-	for i in $(seq 0 $(($dev_num - 1))); do
+	for i in $(seq $dev_start $dev_end); do
 		echo 1 > /sys/block/zram${i}/reset
 	done
 
-	rmmod zram > /dev/null 2>&1
+	for i in $(seq $dev_start $dev_end); do
+		echo $i > /sys/class/zram-control/hot_remove
+	done
+
+	if [ $module_load -eq 1 ]; then
+		rmmod zram > /dev/null 2>&1
+	fi
 }
 
 zram_load()
@@ -51,16 +60,23 @@ zram_load()
 
 	tst_res TINFO "create '$dev_num' zram device(s)"
 
-	modprobe zram num_devices=$dev_num || \
-		tst_brk TBROK "failed to insert zram module"
+	if [ ! -d "/sys/class/zram-control" ]; then
+		modprobe zram num_devices=$dev_num
+		module_load=1
+		dev_start=0
+		dev_end=$(($dev_num - 1))
+		tst_res TPASS "all zram devices(/dev/zram0~$dev_end) successfully created"
+		return
+	fi
 
-	dev_num_created=$(ls /dev/zram* | wc -w)
+	dev_start=$(ls /dev/zram* | wc -w)
+	dev_end=$(($dev_start + $dev_num - 1))
 
-	if [ "$dev_num_created" -ne "$dev_num" ]; then
-		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
-	fi
+	for i in $(seq  $dev_start $dev_end); do
+		cat /sys/class/zram-control/hot_add > /dev/null
+	done
 
-	tst_res TPASS "all zram devices successfully created"
+	tst_res TPASS "all zram devices(/dev/zram$dev_start~$dev_end) successfully created"
 }
 
 zram_max_streams()
@@ -73,7 +89,7 @@ zram_max_streams()
 
 	tst_res TINFO "set max_comp_streams to zram device(s)"
 
-	local i=0
+	local i=$dev_start
 
 	for max_s in $zram_max_streams; do
 		local sys_path="/sys/block/zram${i}/max_comp_streams"
@@ -85,7 +101,7 @@ zram_max_streams()
 			tst_brk TFAIL "can't set max_streams '$max_s', get $max_stream"
 
 		i=$(($i + 1))
-		tst_res TINFO "$sys_path = '$max_streams' ($i/$dev_num)"
+		tst_res TINFO "$sys_path = '$max_streams'"
 	done
 
 	tst_res TPASS "test succeeded"
@@ -100,20 +116,18 @@ zram_compress_alg()
 		return
 	fi
 
-	local i=0
+	local i=$dev_start
 
 	tst_res TINFO "test that we can set compression algorithm"
-	local algs="$(sed 's/[][]//g' /sys/block/zram0/comp_algorithm)"
+	local algs="$(sed 's/[][]//g' /sys/block/zram${i}/comp_algorithm)"
 	tst_res TINFO "supported algs: $algs"
 
-	local dev_max=$(($dev_num - 1))
-
-	for i in $(seq 0 $dev_max); do
+	for i in $(seq $dev_start $dev_end); do
 		for alg in $algs; do
 			local sys_path="/sys/block/zram${i}/comp_algorithm"
 			echo "$alg" >  $sys_path || \
 				tst_brk TFAIL "can't set '$alg' to $sys_path"
-			tst_res TINFO "$sys_path = '$alg' ($i/$dev_max)"
+			tst_res TINFO "$sys_path = '$alg'"
 		done
 	done
 
@@ -122,7 +136,7 @@ zram_compress_alg()
 
 zram_set_disksizes()
 {
-	local i=0
+	local i=$dev_start
 	local ds
 
 	tst_res TINFO "set disk size to zram device(s)"
@@ -132,7 +146,7 @@ zram_set_disksizes()
 			tst_brk TFAIL "can't set '$ds' to $sys_path"
 
 		i=$(($i + 1))
-		tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
+		tst_res TINFO "$sys_path = '$ds'"
 	done
 
 	tst_res TPASS "test succeeded"
@@ -147,7 +161,7 @@ zram_set_memlimit()
 		return
 	fi
 
-	local i=0
+	local i=$dev_start
 	local ds
 
 	tst_res TINFO "set memory limit to zram device(s)"
@@ -158,7 +172,7 @@ zram_set_memlimit()
 			tst_brk TFAIL "can't set '$ds' to $sys_path"
 
 		i=$(($i + 1))
-		tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
+		tst_res TINFO "$sys_path = '$ds'"
 	done
 
 	tst_res TPASS "test succeeded"
-- 
2.23.0


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

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

* [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api
  2021-12-15  7:20 [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Yang Xu
  2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
@ 2021-12-15  7:20 ` Yang Xu
  2021-12-17  8:22   ` Petr Vorel
  2021-12-17  8:24   ` Petr Vorel
  2021-12-15  7:20 ` [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field Yang Xu
  2021-12-17  7:01 ` [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Petr Vorel
  3 siblings, 2 replies; 18+ messages in thread
From: Yang Xu @ 2021-12-15  7:20 UTC (permalink / raw)
  To: ltp

Also add hot_add/hot_remove in setup/cleanup, so this case can adapt the situation that
zram module is being used by zram-generator or zram module is builtin.

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

diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
index 06995fd56..6f6f1db77 100644
--- a/testcases/kernel/device-drivers/zram/zram03.c
+++ b/testcases/kernel/device-drivers/zram/zram03.c
@@ -1,90 +1,45 @@
+// 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 <stdlib.h>
+#include "tst_safe_stdio.h"
+#include "tst_test.h"
 
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "zram03";
-int TST_TOTAL = 1;
-
-#define PATH_ZRAM	"/sys/block/zram0"
-#define OBSOLETE_ZRAM_FILE	"/sys/block/zram0/num_reads"
-#define PATH_ZRAM_STAT	"/sys/block/zram0/stat"
-#define PATH_ZRAM_MM_STAT	"/sys/block/zram0/mm_stat"
-#define SIZE		(512 * 1024 * 1024L)
-#define DEVICE		"/dev/zram0"
+#define ZRAM_CONTROL_PATH 	"/sys/class/zram-control"
+#define HOT_ADD_PATH      	ZRAM_CONTROL_PATH"/hot_add"
+#define HOT_REMOVE_PATH   	ZRAM_CONTROL_PATH"/hot_remove"
+#define SIZE                    (512 * 1024 * 1024L)
 
-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 char zram_block_path[100], zram_dev_path[100], buf[20];
+static int modprobe, dev_num, hot_add_flag;
 
 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);
+	char disksize_path[200];
+	tst_res(TINFO, "create a zram device with %ld bytes in size", SIZE);
+
+	sprintf(disksize_path, "%s/disksize", zram_block_path);
+	SAFE_FILE_PRINTF(disksize_path, "%ld", SIZE);
 }
 
 static void write_device(void)
@@ -92,17 +47,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(zram_dev_path, 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 +65,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(zram_dev_path, O_RDONLY);
+	s = SAFE_MMAP(NULL, SIZE, PROT_READ, MAP_PRIVATE, fd, 0);
 
 	while (s[i] && i < SIZE - 1) {
 		if (s[i] != 'a')
@@ -121,104 +75,73 @@ 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");
-	}
+	char reset_path[200];
 
-	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");
+	sprintf(reset_path, "%s/reset", zram_block_path);
+	SAFE_FILE_PRINTF(reset_path, "1");
 }
 
 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);
+	tst_res(TINFO, "%s",  zram_block_path);
+	sprintf(filename, "%s/%s", zram_block_path, string);
+	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];
+	char zram_stat_path[100];
 
-	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);
+	sprintf(zram_stat_path, "/sys/block/zram%d/stat", dev_num);
+	SAFE_FILE_SCANF(zram_stat_path, "%s %*s %*s %*s %s", nread_val, nwrite_val);
+	tst_res(TINFO, "%s from %s is %s", nread, zram_stat_path, nread_val);
+	tst_res(TINFO, "%s from %s is %s", nwrite, zram_stat_path, nwrite_val);
 }
 
 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];
+	char zram_mm_stat_path[100];
 
-	SAFE_FILE_SCANF(cleanup, PATH_ZRAM_MM_STAT, "%s %s %s %*s %*s %s",
+	sprintf(zram_mm_stat_path, "/sys/block/zram%d/mm_stat", dev_num);
+	SAFE_FILE_SCANF(zram_mm_stat_path, "%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, zram_mm_stat_path, orig_val);
+	tst_res(TINFO, "%s from %s is %s", compr, zram_mm_stat_path, compr_val);
+	tst_res(TINFO, "%s from %s is %s", mem, zram_mm_stat_path, mem_val);
+	tst_res(TINFO, "%s from %s is %s", zero, zram_mm_stat_path, zero_val);
 }
 
 static void dump_info(void)
 {
+	char zram_obsolete_file_path[100];
+
+	sprintf(zram_obsolete_file_path, "/sys/block/zram%d/num_reads", dev_num);
 	print("initstate");
 	print("disksize");
-	if (!access(OBSOLETE_ZRAM_FILE, F_OK)) {
+	if (!access(zram_obsolete_file_path, F_OK)) {
 		print("orig_data_size");
 		print("compr_data_size");
 		print("mem_used_total");
@@ -231,3 +154,65 @@ 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};
+	int hot_add_fd;
+
+	if (!access(ZRAM_CONTROL_PATH, F_OK)) {
+		hot_add_fd = SAFE_OPEN(HOT_ADD_PATH, O_RDONLY);
+		SAFE_READ(0, hot_add_fd, &buf, 20);
+		dev_num = atoi(buf);
+		SAFE_CLOSE(hot_add_fd);
+		hot_add_flag = 1;
+	} else {
+		SAFE_CMD(cmd_modprobe, NULL, NULL);
+		modprobe = 1;
+	}
+	sprintf(zram_block_path, "/sys/block/zram%d", dev_num);
+	sprintf(zram_dev_path, "/dev/zram%d", dev_num);
+}
+
+static void cleanup(void)
+{
+	const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
+	int hot_remove_fd;
+
+	if (hot_add_flag) {
+		hot_remove_fd = SAFE_OPEN(HOT_REMOVE_PATH, O_WRONLY);
+		SAFE_WRITE(0, hot_remove_fd, buf, 20);
+		SAFE_CLOSE(hot_remove_fd);
+	}
+
+	if (modprobe)
+		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

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

* [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field
  2021-12-15  7:20 [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Yang Xu
  2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
  2021-12-15  7:20 ` [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api Yang Xu
@ 2021-12-15  7:20 ` Yang Xu
  2021-12-17  8:47   ` Petr Vorel
  2021-12-17  7:01 ` [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Petr Vorel
  3 siblings, 1 reply; 18+ messages in thread
From: Yang Xu @ 2021-12-15  7:20 UTC (permalink / raw)
  To: ltp

Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we
used free -m changes to calculate the compression ratio.

After the above patch, we used compr_data_size to calculate. kernel documentation[2] has
the following info:
orig_data_size: uncompressed size of data stored in this disk.
compr_data_size: compressed size of data stored in this disk
mem_used_total: the amount of memory allocated for this disk

We should calculate the compression ratio by used disk size divided into used mem size.
It can also avoid the situation that division by 0 as below:
zram01 7 TINFO: filling zram4 (it can take long time)
zram01 7 TPASS: zram4 was filled with '25568' KB
zram01 7 TINFO: compr_size 0
 /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0")

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/device-drivers/zram/zram01.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index 5e13f387c..5b4c05434 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -125,8 +125,8 @@ zram_fill_fs()
 			continue
 		fi
 
-		local compr_size=`awk '{print $2}' "/sys/block/zram$i/mm_stat"`
-		local v=$((100 * 1024 * $b / $compr_size))
+		local mem_used_total=`awk '{print $3}' "/sys/block/zram$i/mm_stat"`
+		local v=$((100 * 1024 * $b / $mem_used_total))
 		local r=`echo "scale=2; $v / 100 " | bc`
 
 		if [ "$v" -lt 100 ]; then
-- 
2.23.0


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

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

* Re: [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used
  2021-12-15  7:20 [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Yang Xu
                   ` (2 preceding siblings ...)
  2021-12-15  7:20 ` [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field Yang Xu
@ 2021-12-17  7:01 ` Petr Vorel
  2021-12-17  7:04   ` xuyang2018.jy
  3 siblings, 1 reply; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  7:01 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi 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.

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

Could you please fix this before merge?
swapping01.c:84: ERROR: switch and case should be at the same indent

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used
  2021-12-17  7:01 ` [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Petr Vorel
@ 2021-12-17  7:04   ` xuyang2018.jy
  0 siblings, 0 replies; 18+ messages in thread
From: xuyang2018.jy @ 2021-12-17  7:04 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr
> Hi 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.
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
> Could you please fix this before merge?
> swapping01.c:84: ERROR: switch and case should be at the same indent
Thanks for your review, of course,  Will fix this when merge.

Best Regards
Yang Xu
>
> Kind regards,
> Petr

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
@ 2021-12-17  7:48   ` Petr Vorel
  2021-12-17  9:34     ` xuyang2018.jy
  2021-12-17  7:49   ` Petr Vorel
  1 sibling, 1 reply; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  7:48 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Xu,

> If zram-generator package is installed and works, then we can not remove zram module
> because zram swap is being used. This case needs a clean zram environment, change this
> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
> still can add zram device and remove them in cleanup.

BTW this was added in v4.2-rc1 (6 years ago, 6566d1a32bf7 ("zram: add dynamic
device add/remove functionality")). Hopefully anybody still supporting older
kernels is using old LTP for it.

> Also, zram01,02 case are adjuested to adapt the situation that CONFIG_ZRAM=y.
Very nice, thanks!

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

> 1]https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices
nit: [1] https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices

...
> -	modprobe zram num_devices=$dev_num || \
> -		tst_brk TBROK "failed to insert zram module"
> +	if [ ! -d "/sys/class/zram-control" ]; then
> +		modprobe zram num_devices=$dev_num
> +		module_load=1
> +		dev_start=0
> +		dev_end=$(($dev_num - 1))
> +		tst_res TPASS "all zram devices(/dev/zram0~$dev_end) successfully created"
nit: please all zram devices (/dev/zram0~$dev_end) ... (add space after devices)
> +		return
> +	fi

> -	dev_num_created=$(ls /dev/zram* | wc -w)
> +	dev_start=$(ls /dev/zram* | wc -w)
> +	dev_end=$(($dev_start + $dev_num - 1))

> -	if [ "$dev_num_created" -ne "$dev_num" ]; then
> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
> -	fi
> +	for i in $(seq  $dev_start $dev_end); do
> +		cat /sys/class/zram-control/hot_add > /dev/null
> +	done

> -	tst_res TPASS "all zram devices successfully created"
> +	tst_res TPASS "all zram devices(/dev/zram$dev_start~$dev_end) successfully created"
and here.

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
  2021-12-17  7:48   ` Petr Vorel
@ 2021-12-17  7:49   ` Petr Vorel
  1 sibling, 0 replies; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  7:49 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

> If zram-generator package is installed and works, then we can not remove zram module
> because zram swap is being used. This case needs a clean zram environment, change this
> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
> still can add zram device and remove them in cleanup.
BTW this also helped to run zram01.sh simultaneously, which is nice. Thank you.

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api
  2021-12-15  7:20 ` [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api Yang Xu
@ 2021-12-17  8:22   ` Petr Vorel
  2021-12-17  9:42     ` xuyang2018.jy
  2021-12-17  8:24   ` Petr Vorel
  1 sibling, 1 reply; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  8:22 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Xu,

> Also add hot_add/hot_remove in setup/cleanup, so this case can adapt the situation that
> zram module is being used by zram-generator or zram module is builtin.
Very nice. Again, I like you added both CONFIG_ZRAM=y support and simultaneous
run.

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

> diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
...
> - * 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.
>   */

> +
nit: I'd remove this extra line
>  #include <sys/types.h>
>  #include <sys/stat.h>

...
>  	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");
nit: I'd join this line (less than over 100)
>  	}

...
> +static void setup(void)
> +{
> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
> +	int hot_add_fd;
> +
> +	if (!access(ZRAM_CONTROL_PATH, F_OK)) {
> +		hot_add_fd = SAFE_OPEN(HOT_ADD_PATH, O_RDONLY);
> +		SAFE_READ(0, hot_add_fd, &buf, 20);
> +		dev_num = atoi(buf);
> +		SAFE_CLOSE(hot_add_fd);
> +		hot_add_flag = 1;
We have SAFE_FILE_SCANF(), you can use just:
		SAFE_FILE_SCANF(HOT_ADD_PATH, "%d", &dev_num);

> +	} else {
> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
> +		modprobe = 1;
> +	}
> +	sprintf(zram_block_path, "/sys/block/zram%d", dev_num);
> +	sprintf(zram_dev_path, "/dev/zram%d", dev_num);
> +}
> +
> +static void cleanup(void)
> +{
> +	const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
> +	int hot_remove_fd;
> +
> +	if (hot_add_flag) {
> +		hot_remove_fd = SAFE_OPEN(HOT_REMOVE_PATH, O_WRONLY);
> +		SAFE_WRITE(0, hot_remove_fd, buf, 20);
> +		SAFE_CLOSE(hot_remove_fd);
> +	}
Ad here
	if (hot_add_flag)
		SAFE_FILE_PRINTF(HOT_REMOVE_PATH, "%d", dev_num);

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api
  2021-12-15  7:20 ` [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api Yang Xu
  2021-12-17  8:22   ` Petr Vorel
@ 2021-12-17  8:24   ` Petr Vorel
  1 sibling, 0 replies; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  8:24 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Xu,

nit: checkpatch.pl reports:
zram03.c:28: WARNING: please, no space before tabs
zram03.c:29: WARNING: please, no space before tabs
zram03.c:30: WARNING: please, no space before tabs
zram03.c:39: WARNING: Missing a blank line after declarations
zram03.c:97: WARNING: Prefer using '"%s...", __func__' to using 'reset', this function's name, in a string
zram03.c:104: WARNING: Missing a blank line after declarations

At least "no space before tabs" and "Missing a blank line" are easily to be
fixed.

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field
  2021-12-15  7:20 ` [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field Yang Xu
@ 2021-12-17  8:47   ` Petr Vorel
  2021-12-17  9:45     ` xuyang2018.jy
  0 siblings, 1 reply; 18+ messages in thread
From: Petr Vorel @ 2021-12-17  8:47 UTC (permalink / raw)
  To: Yang Xu; +Cc: Martin Doucha, ltp

Hi,

[ Cc Martin ]

> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we
> used free -m changes to calculate the compression ratio.

> After the above patch, we used compr_data_size to calculate. kernel documentation[2] has
> the following info:
> orig_data_size: uncompressed size of data stored in this disk.
> compr_data_size: compressed size of data stored in this disk
> mem_used_total: the amount of memory allocated for this disk

> We should calculate the compression ratio by used disk size divided into used mem size.
> It can also avoid the situation that division by 0 as below:
> zram01 7 TINFO: filling zram4 (it can take long time)
> zram01 7 TPASS: zram4 was filled with '25568' KB
> zram01 7 TINFO: compr_size 0
>  /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0")

Thank you for addressing this issue. replacing "data *stored* in this disk" with
"*allocated* for this disk" could help (although looking at kernel code
mm_stat_show(), I would not be sure).

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

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-17  7:48   ` Petr Vorel
@ 2021-12-17  9:34     ` xuyang2018.jy
  2021-12-17 10:34       ` Petr Vorel
  0 siblings, 1 reply; 18+ messages in thread
From: xuyang2018.jy @ 2021-12-17  9:34 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr
> Hi Xu,
>
>> If zram-generator package is installed and works, then we can not remove zram module
>> because zram swap is being used. This case needs a clean zram environment, change this
>> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
>> still can add zram device and remove them in cleanup.
>
> BTW this was added in v4.2-rc1 (6 years ago, 6566d1a32bf7 ("zram: add dynamic
> device add/remove functionality")). Hopefully anybody still supporting older
> kernels is using old LTP for it.
Oh, I don't realize it before. I tested it on centos7 then I think this 
control interface maybe introduced long time ago.

To be honst, I don't want to make this case more complex. How about 
adding  /sys/class/zram-control check after load zram module. If not, 
just report  case needs to use hot_add/hot_remove interface .

Old kernel can use old ltp since it is 4.2 not like 4.4 4.19(they are 
long stable branch, we should consider them).

>
>> Also, zram01,02 case are adjuested to adapt the situation that CONFIG_ZRAM=y.
> Very nice, thanks!
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
>> 1]https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices
> nit: [1] https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices
>
> ...
>> -	modprobe zram num_devices=$dev_num || \
>> -		tst_brk TBROK "failed to insert zram module"
>> +	if [ ! -d "/sys/class/zram-control" ]; then
>> +		modprobe zram num_devices=$dev_num
>> +		module_load=1
>> +		dev_start=0
>> +		dev_end=$(($dev_num - 1))
>> +		tst_res TPASS "all zram devices(/dev/zram0~$dev_end) successfully created"
> nit: please all zram devices (/dev/zram0~$dev_end) ... (add space after devices)
OK
>> +		return
>> +	fi
>
>> -	dev_num_created=$(ls /dev/zram* | wc -w)
>> +	dev_start=$(ls /dev/zram* | wc -w)
>> +	dev_end=$(($dev_start + $dev_num - 1))
>
>> -	if [ "$dev_num_created" -ne "$dev_num" ]; then
>> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
>> -	fi
>> +	for i in $(seq  $dev_start $dev_end); do
>> +		cat /sys/class/zram-control/hot_add>  /dev/null
>> +	done
>
>> -	tst_res TPASS "all zram devices successfully created"
>> +	tst_res TPASS "all zram devices(/dev/zram$dev_start~$dev_end) successfully created"
> and here.
OK

Best Regards
Yang Xu
>
> Kind regards,
> Petr

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

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

* Re: [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api
  2021-12-17  8:22   ` Petr Vorel
@ 2021-12-17  9:42     ` xuyang2018.jy
  0 siblings, 0 replies; 18+ messages in thread
From: xuyang2018.jy @ 2021-12-17  9:42 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr
> Hi Xu,
>
>> Also add hot_add/hot_remove in setup/cleanup, so this case can adapt the situation that
>> zram module is being used by zram-generator or zram module is builtin.
> Very nice. Again, I like you added both CONFIG_ZRAM=y support and simultaneous
> run.
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
>> diff --git a/testcases/kernel/device-drivers/zram/zram03.c b/testcases/kernel/device-drivers/zram/zram03.c
> ...
>> - * 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.
>>    */
>
>> +
> nit: I'd remove this extra line
OK
>>   #include<sys/types.h>
>>   #include<sys/stat.h>
>
> ...
>>   	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");
> nit: I'd join this line (less than over 100)
ok
>>   	}
>
> ...
>> +static void setup(void)
>> +{
>> +	const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
>> +	int hot_add_fd;
>> +
>> +	if (!access(ZRAM_CONTROL_PATH, F_OK)) {
>> +		hot_add_fd = SAFE_OPEN(HOT_ADD_PATH, O_RDONLY);
>> +		SAFE_READ(0, hot_add_fd,&buf, 20);
>> +		dev_num = atoi(buf);
>> +		SAFE_CLOSE(hot_add_fd);
>> +		hot_add_flag = 1;
> We have SAFE_FILE_SCANF(), you can use just:
> 		SAFE_FILE_SCANF(HOT_ADD_PATH, "%d",&dev_num);
Oh, yes, I forgot it...
>
>> +	} else {
>> +		SAFE_CMD(cmd_modprobe, NULL, NULL);
>> +		modprobe = 1;
>> +	}
>> +	sprintf(zram_block_path, "/sys/block/zram%d", dev_num);
>> +	sprintf(zram_dev_path, "/dev/zram%d", dev_num);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	const char *const cmd_rmmod[] = {"rmmod", "zram", NULL};
>> +	int hot_remove_fd;
>> +
>> +	if (hot_add_flag) {
>> +		hot_remove_fd = SAFE_OPEN(HOT_REMOVE_PATH, O_WRONLY);
>> +		SAFE_WRITE(0, hot_remove_fd, buf, 20);
>> +		SAFE_CLOSE(hot_remove_fd);
>> +	}
> Ad here
> 	if (hot_add_flag)
> 		SAFE_FILE_PRINTF(HOT_REMOVE_PATH, "%d", dev_num);
>
> Kind regards,
> Petr

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

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

* Re: [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field
  2021-12-17  8:47   ` Petr Vorel
@ 2021-12-17  9:45     ` xuyang2018.jy
  2021-12-17 10:36       ` Petr Vorel
  0 siblings, 1 reply; 18+ messages in thread
From: xuyang2018.jy @ 2021-12-17  9:45 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Martin Doucha, ltp

Hi Petr
> Hi,
>
> [ Cc Martin ]
>
>> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we
>> used free -m changes to calculate the compression ratio.
>
>> After the above patch, we used compr_data_size to calculate. kernel documentation[2] has
>> the following info:
>> orig_data_size: uncompressed size of data stored in this disk.
>> compr_data_size: compressed size of data stored in this disk
>> mem_used_total: the amount of memory allocated for this disk
>
>> We should calculate the compression ratio by used disk size divided into used mem size.
>> It can also avoid the situation that division by 0 as below:
>> zram01 7 TINFO: filling zram4 (it can take long time)
>> zram01 7 TPASS: zram4 was filled with '25568' KB
>> zram01 7 TINFO: compr_size 0
>>   /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0")
>
> Thank you for addressing this issue. replacing "data *stored* in this disk" with
> "*allocated* for this disk" could help (although looking at kernel code
> mm_stat_show(), I would not be sure).
Sound reasonable.

Will send a v5 next week and wait some time to listen more advise from 
others for this patchset.

Best Regards
Yang Xu
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
> Kind regards,
> Petr

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-17  9:34     ` xuyang2018.jy
@ 2021-12-17 10:34       ` Petr Vorel
  2021-12-20  2:58         ` xuyang2018.jy
  0 siblings, 1 reply; 18+ messages in thread
From: Petr Vorel @ 2021-12-17 10:34 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

Hi Xu,

> >> If zram-generator package is installed and works, then we can not remove zram module
> >> because zram swap is being used. This case needs a clean zram environment, change this
> >> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
> >> still can add zram device and remove them in cleanup.

> > BTW this was added in v4.2-rc1 (6 years ago, 6566d1a32bf7 ("zram: add dynamic
> > device add/remove functionality")). Hopefully anybody still supporting older
> > kernels is using old LTP for it.
> Oh, I don't realize it before. I tested it on centos7 then I think this 
> control interface maybe introduced long time ago.

> To be honst, I don't want to make this case more complex. How about 
> adding  /sys/class/zram-control check after load zram module. If not, 
> just report  case needs to use hot_add/hot_remove interface .

Would it work something like this?

ZRAM_SYSCTL_KERNEL_VERSION="4.2"
...
zram_cleanup()
{
...
	if tst_kvcmp -lt $ZRAM_SYSCTL_KERNEL_VERSION; then
		for i in $(seq $dev_start $dev_end); do
			echo $i > /sys/class/zram-control/hot_remove
		done
	fi

zram_load()
{
...
	if [ ! -d "/sys/class/zram-control" ]; then
		modprobe zram num_devices=$dev_num
		module_load=1
		dev_start=0
		dev_end=$(($dev_num - 1))
		tst_res TPASS "all zram devices(/dev/zram0~$dev_end) successfully created"
		return
	fi

	tst_kvcmp -lt $ZRAM_SYSCTL_KERNEL_VERSION && \
		tst_brk TCONF "test requires kernel $ZRAM_SYSCTL_KERNEL_VERSION+"

	dev_start=$(ls /dev/zram* | wc -w)
	dev_end=$(($dev_start + $dev_num - 1))

	for i in $(seq  $dev_start $dev_end); do
		cat /sys/class/zram-control/hot_add > /dev/null
	done

Kind regards,
Petr

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

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

* Re: [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field
  2021-12-17  9:45     ` xuyang2018.jy
@ 2021-12-17 10:36       ` Petr Vorel
  0 siblings, 0 replies; 18+ messages in thread
From: Petr Vorel @ 2021-12-17 10:36 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: Martin Doucha, ltp

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

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-17 10:34       ` Petr Vorel
@ 2021-12-20  2:58         ` xuyang2018.jy
  2021-12-20  8:52           ` Petr Vorel
  0 siblings, 1 reply; 18+ messages in thread
From: xuyang2018.jy @ 2021-12-20  2:58 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi Petr
> Hi Xu,
>
>>>> If zram-generator package is installed and works, then we can not remove zram module
>>>> because zram swap is being used. This case needs a clean zram environment, change this
>>>> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
>>>> still can add zram device and remove them in cleanup.
>
>>> BTW this was added in v4.2-rc1 (6 years ago, 6566d1a32bf7 ("zram: add dynamic
>>> device add/remove functionality")). Hopefully anybody still supporting older
>>> kernels is using old LTP for it.
>> Oh, I don't realize it before. I tested it on centos7 then I think this
>> control interface maybe introduced long time ago.
>
>> To be honst, I don't want to make this case more complex. How about
>> adding  /sys/class/zram-control check after load zram module. If not,
>> just report  case needs to use hot_add/hot_remove interface .
>
> Would it work something like this?
I don't want to use kernel version check because the hot_add/hot_remove 
interface is easy to backport. I will send a v5
1) new kernel and not load zram kernel module, then modprobe and rmmod 
is enough, doesn't need to use hot_add/hot_remove
2) new kernel and load zram kernel module or built in kernel, use 
hot_add/hot_remove
3) old kernel and can be modprobe and rmmod, work as 1)
4)old kernel but is being used or built in kernel, then skip this case 
like patch v3 does

Best Regards
Yang Xu
>
> ZRAM_SYSCTL_KERNEL_VERSION="4.2"
> ...
> zram_cleanup()
> {
> ...
> 	if tst_kvcmp -lt $ZRAM_SYSCTL_KERNEL_VERSION; then
> 		for i in $(seq $dev_start $dev_end); do
> 			echo $i>  /sys/class/zram-control/hot_remove
> 		done
> 	fi
>
> zram_load()
> {
> ...
> 	if [ ! -d "/sys/class/zram-control" ]; then
> 		modprobe zram num_devices=$dev_num
> 		module_load=1
> 		dev_start=0
> 		dev_end=$(($dev_num - 1))
> 		tst_res TPASS "all zram devices(/dev/zram0~$dev_end) successfully created"
> 		return
> 	fi
>
> 	tst_kvcmp -lt $ZRAM_SYSCTL_KERNEL_VERSION&&  \
> 		tst_brk TCONF "test requires kernel $ZRAM_SYSCTL_KERNEL_VERSION+"
>
> 	dev_start=$(ls /dev/zram* | wc -w)
> 	dev_end=$(($dev_start + $dev_num - 1))
>
> 	for i in $(seq  $dev_start $dev_end); do
> 		cat /sys/class/zram-control/hot_add>  /dev/null
> 	done
>
> Kind regards,
> Petr

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

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

* Re: [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
  2021-12-20  2:58         ` xuyang2018.jy
@ 2021-12-20  8:52           ` Petr Vorel
  0 siblings, 0 replies; 18+ messages in thread
From: Petr Vorel @ 2021-12-20  8:52 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: ltp

> Hi Petr
> > Hi Xu,

> >>>> If zram-generator package is installed and works, then we can not remove zram module
> >>>> because zram swap is being used. This case needs a clean zram environment, change this
> >>>> test by using hot_add/hot_remove interface[1]. So even zram device is being used, we
> >>>> still can add zram device and remove them in cleanup.

> >>> BTW this was added in v4.2-rc1 (6 years ago, 6566d1a32bf7 ("zram: add dynamic
> >>> device add/remove functionality")). Hopefully anybody still supporting older
> >>> kernels is using old LTP for it.
> >> Oh, I don't realize it before. I tested it on centos7 then I think this
> >> control interface maybe introduced long time ago.

> >> To be honst, I don't want to make this case more complex. How about
> >> adding  /sys/class/zram-control check after load zram module. If not,
> >> just report  case needs to use hot_add/hot_remove interface .

> > Would it work something like this?
> I don't want to use kernel version check because the hot_add/hot_remove 
> interface is easy to backport. I will send a v5
> 1) new kernel and not load zram kernel module, then modprobe and rmmod 
> is enough, doesn't need to use hot_add/hot_remove
> 2) new kernel and load zram kernel module or built in kernel, use 
> hot_add/hot_remove
> 3) old kernel and can be modprobe and rmmod, work as 1)
> 4)old kernel but is being used or built in kernel, then skip this case 
> like patch v3 does
Sure, make sense (although you didn't avoid more complexity).
Thanks working on this!

Kind regards,
Petr

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

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

end of thread, other threads:[~2021-12-20  8:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15  7:20 [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Yang Xu
2021-12-15  7:20 ` [LTP] [PATCH v4 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
2021-12-17  7:48   ` Petr Vorel
2021-12-17  9:34     ` xuyang2018.jy
2021-12-17 10:34       ` Petr Vorel
2021-12-20  2:58         ` xuyang2018.jy
2021-12-20  8:52           ` Petr Vorel
2021-12-17  7:49   ` Petr Vorel
2021-12-15  7:20 ` [LTP] [PATCH v4 3/4] zram/zram03: Convert into new api Yang Xu
2021-12-17  8:22   ` Petr Vorel
2021-12-17  9:42     ` xuyang2018.jy
2021-12-17  8:24   ` Petr Vorel
2021-12-15  7:20 ` [LTP] [PATCH v4 4/4] zram/zram01.sh: Use mem_used_total field instead of compr_data_size field Yang Xu
2021-12-17  8:47   ` Petr Vorel
2021-12-17  9:45     ` xuyang2018.jy
2021-12-17 10:36       ` Petr Vorel
2021-12-17  7:01 ` [LTP] [PATCH v4 1/4] swapping01: skip test if zram-swap is being used Petr Vorel
2021-12-17  7:04   ` xuyang2018.jy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.