* [LTP] [PATCH v2 0/6] zram cleanup
@ 2021-01-29 19:41 Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 1/6] zram: Calculate dev_num variable Petr Vorel
` (5 more replies)
0 siblings, 6 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Hi,
changes v1->v2:
* rewritten variable setup in commit
zram01.sh: Check properly mkfs.* dependencies
to reflect Cyril's suggestions
* 2 new commits related to timeout:
tst_test.sh: Run _tst_setup_timer after $TST_SETUP
zram: Increase timeout according to used devices
Kind regards,
Petr
Petr Vorel (6):
zram: Calculate dev_num variable
zram01.sh: Generate test setup variables in setup
zram: Move zram_compress_alg() to zram02.sh
zram: Move test specific functions out of zram_lib.sh
tst_test.sh: Run _tst_setup_timer after $TST_SETUP
zram: Increase timeout according to used devices
doc/test-writing-guidelines.txt | 5 +-
.../kernel/device-drivers/zram/zram01.sh | 113 +++++++++++++----
.../kernel/device-drivers/zram/zram02.sh | 77 ++++++++++--
.../kernel/device-drivers/zram/zram_lib.sh | 116 +++---------------
testcases/lib/tst_test.sh | 4 +-
5 files changed, 180 insertions(+), 135 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 1/6] zram: Calculate dev_num variable
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
` (4 subsequent siblings)
5 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Instead of requiring to be set.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
Added paranoid check:
+ if [ $dev_num -le 0 ]; then
+ tst_brk TBROK "dev_num must be > 0"
+ fi
Kind regards,
Petr
testcases/kernel/device-drivers/zram/zram01.sh | 8 +++-----
testcases/kernel/device-drivers/zram/zram02.sh | 8 +++-----
testcases/kernel/device-drivers/zram/zram_lib.sh | 16 ++++++++++++++--
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index 8de2c0cad..a795ff89f 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -1,6 +1,6 @@
#!/bin/sh
# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
-# Copyright (c) 2019 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2019-2021 Petr Vorel <pvorel@suse.cz>
# Author: Alexey Kodanev <alexey.kodanev@oracle.com>
#
# Test creates several zram devices with different filesystems on them.
@@ -11,10 +11,8 @@ TST_TESTFUNC="do_test"
TST_NEEDS_CMDS="awk bc dd"
. zram_lib.sh
-# Test will create the following number of zram devices:
-dev_num=4
-# This is a list of parameters for zram devices.
-# Number of items must be equal to 'dev_num' parameter.
+# List of parameters for zram devices.
+# For each number the test creates own zram device.
zram_max_streams="2 3 5 8"
FS_SIZE="402653184"
diff --git a/testcases/kernel/device-drivers/zram/zram02.sh b/testcases/kernel/device-drivers/zram/zram02.sh
index f97cf646c..b4d706568 100755
--- a/testcases/kernel/device-drivers/zram/zram02.sh
+++ b/testcases/kernel/device-drivers/zram/zram02.sh
@@ -1,6 +1,6 @@
#!/bin/sh
# Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
-# Copyright (c) 2019 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2019-2021 Petr Vorel <pvorel@suse.cz>
# Author: Alexey Kodanev <alexey.kodanev@oracle.com>
#
# Test checks that we can create swap zram device.
@@ -9,10 +9,8 @@ TST_CNT=5
TST_TESTFUNC="do_test"
. zram_lib.sh
-# Test will create the following number of zram devices:
-dev_num=1
-# This is a list of parameters for zram devices.
-# Number of items must be equal to 'dev_num' parameter.
+# List of parameters for zram devices.
+# For each number the test creates own zram device.
zram_max_streams="2"
# The zram sysfs node 'disksize' value can be either in bytes,
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index 6fa6552ca..a7e8b9f5b 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -34,7 +34,19 @@ zram_cleanup()
zram_load()
{
+ local tmp
+
+ dev_num=0
+ for tmp in $zram_max_streams; do
+ dev_num=$((dev_num+1))
+ done
+
+ if [ $dev_num -le 0 ]; then
+ tst_brk TBROK "dev_num must be > 0"
+ fi
+
tst_res TINFO "create '$dev_num' zram device(s)"
+
modprobe zram num_devices=$dev_num || \
tst_brk TBROK "failed to insert zram module"
@@ -42,9 +54,9 @@ zram_load()
if [ "$dev_num_created" -ne "$dev_num" ]; then
tst_brk TFAIL "unexpected num of devices: $dev_num_created"
- else
- tst_res TPASS "test succeeded"
fi
+
+ tst_res TPASS "all zram devices successfully created"
}
zram_max_streams()
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 1/6] zram: Calculate dev_num variable Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-01-29 20:17 ` Petr Vorel
` (2 more replies)
2021-01-29 19:41 ` [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh Petr Vorel
` (3 subsequent siblings)
5 siblings, 3 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Generate variables in setup, based on output of tst_supported_fs.
This is more clean approach and it fixes various things:
* Error when there is no filesystem support and also mkfs.ext2
(fallback) not installed:
/opt/ltp/testcases/bin/zram01.sh: line 198: mkfs.ext2: not found
Instead quit if there is no fs support. But this can lead to skipping
zram_compress_alg(), it will be solved next commit by moving
zram_compress_alg() to zram02.sh.
* Having ext2 as fallback could lead to run it more than once.
There is no much point to do that.
* Drop tst_require_cmds mkfs check, because mkfs is not actually needed.
Improvements:
* Test all suitable filesystems (will need increase timeout).
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v1->v2:
Completely rewritten.
.../kernel/device-drivers/zram/zram01.sh | 62 ++++++++++++++-----
.../kernel/device-drivers/zram/zram_lib.sh | 18 +++---
2 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index a795ff89f..c5d4a3e51 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -8,23 +8,25 @@
TST_CNT=7
TST_TESTFUNC="do_test"
-TST_NEEDS_CMDS="awk bc dd"
+TST_NEEDS_CMDS="awk bc dd grep"
. zram_lib.sh
+TST_SETUP="setup"
-# List of parameters for zram devices.
-# For each number the test creates own zram device.
-zram_max_streams="2 3 5 8"
-
-FS_SIZE="402653184"
-FS_TYPE="btrfs"
+get_btrfs_size()
+{
+ local ram_size
-RAM_SIZE=$(awk '/MemTotal:/ {print $2}' /proc/meminfo)
-if [ "$RAM_SIZE" -lt 1048576 ]; then
- tst_res TINFO "not enough space for Btrfs"
- FS_SIZE="26214400"
- FS_TYPE="ext2"
-fi
+ ram_size=$(awk '/MemTotal:/ {print $2}' /proc/meminfo)
+ if [ "$ram_size" -lt 1048576 ]; then
+ tst_res TINFO "not enough space for Btrfs"
+ return 1
+ fi
+ return 0
+}
+# List of parameters for zram devices.
+# For each number the test creates own zram device.
+# NOTE about size:
# The zram sysfs node 'disksize' value can be either in bytes,
# or you can use mem suffixes. But in some old kernels, mem
# suffixes are not supported, for example, in RHEL6.6GA's kernel
@@ -32,9 +34,37 @@ fi
# not support mem suffixes, in some newer kernels, they use
# memparse() which supports mem suffixes. So here we just use
# bytes to make sure everything works correctly.
-zram_sizes="26214400 26214400 26214400 $FS_SIZE"
-zram_mem_limits="25M 25M 25M $((FS_SIZE/1024/1024))M"
-zram_filesystems="ext3 ext4 xfs $FS_TYPE"
+generate_vars()
+{
+ local fs limit size stream=-1
+ dev_num=0
+
+ for fs in $(tst_supported_fs | grep -v -e fat -e ntfs -e fuse); do
+ size="26214400"
+ limit="25M"
+ if [ "$fs" = "btrfs" ]; then
+ get_btrfs_size || continue
+ size="402653184"
+ limit="$((size/1024/1024))M"
+ fi
+
+ stream=$((stream+3))
+ dev_num=$((dev_num+1))
+ zram_filesystems="$zram_filesystems $fs"
+ zram_mem_limits="$zram_mem_limits $limit"
+ zram_sizes="$zram_sizes $size"
+ zram_max_streams="$zram_max_streams $stream"
+ done
+
+ [ $dev_num -eq 0 ] && \
+ tst_brk TCONF "no suitable filesystem"
+}
+
+setup()
+{
+ generate_vars
+ zram_load
+}
zram_fill_fs()
{
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index a7e8b9f5b..d4aaf0c3e 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -36,10 +36,12 @@ zram_load()
{
local tmp
- dev_num=0
- for tmp in $zram_max_streams; do
- dev_num=$((dev_num+1))
- done
+ if [ -z "$dev_num" ]; then
+ dev_num=0
+ for tmp in $zram_max_streams; do
+ dev_num=$((dev_num+1))
+ done
+ fi
if [ $dev_num -le 0 ]; then
tst_brk TBROK "dev_num must be > 0"
@@ -129,6 +131,7 @@ zram_set_disksizes()
i=$(($i + 1))
tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
+ [ $i -eq $dev_num ] && break
done
tst_res TPASS "test succeeded"
@@ -155,6 +158,7 @@ zram_set_memlimit()
i=$(($i + 1))
tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
+ [ $i -eq $dev_num ] && break
done
tst_res TPASS "test succeeded"
@@ -191,13 +195,10 @@ zram_swapoff()
zram_makefs()
{
- tst_require_cmds mkfs
local i=0
+ local fs
for fs in $zram_filesystems; do
- # if requested fs not supported default it to ext2
- tst_supported_fs $fs 2> /dev/null || fs=ext2
-
tst_res TINFO "make $fs filesystem on /dev/zram$i"
mkfs.$fs /dev/zram$i > err.log 2>&1
if [ $? -ne 0 ]; then
@@ -206,6 +207,7 @@ zram_makefs()
fi
i=$(($i + 1))
+ [ $i -eq $dev_num ] && break
done
tst_res TPASS "zram_makefs succeeded"
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 1/6] zram: Calculate dev_num variable Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-03-01 14:55 ` Cyril Hrubis
2021-01-29 19:41 ` [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh Petr Vorel
` (2 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Quit at setup in case there is no fs support (change in previous commit)
can lead to skipping zram_compress_alg(). Move to zram02.sh where is no
such limitation.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1.
testcases/kernel/device-drivers/zram/zram01.sh | 13 ++++++-------
testcases/kernel/device-drivers/zram/zram02.sh | 11 ++++++-----
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index c5d4a3e51..5e99f8bb8 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -6,7 +6,7 @@
# Test creates several zram devices with different filesystems on them.
# It fills each device with zeros and checks that compression works.
-TST_CNT=7
+TST_CNT=6
TST_TESTFUNC="do_test"
TST_NEEDS_CMDS="awk bc dd grep"
. zram_lib.sh
@@ -108,12 +108,11 @@ do_test()
{
case $1 in
1) zram_max_streams;;
- 2) zram_compress_alg;;
- 3) zram_set_disksizes;;
- 4) zram_set_memlimit;;
- 5) zram_makefs;;
- 6) zram_mount;;
- 7) zram_fill_fs;;
+ 2) zram_set_disksizes;;
+ 3) zram_set_memlimit;;
+ 4) zram_makefs;;
+ 5) zram_mount;;
+ 6) zram_fill_fs;;
esac
}
diff --git a/testcases/kernel/device-drivers/zram/zram02.sh b/testcases/kernel/device-drivers/zram/zram02.sh
index b4d706568..d09977ec1 100755
--- a/testcases/kernel/device-drivers/zram/zram02.sh
+++ b/testcases/kernel/device-drivers/zram/zram02.sh
@@ -5,7 +5,7 @@
#
# Test checks that we can create swap zram device.
-TST_CNT=5
+TST_CNT=6
TST_TESTFUNC="do_test"
. zram_lib.sh
@@ -27,10 +27,11 @@ do_test()
{
case $1 in
1) zram_max_streams;;
- 2) zram_set_disksizes;;
- 3) zram_set_memlimit;;
- 4) zram_makeswap;;
- 5) zram_swapoff;;
+ 2) zram_compress_alg;;
+ 3) zram_set_disksizes;;
+ 4) zram_set_memlimit;;
+ 5) zram_makeswap;;
+ 6) zram_swapoff;;
esac
}
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
` (2 preceding siblings ...)
2021-01-29 19:41 ` [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-03-01 14:57 ` Cyril Hrubis
2021-01-29 19:41 ` [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices Petr Vorel
5 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Refactoring, as code is confusing enough, when use global variables and
functions which are single test specific + there is zram_fill_fs already
in zram01.sh.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1.
.../kernel/device-drivers/zram/zram01.sh | 34 +++++++
.../kernel/device-drivers/zram/zram02.sh | 58 ++++++++++++
.../kernel/device-drivers/zram/zram_lib.sh | 92 -------------------
3 files changed, 92 insertions(+), 92 deletions(-)
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index 5e99f8bb8..695de14a1 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -66,6 +66,40 @@ setup()
zram_load
}
+zram_makefs()
+{
+ local i=0
+ local fs
+
+ for fs in $zram_filesystems; do
+ tst_res TINFO "make $fs filesystem on /dev/zram$i"
+ mkfs.$fs /dev/zram$i > err.log 2>&1
+ if [ $? -ne 0 ]; then
+ cat err.log
+ tst_brk TFAIL "failed to make $fs on /dev/zram$i"
+ fi
+
+ i=$(($i + 1))
+ [ $i -eq $dev_num ] && break
+ done
+
+ tst_res TPASS "zram_makefs succeeded"
+}
+
+zram_mount()
+{
+ local i=0
+
+ for i in $(seq 0 $(($dev_num - 1))); do
+ tst_res TINFO "mount /dev/zram$i"
+ mkdir zram$i
+ ROD mount /dev/zram$i zram$i
+ dev_mounted=$i
+ done
+
+ tst_res TPASS "mount of zram device(s) succeeded"
+}
+
zram_fill_fs()
{
for i in $(seq 0 $(($dev_num - 1))); do
diff --git a/testcases/kernel/device-drivers/zram/zram02.sh b/testcases/kernel/device-drivers/zram/zram02.sh
index d09977ec1..803b8dc29 100755
--- a/testcases/kernel/device-drivers/zram/zram02.sh
+++ b/testcases/kernel/device-drivers/zram/zram02.sh
@@ -23,6 +23,64 @@ zram_max_streams="2"
zram_sizes="107374182400" # 100GB
zram_mem_limits="1M"
+zram_compress_alg()
+{
+ if tst_kvcmp -lt "3.15"; then
+ tst_res TCONF "device attribute comp_algorithm is"\
+ "introduced since kernel v3.15, the running kernel"\
+ "does not support it"
+ return
+ fi
+
+ local i=0
+
+ tst_res TINFO "test that we can set compression algorithm"
+ local algs="$(sed 's/[][]//g' /sys/block/zram0/comp_algorithm)"
+ tst_res TINFO "supported algs: $algs"
+
+ local dev_max=$(($dev_num - 1))
+
+ for i in $(seq 0 $dev_max); 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)"
+ done
+ done
+
+ tst_res TPASS "test succeeded"
+}
+
+zram_makeswap()
+{
+ tst_res TINFO "make swap with zram device(s)"
+ tst_require_cmds mkswap swapon swapoff
+ local i=0
+
+ for i in $(seq 0 $(($dev_num - 1))); do
+ ROD mkswap /dev/zram$i
+ ROD swapon /dev/zram$i
+ tst_res TINFO "done with /dev/zram$i"
+ dev_makeswap=$i
+ done
+
+ tst_res TPASS "making zram swap succeeded"
+}
+
+zram_swapoff()
+{
+ tst_require_cmds swapoff
+ local i
+
+ for i in $(seq 0 $dev_makeswap); do
+ ROD swapoff /dev/zram$i
+ done
+ dev_makeswap=-1
+
+ tst_res TPASS "swapoff completed"
+}
+
do_test()
{
case $1 in
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index d4aaf0c3e..26ed1846b 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -89,35 +89,6 @@ zram_max_streams()
tst_res TPASS "test succeeded"
}
-zram_compress_alg()
-{
- if tst_kvcmp -lt "3.15"; then
- tst_res TCONF "device attribute comp_algorithm is"\
- "introduced since kernel v3.15, the running kernel"\
- "does not support it"
- return
- fi
-
- local i=0
-
- tst_res TINFO "test that we can set compression algorithm"
- local algs="$(sed 's/[][]//g' /sys/block/zram0/comp_algorithm)"
- tst_res TINFO "supported algs: $algs"
-
- local dev_max=$(($dev_num - 1))
-
- for i in $(seq 0 $dev_max); 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)"
- done
- done
-
- tst_res TPASS "test succeeded"
-}
-
zram_set_disksizes()
{
local i=0
@@ -163,66 +134,3 @@ zram_set_memlimit()
tst_res TPASS "test succeeded"
}
-
-zram_makeswap()
-{
- tst_res TINFO "make swap with zram device(s)"
- tst_require_cmds mkswap swapon swapoff
- local i=0
-
- for i in $(seq 0 $(($dev_num - 1))); do
- ROD mkswap /dev/zram$i
- ROD swapon /dev/zram$i
- tst_res TINFO "done with /dev/zram$i"
- dev_makeswap=$i
- done
-
- tst_res TPASS "making zram swap succeeded"
-}
-
-zram_swapoff()
-{
- tst_require_cmds swapoff
- local i
-
- for i in $(seq 0 $dev_makeswap); do
- ROD swapoff /dev/zram$i
- done
- dev_makeswap=-1
-
- tst_res TPASS "swapoff completed"
-}
-
-zram_makefs()
-{
- local i=0
- local fs
-
- for fs in $zram_filesystems; do
- tst_res TINFO "make $fs filesystem on /dev/zram$i"
- mkfs.$fs /dev/zram$i > err.log 2>&1
- if [ $? -ne 0 ]; then
- cat err.log
- tst_brk TFAIL "failed to make $fs on /dev/zram$i"
- fi
-
- i=$(($i + 1))
- [ $i -eq $dev_num ] && break
- done
-
- tst_res TPASS "zram_makefs succeeded"
-}
-
-zram_mount()
-{
- local i=0
-
- for i in $(seq 0 $(($dev_num - 1))); do
- tst_res TINFO "mount /dev/zram$i"
- mkdir zram$i
- ROD mount /dev/zram$i zram$i
- dev_mounted=$i
- done
-
- tst_res TPASS "mount of zram device(s) succeeded"
-}
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
` (3 preceding siblings ...)
2021-01-29 19:41 ` [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-01-29 20:17 ` Petr Vorel
2021-03-01 15:06 ` Cyril Hrubis
2021-01-29 19:41 ` [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices Petr Vorel
5 siblings, 2 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
Postpone starting timer a bit after the execution allow to setup
timer in test setup. This is useful if test length depends on input
decided during setup.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2.
Hi,
I'm ok, if you consider this risky.
Next commit can be then changed to have TST_TIMEOUT=-1.
Kind regards,
Petr
doc/test-writing-guidelines.txt | 5 ++++-
testcases/lib/tst_test.sh | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index f3a55cf26..61dccd400 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2398,7 +2398,10 @@ simply by setting right '$TST_NEEDS_FOO'.
| 'TST_TIMEOUT' | Maximum timeout set for the test in sec. Must be int >= 1,
or -1 (special value to disable timeout), default is 300.
Variable is meant be set in tests, not by user.
- It's equivalent of `tst_test.timeout` in C.
+ It's equivalent of `tst_test.timeout` in C, but (unlike
+ the C API) it can be set also in the setup function.
+ This is useful if test length depends on input decided
+ during setup.
|=============================================================================
NOTE: Network tests (see testcases/network/README.md) use additional variables
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 69f007d89..4fa1674cd 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -552,8 +552,6 @@ tst_run()
tst_brk TCONF "test requires kernel $TST_MIN_KVER+"
fi
- _tst_setup_timer
-
[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
@@ -594,6 +592,8 @@ tst_run()
fi
fi
+ _tst_setup_timer
+
#TODO check that test reports some results for each test function call
while [ $TST_ITERATIONS -gt 0 ]; do
if [ -n "$TST_TEST_DATA" ]; then
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
` (4 preceding siblings ...)
2021-01-29 19:41 ` [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP Petr Vorel
@ 2021-01-29 19:41 ` Petr Vorel
2021-01-29 20:10 ` Petr Vorel
5 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 19:41 UTC (permalink / raw)
To: ltp
to avoid unexpected timeout, which occurred even on just 4 devices.
Default setup is 300, using 200 should be safe.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2.
testcases/kernel/device-drivers/zram/zram_lib.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index 26ed1846b..c24b83cfc 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -47,6 +47,8 @@ zram_load()
tst_brk TBROK "dev_num must be > 0"
fi
+ TST_TIMEOUT=$((dev_num*200))
+
tst_res TINFO "create '$dev_num' zram device(s)"
modprobe zram num_devices=$dev_num || \
--
2.30.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-29 19:41 ` [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices Petr Vorel
@ 2021-01-29 20:10 ` Petr Vorel
2021-01-29 20:15 ` Petr Vorel
2021-03-01 15:14 ` Cyril Hrubis
0 siblings, 2 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 20:10 UTC (permalink / raw)
To: ltp
Hi,
> + TST_TIMEOUT=$((dev_num*200))
Actually on heavy loaded machine this is not enough due BTRFS.
I can add something like dev_num*600 or even -1 (then previous commit would not
be needed, but IMHO still useful).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-29 20:10 ` Petr Vorel
@ 2021-01-29 20:15 ` Petr Vorel
2021-01-30 8:26 ` Li Wang
2021-03-01 15:14 ` Cyril Hrubis
1 sibling, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 20:15 UTC (permalink / raw)
To: ltp
> Hi,
> > + TST_TIMEOUT=$((dev_num*200))
> Actually on heavy loaded machine this is not enough due BTRFS.
> I can add something like dev_num*600 or even -1 (then previous commit would not
> be needed, but IMHO still useful).
And bad thing is that it breaks other zram tests, because the timer probably
does not allow to run the cleanup:
_tst_setup_timer()
{
...
sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid &
I'm not sure if shell allow us to do it better. Maybe sent different signal than
SIGKILL and define 'trap _tst_do_exit' for that signal?
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP
2021-01-29 19:41 ` [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP Petr Vorel
@ 2021-01-29 20:17 ` Petr Vorel
2021-03-01 15:06 ` Cyril Hrubis
1 sibling, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 20:17 UTC (permalink / raw)
To: ltp
Hi,
> I'm ok, if you consider this risky.
If taken, I'll write a test for it.
Kind regards,
Petr
> Next commit can be then changed to have TST_TIMEOUT=-1.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
@ 2021-01-29 20:17 ` Petr Vorel
2021-01-30 5:37 ` Li Wang
2021-03-01 14:34 ` Cyril Hrubis
2 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-01-29 20:17 UTC (permalink / raw)
To: ltp
Hi,
...
> -# List of parameters for zram devices.
> -# For each number the test creates own zram device.
> -zram_max_streams="2 3 5 8"
Not sure about the meaning of zram_max_streams.
It looks like sequence of i+3, starting from 0.
Maybe instead of having variable behaving like array,
zram_max_streams() loop could be based on $dev_num.
But that's just cosmetic.
...
> +generate_vars()
> +{
> + local fs limit size stream=-1
> + dev_num=0
> +
> + for fs in $(tst_supported_fs | grep -v -e fat -e ntfs -e fuse); do
> + size="26214400"
> + limit="25M"
> + if [ "$fs" = "btrfs" ]; then
> + get_btrfs_size || continue
> + size="402653184"
> + limit="$((size/1024/1024))M"
> + fi
> +
> + stream=$((stream+3))
> + dev_num=$((dev_num+1))
> + zram_filesystems="$zram_filesystems $fs"
> + zram_mem_limits="$zram_mem_limits $limit"
> + zram_sizes="$zram_sizes $size"
> + zram_max_streams="$zram_max_streams $stream"
...
> +++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
> @@ -36,10 +36,12 @@ zram_load()
> {
> local tmp
> - dev_num=0
> - for tmp in $zram_max_streams; do
> - dev_num=$((dev_num+1))
> - done
> + if [ -z "$dev_num" ]; then
> + dev_num=0
> + for tmp in $zram_max_streams; do
> + dev_num=$((dev_num+1))
> + done
> + fi
> if [ $dev_num -le 0 ]; then
> tst_brk TBROK "dev_num must be > 0"
> @@ -129,6 +131,7 @@ zram_set_disksizes()
> i=$(($i + 1))
> tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
> + [ $i -eq $dev_num ] && break
This check is not needed any more.
> done
> tst_res TPASS "test succeeded"
> @@ -155,6 +158,7 @@ zram_set_memlimit()
> i=$(($i + 1))
> tst_res TINFO "$sys_path = '$ds' ($i/$dev_num)"
> + [ $i -eq $dev_num ] && break
And here.
> done
> tst_res TPASS "test succeeded"
> @@ -191,13 +195,10 @@ zram_swapoff()
> zram_makefs()
> {
> - tst_require_cmds mkfs
> local i=0
> + local fs
> for fs in $zram_filesystems; do
> - # if requested fs not supported default it to ext2
> - tst_supported_fs $fs 2> /dev/null || fs=ext2
> -
> tst_res TINFO "make $fs filesystem on /dev/zram$i"
> mkfs.$fs /dev/zram$i > err.log 2>&1
> if [ $? -ne 0 ]; then
> @@ -206,6 +207,7 @@ zram_makefs()
> fi
> i=$(($i + 1))
> + [ $i -eq $dev_num ] && break
And here
> done
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
2021-01-29 20:17 ` Petr Vorel
@ 2021-01-30 5:37 ` Li Wang
2021-02-02 7:17 ` Petr Vorel
2021-03-01 14:34 ` Cyril Hrubis
2 siblings, 1 reply; 31+ messages in thread
From: Li Wang @ 2021-01-30 5:37 UTC (permalink / raw)
To: ltp
Hi Petr,
Petr Vorel <pvorel@suse.cz> wrote:
> ...
>
> diff --git a/testcases/kernel/device-drivers/zram/zram01.sh
> b/testcases/kernel/device-drivers/zram/zram01.sh
> index a795ff89f..c5d4a3e51 100755
> --- a/testcases/kernel/device-drivers/zram/zram01.sh
> +++ b/testcases/kernel/device-drivers/zram/zram01.sh
> @@ -8,23 +8,25 @@
>
> TST_CNT=7
> TST_TESTFUNC="do_test"
> -TST_NEEDS_CMDS="awk bc dd"
> +TST_NEEDS_CMDS="awk bc dd grep"
> . zram_lib.sh
> +TST_SETUP="setup"
>
> -# List of parameters for zram devices.
> -# For each number the test creates own zram device.
> -zram_max_streams="2 3 5 8"
> -
> -FS_SIZE="402653184"
> -FS_TYPE="btrfs"
> +get_btrfs_size()
>
What about renaming at_least_1G_mem() or check_space_for_btrfs()?
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210130/cf12bffe/attachment.htm>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-29 20:15 ` Petr Vorel
@ 2021-01-30 8:26 ` Li Wang
2021-02-02 7:38 ` Petr Vorel
0 siblings, 1 reply; 31+ messages in thread
From: Li Wang @ 2021-01-30 8:26 UTC (permalink / raw)
To: ltp
On Sat, Jan 30, 2021 at 4:16 AM Petr Vorel <pvorel@suse.cz> wrote:
> > Hi,
>
> > > + TST_TIMEOUT=$((dev_num*200))
> > Actually on heavy loaded machine this is not enough due BTRFS.
> > I can add something like dev_num*600 or even -1 (then previous commit
> would not
> > be needed, but IMHO still useful).
>
I personally think -1 is better.
> And bad thing is that it breaks other zram tests, because the timer
> probably
> does not allow to run the cleanup:
>
> _tst_setup_timer()
> {
> ...
> sleep $sec && tst_res TBROK "test killed, timeout! If you are
> running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9
> -$pid &
>
> I'm not sure if shell allow us to do it better. Maybe sent different
> signal than
> SIGKILL and define 'trap _tst_do_exit' for that signal?
Sounds practicable. I guess sending SIGINT could make more sense, since
sometimes we use CTRL+C stop test in debugging by manual, test should
do cleanup work for that behavior too.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210130/5616a62d/attachment-0001.htm>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-01-30 5:37 ` Li Wang
@ 2021-02-02 7:17 ` Petr Vorel
2021-02-02 11:16 ` Li Wang
0 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-02-02 7:17 UTC (permalink / raw)
To: ltp
Hi Li,
> Hi Petr,
> Petr Vorel <pvorel@suse.cz> wrote:
> > ...
> > diff --git a/testcases/kernel/device-drivers/zram/zram01.sh
> > b/testcases/kernel/device-drivers/zram/zram01.sh
> > index a795ff89f..c5d4a3e51 100755
> > --- a/testcases/kernel/device-drivers/zram/zram01.sh
> > +++ b/testcases/kernel/device-drivers/zram/zram01.sh
> > @@ -8,23 +8,25 @@
> > TST_CNT=7
> > TST_TESTFUNC="do_test"
> > -TST_NEEDS_CMDS="awk bc dd"
> > +TST_NEEDS_CMDS="awk bc dd grep"
> > . zram_lib.sh
> > +TST_SETUP="setup"
> > -# List of parameters for zram devices.
> > -# For each number the test creates own zram device.
> > -zram_max_streams="2 3 5 8"
> > -
> > -FS_SIZE="402653184"
> > -FS_TYPE="btrfs"
> > +get_btrfs_size()
> What about renaming at_least_1G_mem() or check_space_for_btrfs()?
Good point. I'm slightly for check_space_for_btrfs().
at_least_1G_mem() is also good, but for that I'd also move tst_res TINFO "not
enough space for Btrfs" out of the function and put it into generate_vars(). But
since it's used only for btrfs I slightly prefer check_space_for_btrfs(). But no
strong opinion about it.
Thanks for your review!
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-30 8:26 ` Li Wang
@ 2021-02-02 7:38 ` Petr Vorel
2021-02-02 11:25 ` Li Wang
0 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-02-02 7:38 UTC (permalink / raw)
To: ltp
Hi Li,
> On Sat, Jan 30, 2021 at 4:16 AM Petr Vorel <pvorel@suse.cz> wrote:
> > > Hi,
> > > > + TST_TIMEOUT=$((dev_num*200))
> > > Actually on heavy loaded machine this is not enough due BTRFS.
> > > I can add something like dev_num*600 or even -1 (then previous commit
> > would not
> > > be needed, but IMHO still useful).
> I personally think -1 is better.
OK. In that case we might avoid now unneeded previous commit.
> > And bad thing is that it breaks other zram tests, because the timer
> > probably
> > does not allow to run the cleanup:
> > _tst_setup_timer()
> > {
> > ...
> > sleep $sec && tst_res TBROK "test killed, timeout! If you are
> > running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9
> > -$pid &
> > I'm not sure if shell allow us to do it better. Maybe sent different
> > signal than
> > SIGKILL and define 'trap _tst_do_exit' for that signal?
> Sounds practicable. I guess sending SIGINT could make more sense, since
> sometimes we use CTRL+C stop test in debugging by manual, test should
> do cleanup work for that behavior too.
We have already SIGINT defined for main shell process:
trap "tst_brk TBROK 'test interrupted'" INT
so CTRL+C is covered. So maybe run first SIGINT and then SIGKILL for safety
reasons?
Kind regards,
Petr
diff --git testcases/lib/tst_test.sh testcases/lib/tst_test.sh
index 4fa1674cd..61a6fbcdd 100644
--- testcases/lib/tst_test.sh
+++ testcases/lib/tst_test.sh
@@ -459,7 +459,7 @@ _tst_setup_timer()
tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s"
- sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid &
+ sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && { kill -2 -$pid; sleep 5; kill -9 -$pid; } &
_tst_setup_timer_pid=$!
}
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-02-02 7:17 ` Petr Vorel
@ 2021-02-02 11:16 ` Li Wang
2021-02-06 19:59 ` Petr Vorel
0 siblings, 1 reply; 31+ messages in thread
From: Li Wang @ 2021-02-02 11:16 UTC (permalink / raw)
To: ltp
Hi Petr,
Petr Vorel <pvorel@suse.cz> wrote:
...
> > > +get_btrfs_size()
>
>
> > What about renaming at_least_1G_mem() or check_space_for_btrfs()?
> Good point. I'm slightly for check_space_for_btrfs().
>
> at_least_1G_mem() is also good, but for that I'd also move tst_res TINFO
> "not
> enough space for Btrfs" out of the function and put it into
> generate_vars(). But
> since it's used only for btrfs I slightly prefer check_space_for_btrfs().
> But no
> strong opinion about it.
>
Agree, thanks!
Btw I suddenly think that we could have a nicer name initialize_vars()
to replace generate_vars(), because we just use it once to initiate the
test variables in the setup phase.
Anyway, it's only my feelings and also depends on your preference too.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210202/64014556/attachment.htm>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-02-02 7:38 ` Petr Vorel
@ 2021-02-02 11:25 ` Li Wang
0 siblings, 0 replies; 31+ messages in thread
From: Li Wang @ 2021-02-02 11:25 UTC (permalink / raw)
To: ltp
Hi Petr,
On Tue, Feb 2, 2021 at 3:38 PM Petr Vorel <pvorel@suse.cz> wrote:
> Hi Li,
>
> > On Sat, Jan 30, 2021 at 4:16 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > > Hi,
>
> > > > > + TST_TIMEOUT=$((dev_num*200))
> > > > Actually on heavy loaded machine this is not enough due BTRFS.
> > > > I can add something like dev_num*600 or even -1 (then previous commit
> > > would not
> > > > be needed, but IMHO still useful).
>
>
> > I personally think -1 is better.
> OK. In that case we might avoid now unneeded previous commit.
>
>
>
> > > And bad thing is that it breaks other zram tests, because the timer
> > > probably
> > > does not allow to run the cleanup:
>
> > > _tst_setup_timer()
> > > {
> > > ...
> > > sleep $sec && tst_res TBROK "test killed, timeout! If you are
> > > running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9
> > > -$pid &
>
> > > I'm not sure if shell allow us to do it better. Maybe sent different
> > > signal than
> > > SIGKILL and define 'trap _tst_do_exit' for that signal?
>
>
> > Sounds practicable. I guess sending SIGINT could make more sense, since
> > sometimes we use CTRL+C stop test in debugging by manual, test should
> > do cleanup work for that behavior too.
> We have already SIGINT defined for main shell process:
> trap "tst_brk TBROK 'test interrupted'" INT
>
> so CTRL+C is covered. So maybe run first SIGINT and then SIGKILL for safety
> reasons?
>
I'm fine with using SIGINT + SIGKILL.
Or, maybe another choice is to catch a signal SIGTERM? Once we
cancel the specified signal in KILL command, it will try with SIGTERM
by default, if the test process can not be terminated, then a SIGKILL
may be sent.
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -22,6 +22,7 @@ export TST_LIB_LOADED=1
# default trap function
trap "tst_brk TBROK 'test interrupted'" INT
+trap "_tst_do_exit" TERM
_tst_do_exit()
{
@@ -459,7 +460,7 @@ _tst_setup_timer()
tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s"
- sleep $sec && tst_res TBROK "test killed, timeout! If you are
running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9
-$pid &
+ sleep $sec && tst_res TBROK "test terminated, timeout! If you are
running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill --
-$pid &
_tst_setup_timer_pid=$!
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210202/04007113/attachment.htm>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-02-02 11:16 ` Li Wang
@ 2021-02-06 19:59 ` Petr Vorel
2021-02-08 7:11 ` Petr Vorel
0 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-02-06 19:59 UTC (permalink / raw)
To: ltp
Hi Li,
...
> > > > +get_btrfs_size()
> > > What about renaming at_least_1G_mem() or check_space_for_btrfs()?
> > Good point. I'm slightly for check_space_for_btrfs().
> > at_least_1G_mem() is also good, but for that I'd also move tst_res TINFO
> > "not
> > enough space for Btrfs" out of the function and put it into
> > generate_vars(). But
> > since it's used only for btrfs I slightly prefer check_space_for_btrfs().
> > But no
> > strong opinion about it.
> Agree, thanks!
> Btw I suddenly think that we could have a nicer name initialize_vars()
> to replace generate_vars(), because we just use it once to initiate the
> test variables in the setup phase.
Good idea, thanks!
BTW I tested zram on all filesystems including fuse/*fat/ntfs:
zram01 4 TINFO: make ext2 filesystem on /dev/zram0
zram01 4 TINFO: make ext3 filesystem on /dev/zram1
zram01 4 TINFO: make ext4 filesystem on /dev/zram2
zram01 4 TINFO: make xfs filesystem on /dev/zram3
zram01 4 TINFO: make btrfs filesystem on /dev/zram4
zram01 4 TINFO: make vfat filesystem on /dev/zram5
zram01 4 TINFO: make exfat filesystem on /dev/zram6
zram01 4 TINFO: make ntfs filesystem on /dev/zram7
and it's working well, thus I suggest to test everything available:
- for fs in $(tst_supported_fs | grep -v -e fat -e ntfs -e fuse); do
+ for fs in $(tst_supported_fs); do
Before sending v3 or just merge with fixes I'm also waiting for Cyril's (or
somebody else) review / suggestions on the timeout (whether simply use -1 for
timeout and drop patch "tst_test.sh: Run _tst_setup_timer after $TST_SETUP")
https://patchwork.ozlabs.org/project/ltp/patch/20210129194144.31299-6-pvorel@suse.cz/
https://patchwork.ozlabs.org/project/ltp/patch/20210129194144.31299-7-pvorel@suse.cz/
Kind regards,
Petr
> Anyway, it's only my feelings and also depends on your preference too.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-02-06 19:59 ` Petr Vorel
@ 2021-02-08 7:11 ` Petr Vorel
2021-02-18 9:00 ` Li Wang
0 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2021-02-08 7:11 UTC (permalink / raw)
To: ltp
Hi Li,
> BTW I tested zram on all filesystems including fuse/*fat/ntfs:
> zram01 4 TINFO: make ext2 filesystem on /dev/zram0
> zram01 4 TINFO: make ext3 filesystem on /dev/zram1
> zram01 4 TINFO: make ext4 filesystem on /dev/zram2
> zram01 4 TINFO: make xfs filesystem on /dev/zram3
> zram01 4 TINFO: make btrfs filesystem on /dev/zram4
> zram01 4 TINFO: make vfat filesystem on /dev/zram5
> zram01 4 TINFO: make exfat filesystem on /dev/zram6
> zram01 4 TINFO: make ntfs filesystem on /dev/zram7
> and it's working well, thus I suggest to test everything available:
> - for fs in $(tst_supported_fs | grep -v -e fat -e ntfs -e fuse); do
> + for fs in $(tst_supported_fs); do
Although running all 8 filesystems run takes more than 10 min. If it's too long,
it might be better to keep grep and also limit number of tested filesystems.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-02-08 7:11 ` Petr Vorel
@ 2021-02-18 9:00 ` Li Wang
0 siblings, 0 replies; 31+ messages in thread
From: Li Wang @ 2021-02-18 9:00 UTC (permalink / raw)
To: ltp
Hi Petr,
Sorry for the late reply, I was on the Spring Festival holidays last two
weeks.
On Mon, Feb 8, 2021 at 3:11 PM Petr Vorel <pvorel@suse.cz> wrote:
> Hi Li,
>
> > BTW I tested zram on all filesystems including fuse/*fat/ntfs:
> > zram01 4 TINFO: make ext2 filesystem on /dev/zram0
> > zram01 4 TINFO: make ext3 filesystem on /dev/zram1
> > zram01 4 TINFO: make ext4 filesystem on /dev/zram2
> > zram01 4 TINFO: make xfs filesystem on /dev/zram3
> > zram01 4 TINFO: make btrfs filesystem on /dev/zram4
> > zram01 4 TINFO: make vfat filesystem on /dev/zram5
> > zram01 4 TINFO: make exfat filesystem on /dev/zram6
> > zram01 4 TINFO: make ntfs filesystem on /dev/zram7
>
> > and it's working well, thus I suggest to test everything available:
>
> > - for fs in $(tst_supported_fs | grep -v -e fat -e ntfs -e fuse);
> do
> > + for fs in $(tst_supported_fs); do
>
> Although running all 8 filesystems run takes more than 10 min. If it's too
> long,
> it might be better to keep grep and also limit number of tested
> filesystems.
>
+1
I tend to keep grep there cause Linux users might rarely use fat/ntfs.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210218/79c358ac/attachment.htm>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
2021-01-29 20:17 ` Petr Vorel
2021-01-30 5:37 ` Li Wang
@ 2021-03-01 14:34 ` Cyril Hrubis
2021-03-01 16:04 ` Petr Vorel
2 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 14:34 UTC (permalink / raw)
To: ltp
Hi!
Looks good including the changes proposed in the thread.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh
2021-01-29 19:41 ` [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh Petr Vorel
@ 2021-03-01 14:55 ` Cyril Hrubis
2021-03-01 15:35 ` Petr Vorel
0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 14:55 UTC (permalink / raw)
To: ltp
Hi!
> Quit at setup in case there is no fs support (change in previous commit)
> can lead to skipping zram_compress_alg(). Move to zram02.sh where is no
> such limitation.
Actually I think that we should keep it in both for now, since the tests
do depend on each other, the zram_fill_fs actually checks compression
ration, which only makes sense if compression is enabled.
Ideally we should use different compression algorithms in the zram01
test, so I guess that we can, later on distribute the compression
algorithms between the created devices, but that shouldn't stop this
patchset.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh
2021-01-29 19:41 ` [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh Petr Vorel
@ 2021-03-01 14:57 ` Cyril Hrubis
0 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 14:57 UTC (permalink / raw)
To: ltp
Hi!
This is okay, but we should keep the zram_compress_alg() in the lib if
we are going to have the test step in both tests.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP
2021-01-29 19:41 ` [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP Petr Vorel
2021-01-29 20:17 ` Petr Vorel
@ 2021-03-01 15:06 ` Cyril Hrubis
1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 15:06 UTC (permalink / raw)
To: ltp
Hi!
We do have a function to set the timeout in the C library
(tst_set_timeout), maybe we should just add one to the shell just to
match the API.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-01-29 20:10 ` Petr Vorel
2021-01-29 20:15 ` Petr Vorel
@ 2021-03-01 15:14 ` Cyril Hrubis
2021-03-01 15:41 ` Petr Vorel
1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 15:14 UTC (permalink / raw)
To: ltp
Hi!
> > + TST_TIMEOUT=$((dev_num*200))
> Actually on heavy loaded machine this is not enough due BTRFS.
> I can add something like dev_num*600 or even -1 (then previous commit would not
> be needed, but IMHO still useful).
I would still prefer if we had a timeout there, -1 is for something that
cannot be predicted.
Also we do not expect machine to be heavily loaded, in that case half of
LTP tests would time out.
So I would just measure how long the test takes, then multiply it by 5
or something like that and put that in as a timeout.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh
2021-03-01 14:55 ` Cyril Hrubis
@ 2021-03-01 15:35 ` Petr Vorel
0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-03-01 15:35 UTC (permalink / raw)
To: ltp
Hi Cyril,
> Hi!
> > Quit at setup in case there is no fs support (change in previous commit)
> > can lead to skipping zram_compress_alg(). Move to zram02.sh where is no
> > such limitation.
> Actually I think that we should keep it in both for now, since the tests
> do depend on each other, the zram_fill_fs actually checks compression
> ration, which only makes sense if compression is enabled.
Makes sense.
> Ideally we should use different compression algorithms in the zram01
> test, so I guess that we can, later on distribute the compression
> algorithms between the created devices, but that shouldn't stop this
> patchset.
OK. I'll change this patchset to *add* zram_compress_alg() to zram02.sh (keep it
in zram01.sh) and update following commit.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-03-01 15:14 ` Cyril Hrubis
@ 2021-03-01 15:41 ` Petr Vorel
2021-03-01 15:45 ` Petr Vorel
2021-03-01 15:48 ` Cyril Hrubis
0 siblings, 2 replies; 31+ messages in thread
From: Petr Vorel @ 2021-03-01 15:41 UTC (permalink / raw)
To: ltp
> Hi!
> > > + TST_TIMEOUT=$((dev_num*200))
> > Actually on heavy loaded machine this is not enough due BTRFS.
> > I can add something like dev_num*600 or even -1 (then previous commit would not
> > be needed, but IMHO still useful).
> I would still prefer if we had a timeout there, -1 is for something that
> cannot be predicted.
> Also we do not expect machine to be heavily loaded, in that case half of
> LTP tests would time out.
> So I would just measure how long the test takes, then multiply it by 5
> or something like that and put that in as a timeout.
Do you mean to use high enough static timeout defined before startup (working
for maximum possible filesystems)? And create tst_set_timeout() for shell as
independent effort?
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-03-01 15:41 ` Petr Vorel
@ 2021-03-01 15:45 ` Petr Vorel
2021-03-01 15:48 ` Cyril Hrubis
1 sibling, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-03-01 15:45 UTC (permalink / raw)
To: ltp
> > Hi!
> > > > + TST_TIMEOUT=$((dev_num*200))
> > > Actually on heavy loaded machine this is not enough due BTRFS.
> > > I can add something like dev_num*600 or even -1 (then previous commit would not
> > > be needed, but IMHO still useful).
> > I would still prefer if we had a timeout there, -1 is for something that
> > cannot be predicted.
> > Also we do not expect machine to be heavily loaded, in that case half of
> > LTP tests would time out.
> > So I would just measure how long the test takes, then multiply it by 5
> > or something like that and put that in as a timeout.
> Do you mean to use high enough static timeout defined before startup (working
> for maximum possible filesystems)? And create tst_set_timeout() for shell as
> independent effort?
BTW looking at the docs for C API tst_set_timeout() is expected to be run in the
setup function. Thus this patchset can be reused (just extended).
Kind regards,
Petr
> Kind regards,
> Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-03-01 15:41 ` Petr Vorel
2021-03-01 15:45 ` Petr Vorel
@ 2021-03-01 15:48 ` Cyril Hrubis
2021-03-01 16:18 ` Petr Vorel
1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2021-03-01 15:48 UTC (permalink / raw)
To: ltp
Hi!
> > I would still prefer if we had a timeout there, -1 is for something that
> > cannot be predicted.
>
> > Also we do not expect machine to be heavily loaded, in that case half of
> > LTP tests would time out.
>
> > So I would just measure how long the test takes, then multiply it by 5
> > or something like that and put that in as a timeout.
> Do you mean to use high enough static timeout defined before startup (working
> for maximum possible filesystems)? And create tst_set_timeout() for shell as
> independent effort?
I would do:
* Add tst_set_timeout for shell, so that it mirrors the C API
* Measure runtime of the test divide it by the number of supported
filesystems, that way we would get mean runtime per filesystem
now I would multiply this number with arbitrary constantm, e.g. 5 or
even more, this is now timeout per iteration
with this number the actuall timeout would be:
number_of_filesystems * mean_max_per_run
Does this sound sane?
I guess that in the end we will end up with something similar what you
had there, but we would have some data that supports this decision.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup
2021-03-01 14:34 ` Cyril Hrubis
@ 2021-03-01 16:04 ` Petr Vorel
0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-03-01 16:04 UTC (permalink / raw)
To: ltp
Hi Cyril,
> Hi!
> Looks good including the changes proposed in the thread.
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Thanks! I suppose this include keep "grep -v -e fat -e ntfs -e fuse"
to avoid fat/ntfs testing (I'd personally use it, because we don't whitelist
testing them for other tests, e.g. fanotify).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
* [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices
2021-03-01 15:48 ` Cyril Hrubis
@ 2021-03-01 16:18 ` Petr Vorel
0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2021-03-01 16:18 UTC (permalink / raw)
To: ltp
> Hi!
> > > I would still prefer if we had a timeout there, -1 is for something that
> > > cannot be predicted.
> > > Also we do not expect machine to be heavily loaded, in that case half of
> > > LTP tests would time out.
> > > So I would just measure how long the test takes, then multiply it by 5
> > > or something like that and put that in as a timeout.
> > Do you mean to use high enough static timeout defined before startup (working
> > for maximum possible filesystems)? And create tst_set_timeout() for shell as
> > independent effort?
> I would do:
> * Add tst_set_timeout for shell, so that it mirrors the C API
+1. BTW C has .all_filesystems for timeout for each run, which allows not to
bother with timeout depending on number of filesystems (unlike fuzzy sync, which
also sometimes needs tweak fzsync_pair.exec_time_p). I'm for ignoring this fact,
just to let know that shell API is far behind C API.
> * Measure runtime of the test divide it by the number of supported
> filesystems, that way we would get mean runtime per filesystem
> now I would multiply this number with arbitrary constantm, e.g. 5 or
> even more, this is now timeout per iteration
> with this number the actuall timeout would be:
> number_of_filesystems * mean_max_per_run
> Does this sound sane?
+1, thanks!
> I guess that in the end we will end up with something similar what you
> had there, but we would have some data that supports this decision.
+1
Kind regards,
Petr
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-03-01 16:18 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29 19:41 [LTP] [PATCH v2 0/6] zram cleanup Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 1/6] zram: Calculate dev_num variable Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 2/6] zram01.sh: Generate test setup variables in setup Petr Vorel
2021-01-29 20:17 ` Petr Vorel
2021-01-30 5:37 ` Li Wang
2021-02-02 7:17 ` Petr Vorel
2021-02-02 11:16 ` Li Wang
2021-02-06 19:59 ` Petr Vorel
2021-02-08 7:11 ` Petr Vorel
2021-02-18 9:00 ` Li Wang
2021-03-01 14:34 ` Cyril Hrubis
2021-03-01 16:04 ` Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 3/6] zram: Move zram_compress_alg() to zram02.sh Petr Vorel
2021-03-01 14:55 ` Cyril Hrubis
2021-03-01 15:35 ` Petr Vorel
2021-01-29 19:41 ` [LTP] [PATCH v2 4/6] zram: Move test specific functions out of zram_lib.sh Petr Vorel
2021-03-01 14:57 ` Cyril Hrubis
2021-01-29 19:41 ` [LTP] [RFC PATCH v2 5/6] tst_test.sh: Run _tst_setup_timer after $TST_SETUP Petr Vorel
2021-01-29 20:17 ` Petr Vorel
2021-03-01 15:06 ` Cyril Hrubis
2021-01-29 19:41 ` [LTP] [PATCH v2 6/6] zram: Increase timeout according to used devices Petr Vorel
2021-01-29 20:10 ` Petr Vorel
2021-01-29 20:15 ` Petr Vorel
2021-01-30 8:26 ` Li Wang
2021-02-02 7:38 ` Petr Vorel
2021-02-02 11:25 ` Li Wang
2021-03-01 15:14 ` Cyril Hrubis
2021-03-01 15:41 ` Petr Vorel
2021-03-01 15:45 ` Petr Vorel
2021-03-01 15:48 ` Cyril Hrubis
2021-03-01 16:18 ` Petr Vorel
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.