2021-12-15 09:56:44

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram has
switched to per-cpu streams. Even kernel still keep this interface for some reasons, but
writing to max_comp_stream doesn't take any effect. So remove it.

Signed-off-by: Yang Xu <[email protected]>
---
tools/testing/selftests/zram/zram01.sh | 4 ----
tools/testing/selftests/zram/zram02.sh | 4 ----
tools/testing/selftests/zram/zram_lib.sh | 22 ----------------------
3 files changed, 30 deletions(-)

diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index 114863d9fb87..28583e4ae546 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -15,9 +15,6 @@ ERR_CODE=0

# 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.
-zram_max_streams="2"

# The zram sysfs node 'disksize' value can be either in bytes,
# or you can use mem suffixes. But in some old kernels, mem
@@ -72,7 +69,6 @@ zram_fill_fs()

check_prereqs
zram_load
-zram_max_streams
zram_compress_alg
zram_set_disksizes
zram_set_memlimit
diff --git a/tools/testing/selftests/zram/zram02.sh b/tools/testing/selftests/zram/zram02.sh
index e83b404807c0..d664974a1317 100755
--- a/tools/testing/selftests/zram/zram02.sh
+++ b/tools/testing/selftests/zram/zram02.sh
@@ -14,9 +14,6 @@ ERR_CODE=0

# 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.
-zram_max_streams="2"

# The zram sysfs node 'disksize' value can be either in bytes,
# or you can use mem suffixes. But in some old kernels, mem
@@ -30,7 +27,6 @@ zram_mem_limits="1M"

check_prereqs
zram_load
-zram_max_streams
zram_set_disksizes
zram_set_memlimit
zram_makeswap
diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
index 6f872f266fd1..0c49f9d1d563 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -82,28 +82,6 @@ zram_load()
fi
}

-zram_max_streams()
-{
- echo "set max_comp_streams to zram device(s)"
-
- local i=0
- for max_s in $zram_max_streams; do
- local sys_path="/sys/block/zram${i}/max_comp_streams"
- echo $max_s > $sys_path || \
- echo "FAIL failed to set '$max_s' to $sys_path"
- sleep 1
- local max_streams=$(cat $sys_path)
-
- [ "$max_s" -ne "$max_streams" ] && \
- echo "FAIL can't set max_streams '$max_s', get $max_stream"
-
- i=$(($i + 1))
- echo "$sys_path = '$max_streams' ($i/$dev_num)"
- done
-
- echo "zram max streams: OK"
-}
-
zram_compress_alg()
{
echo "test that we can set compression algorithm"
--
2.23.0



2021-12-15 09:56:45

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: [PATCH 2/3] selftests/zram01.sh: Fix compression ratio calculation

zram01 uses `free -m` to measure zram memory usage. The results are nonsense
because they are polluted by all running processes on the system.

Use the third field of /sys/block/zram<id>/mm_stat to measure memory usage instead. The file is
available since kernel 4.1.

orig_data_size(first): uncompressed size of data stored in this disk.
compr_data_size(second): compressed size of data stored in this disk
mem_used_total(third): the amount of memory allocated for this disk

Also remove useless zram cleanup call in zram_fill_fs and so we don't need to cleanup
zram twice if fails.

Signed-off-by: Yang Xu <[email protected]>
---
tools/testing/selftests/zram/zram01.sh | 30 +++++++-------------------
1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index 28583e4ae546..e083a4c628a8 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -30,8 +30,6 @@ zram_algs="lzo"

zram_fill_fs()
{
- local mem_free0=$(free -m | awk 'NR==2 {print $4}')
-
for i in $(seq 0 $(($dev_num - 1))); do
echo "fill zram$i..."
local b=0
@@ -42,29 +40,17 @@ zram_fill_fs()
b=$(($b + 1))
done
echo "zram$i can be filled with '$b' KB"
- done

- local mem_free1=$(free -m | awk 'NR==2 {print $4}')
- local used_mem=$(($mem_free0 - $mem_free1))
+ local mem_used_total=`awk '{print $3}' "/sys/block/zram$i/mm_stat"`
+ local v=$((100 * 1024 * $b / $mem_used_total))
+ if [ "$v" -lt 100 ]; then
+ echo "FAIL compression ratio: 0.$v:1"
+ ERR_CODE=-1
+ return
+ fi

- local total_size=0
- for sm in $zram_sizes; do
- local s=$(echo $sm | sed 's/M//')
- total_size=$(($total_size + $s))
+ echo "zram compression ratio: $(echo "scale=2; $v / 100 " | bc):1: OK"
done
-
- echo "zram used ${used_mem}M, zram disk sizes ${total_size}M"
-
- local v=$((100 * $total_size / $used_mem))
-
- if [ "$v" -lt 100 ]; then
- echo "FAIL compression ratio: 0.$v:1"
- ERR_CODE=-1
- zram_cleanup
- return
- fi
-
- echo "zram compression ratio: $(echo "scale=2; $v / 100 " | bc):1: OK"
}

check_prereqs
--
2.23.0


2021-12-15 09:56:46

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: [PATCH 3/3] selftests/zram: Adapt the situation that /dev/zram0 is being used

On some linux distributions, if it used /dev/zram0 as default by zram-generate
service, then this case will fail or report EBUSY.

To fix this, use hot_add/hot_remove interface. Also, move module check code into
zram_load from zram.sh. We can use /sys/class/zram-control to detect the
module whether existed.

After this patch, we still run case pass even /dev/zram0 is being used.

Signed-off-by: Yang Xu <[email protected]>
---
tools/testing/selftests/zram/zram.sh | 16 +----
tools/testing/selftests/zram/zram01.sh | 2 +-
tools/testing/selftests/zram/zram_lib.sh | 75 ++++++++++++------------
3 files changed, 41 insertions(+), 52 deletions(-)

diff --git a/tools/testing/selftests/zram/zram.sh b/tools/testing/selftests/zram/zram.sh
index 232e958ec454..d002171fe86f 100755
--- a/tools/testing/selftests/zram/zram.sh
+++ b/tools/testing/selftests/zram/zram.sh
@@ -2,9 +2,6 @@
# SPDX-License-Identifier: GPL-2.0
TCID="zram.sh"

-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
-
. ./zram_lib.sh

run_zram () {
@@ -17,15 +14,4 @@ echo ""
}

check_prereqs
-
-# check zram module exists
-MODULE_PATH=/lib/modules/`uname -r`/kernel/drivers/block/zram/zram.ko
-if [ -f $MODULE_PATH ]; then
- run_zram
-elif [ -b /dev/zram0 ]; then
- run_zram
-else
- echo "$TCID : No zram.ko module or /dev/zram0 device file not found"
- echo "$TCID : CONFIG_ZRAM is not set"
- exit $ksft_skip
-fi
+run_zram
diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index e083a4c628a8..f87cc1ebf20f 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -30,7 +30,7 @@ zram_algs="lzo"

zram_fill_fs()
{
- for i in $(seq 0 $(($dev_num - 1))); do
+ for i in $(seq $dev_start $dev_end); do
echo "fill zram$i..."
local b=0
while [ true ]; do
diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
index 0c49f9d1d563..ee57990c322d 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -8,6 +8,8 @@
MODULE=0
dev_makeswap=-1
dev_mounted=-1
+dev_start=-1
+dev_end=-1

# Kselftest framework requirement - SKIP code is 4.
ksft_skip=4
@@ -29,19 +31,22 @@ zram_cleanup()
{
echo "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
- rm -rf zram$i
+ rm -rf zram${i}
done

+ for i in $(seq $dev_start $dev_end); do
+ echo $i > /sys/class/zram-control/hot_remove
+ done
}

zram_unload()
@@ -54,32 +59,30 @@ zram_unload()

zram_load()
{
- # check zram module exists
- MODULE_PATH=/lib/modules/`uname -r`/kernel/drivers/block/zram/zram.ko
- if [ -f $MODULE_PATH ]; then
- MODULE=1
- echo "create '$dev_num' zram device(s)"
+ echo "create '$dev_num' zram device(s)"
+
+ if [ ! -d "/sys/class/zram-control" ]; then
modprobe zram num_devices=$dev_num
- if [ $? -ne 0 ]; then
- echo "failed to insert zram module"
- exit 1
+ if grep -q '^zram' /proc/modules; then
+ echo "ERROR: No zram.ko module"
+ echo "$TCID : CONFIG_ZRAM is not set"
+ exit $ksft_skip
fi
+ MODULE=1
+ dev_start=0
+ dev_end=$(($dev_num - 1))
+ echo "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
- echo "unexpected num of devices: $dev_num_created"
- ERR_CODE=-1
- else
- echo "zram load module successful"
- fi
- elif [ -b /dev/zram0 ]; then
- echo "/dev/zram0 device file found: OK"
- else
- echo "ERROR: No zram.ko module or no /dev/zram0 device found"
- echo "$TCID : CONFIG_ZRAM is not set"
- exit 1
- fi
+ for i in $(seq $dev_start $dev_end); do
+ cat /sys/class/zram-control/hot_add > /dev/null
+ done
+
+ echo "all zram devices(/dev/zram$dev_start~$dev_end) successfully created"
}

zram_compress_alg()
@@ -88,13 +91,13 @@ zram_compress_alg()

local algs=$(cat /sys/block/zram0/comp_algorithm)
echo "supported algs: $algs"
- local i=0
+ local i=$dev_start
for alg in $zram_algs; do
local sys_path="/sys/block/zram${i}/comp_algorithm"
echo "$alg" > $sys_path || \
echo "FAIL can't set '$alg' to $sys_path"
i=$(($i + 1))
- echo "$sys_path = '$alg' ($i/$dev_num)"
+ echo "$sys_path = '$alg'"
done

echo "zram set compression algorithm: OK"
@@ -103,14 +106,14 @@ zram_compress_alg()
zram_set_disksizes()
{
echo "set disk size to zram device(s)"
- local i=0
+ local i=$dev_start
for ds in $zram_sizes; do
local sys_path="/sys/block/zram${i}/disksize"
echo "$ds" > $sys_path || \
echo "FAIL can't set '$ds' to $sys_path"

i=$(($i + 1))
- echo "$sys_path = '$ds' ($i/$dev_num)"
+ echo "$sys_path = '$ds'"
done

echo "zram set disksizes: OK"
@@ -120,14 +123,14 @@ zram_set_memlimit()
{
echo "set memory limit to zram device(s)"

- local i=0
+ local i=$dev_start
for ds in $zram_mem_limits; do
local sys_path="/sys/block/zram${i}/mem_limit"
echo "$ds" > $sys_path || \
echo "FAIL can't set '$ds' to $sys_path"

i=$(($i + 1))
- echo "$sys_path = '$ds' ($i/$dev_num)"
+ echo "$sys_path = '$ds'"
done

echo "zram set memory limit: OK"
@@ -137,7 +140,7 @@ zram_makeswap()
{
echo "make swap with zram device(s)"
local i=0
- for i in $(seq 0 $(($dev_num - 1))); do
+ for i in $(seq $dev_start $dev_end); do
mkswap /dev/zram$i > err.log 2>&1
if [ $? -ne 0 ]; then
cat err.log
@@ -160,7 +163,7 @@ zram_makeswap()
zram_swapoff()
{
local i=
- for i in $(seq 0 $dev_makeswap); do
+ for i in $(seq $dev_start $dev_makeswap); do
swapoff /dev/zram$i > err.log 2>&1
if [ $? -ne 0 ]; then
cat err.log
@@ -174,7 +177,7 @@ zram_swapoff()

zram_makefs()
{
- local i=0
+ local i=$dev_start
for fs in $zram_filesystems; do
# if requested fs not supported default it to ext2
which mkfs.$fs > /dev/null 2>&1 || fs=ext2
@@ -193,7 +196,7 @@ zram_makefs()
zram_mount()
{
local i=0
- for i in $(seq 0 $(($dev_num - 1))); do
+ for i in $(seq $dev_start $dev_end); do
echo "mount /dev/zram$i"
mkdir zram$i
mount /dev/zram$i zram$i > /dev/null || \
--
2.23.0


2022-01-13 06:26:34

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

Hi Ping

Best Regards
Yang Xu
> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram has
> switched to per-cpu streams. Even kernel still keep this interface for some reasons, but
> writing to max_comp_stream doesn't take any effect. So remove it.
>
> Signed-off-by: Yang Xu<[email protected]>
> ---
> tools/testing/selftests/zram/zram01.sh | 4 ----
> tools/testing/selftests/zram/zram02.sh | 4 ----
> tools/testing/selftests/zram/zram_lib.sh | 22 ----------------------
> 3 files changed, 30 deletions(-)
>
> diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
> index 114863d9fb87..28583e4ae546 100755
> --- a/tools/testing/selftests/zram/zram01.sh
> +++ b/tools/testing/selftests/zram/zram01.sh
> @@ -15,9 +15,6 @@ ERR_CODE=0
>
> # 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.
> -zram_max_streams="2"
>
> # The zram sysfs node 'disksize' value can be either in bytes,
> # or you can use mem suffixes. But in some old kernels, mem
> @@ -72,7 +69,6 @@ zram_fill_fs()
>
> check_prereqs
> zram_load
> -zram_max_streams
> zram_compress_alg
> zram_set_disksizes
> zram_set_memlimit
> diff --git a/tools/testing/selftests/zram/zram02.sh b/tools/testing/selftests/zram/zram02.sh
> index e83b404807c0..d664974a1317 100755
> --- a/tools/testing/selftests/zram/zram02.sh
> +++ b/tools/testing/selftests/zram/zram02.sh
> @@ -14,9 +14,6 @@ ERR_CODE=0
>
> # 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.
> -zram_max_streams="2"
>
> # The zram sysfs node 'disksize' value can be either in bytes,
> # or you can use mem suffixes. But in some old kernels, mem
> @@ -30,7 +27,6 @@ zram_mem_limits="1M"
>
> check_prereqs
> zram_load
> -zram_max_streams
> zram_set_disksizes
> zram_set_memlimit
> zram_makeswap
> diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
> index 6f872f266fd1..0c49f9d1d563 100755
> --- a/tools/testing/selftests/zram/zram_lib.sh
> +++ b/tools/testing/selftests/zram/zram_lib.sh
> @@ -82,28 +82,6 @@ zram_load()
> fi
> }
>
> -zram_max_streams()
> -{
> - echo "set max_comp_streams to zram device(s)"
> -
> - local i=0
> - for max_s in $zram_max_streams; do
> - local sys_path="/sys/block/zram${i}/max_comp_streams"
> - echo $max_s> $sys_path || \
> - echo "FAIL failed to set '$max_s' to $sys_path"
> - sleep 1
> - local max_streams=$(cat $sys_path)
> -
> - [ "$max_s" -ne "$max_streams" ]&& \
> - echo "FAIL can't set max_streams '$max_s', get $max_stream"
> -
> - i=$(($i + 1))
> - echo "$sys_path = '$max_streams' ($i/$dev_num)"
> - done
> -
> - echo "zram max streams: OK"
> -}
> -
> zram_compress_alg()
> {
> echo "test that we can set compression algorithm"

2022-01-26 09:40:41

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

On 12/15/21 2:56 AM, Yang Xu wrote:
> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram has
> switched to per-cpu streams. Even kernel still keep this interface for some reasons, but
> writing to max_comp_stream doesn't take any effect. So remove it.

I get that max_comp_stream doesn't do anything since this referenced
commit. Don't we need this test on older kernels since older kernels
still support max_comp_stream?

thanks,
-- Shuah

2022-01-26 09:40:58

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 2/3] selftests/zram01.sh: Fix compression ratio calculation

On 12/15/21 2:56 AM, Yang Xu wrote:
> zram01 uses `free -m` to measure zram memory usage. The results are nonsense
> because they are polluted by all running processes on the system.
>

Are the results inaccurate or does /sys/block/zram<id>/mm_stat is a quick
way to get the information?

In any case, this patch and all 3 patches in this series have:

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)

Please run checkpatch.pl and clean these up.

thanks,
-- Shuah

2022-01-26 09:42:52

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 3/3] selftests/zram: Adapt the situation that /dev/zram0 is being used

On 12/15/21 2:56 AM, Yang Xu wrote:
> On some linux distributions, if it used /dev/zram0 as default by zram-generate
> service, then this case will fail or report EBUSY.
>
> To fix this, use hot_add/hot_remove interface. Also, move module check code into
> zram_load from zram.sh. We can use /sys/class/zram-control to detect the
> module whether existed.
>
> After this patch, we still run case pass even /dev/zram0 is being used.
>
> Signed-off-by: Yang Xu <[email protected]>
> ---
> tools/testing/selftests/zram/zram.sh | 16 +----
> tools/testing/selftests/zram/zram01.sh | 2 +-
> tools/testing/selftests/zram/zram_lib.sh | 75 ++++++++++++------------
> 3 files changed, 41 insertions(+), 52 deletions(-)
>

This patch looks good to me. A few checkpatch warns to fix and also
it probably depends on the other two patches in this series.

Please fix and send v2 for all 3 patches.

thanks,
-- Shuah

2022-01-26 09:45:55

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

On 1/12/22 11:26 PM, [email protected] wrote:
> Hi Ping
>
> Best Regards
> Yang Xu
>> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram has
>> switched to per-cpu streams. Even kernel still keep this interface for some reasons, but
>> writing to max_comp_stream doesn't take any effect. So remove it.
>>
>> Signed-off-by: Yang Xu<[email protected]>

Reviewed and changes requested.

thanks,
-- Shuah

2022-01-26 18:39:25

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

on 2022/1/26 4:33, Shuah Khan wrote :
> On 12/15/21 2:56 AM, Yang Xu wrote:
>> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams
>> internals"), zram has
>> switched to per-cpu streams. Even kernel still keep this interface for
>> some reasons, but
>> writing to max_comp_stream doesn't take any effect. So remove it.
>
> I get that max_comp_stream doesn't do anything since this referenced
> commit. Don't we need this test on older kernels since older kernels
> still support max_comp_stream?

I read the following info from kernel selftest documentation
https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html

"The kernel contains a set of “self tests” under the
tools/testing/selftests/ directory. These are intended to be small tests
to exercise individual code paths in the kernel. Tests are intended to
be run after building, installing and booting a kernel."

So, we can build older kernel(use older kernel source) if we want to
test older kernel.

IMO, kerenl selftest is different from other testsuit(ie ltp, this
shuould think about api changes because ltp may test on different kernel).
Also cc ltp co-maintainer Petr

Or, did I miss something?

Best Regards
Yang Xu


>
> thanks,
> -- Shuah

2022-01-26 19:54:31

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 2/3] selftests/zram01.sh: Fix compression ratio calculation

on 2022/1/26 4:37, Shuah Khan wrote:
> On 12/15/21 2:56 AM, Yang Xu wrote:
>> zram01 uses `free -m` to measure zram memory usage. The results are
>> nonsense
>> because they are polluted by all running processes on the system.
>>
>
> Are the results inaccurate or does /sys/block/zram<id>/mm_stat is a quick
> way to get the information?
The "free -m" result is inaccurate because it caculted global systemd
free memory instead of process that used zram device.

We should use mm_stat as Documentation/admin-guide/blockdev/zram.rst wrote:

File /sys/block/zram<id>/mm_stat

The mm_stat file represents the device's mm statistics. It consists of a
single
line of text and contains the following stats separated by whitespace:

================
=============================================================
orig_data_size uncompressed size of data stored in this disk.
Unit: bytes
compr_data_size compressed size of data stored in this disk
mem_used_total the amount of memory allocated for this disk. This
includes allocator fragmentation and metadata
>
> In any case, this patch and all 3 patches in this series have:
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
>
> Please run checkpatch.pl and clean these up.
Ok, Will do it in v2.

Best Regards
Yang Xu
>
> thanks,
> -- Shuah

2022-01-26 20:16:45

by Petr Vorel

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

Hi all,

> on 2022/1/26 4:33, Shuah Khan wrote :
> > On 12/15/21 2:56 AM, Yang Xu wrote:
> >> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams
> >> internals"), zram has
> >> switched to per-cpu streams. Even kernel still keep this interface for
> >> some reasons, but
> >> writing to max_comp_stream doesn't take any effect. So remove it.

> > I get that max_comp_stream doesn't do anything since this referenced
> > commit. Don't we need this test on older kernels since older kernels
> > still support max_comp_stream?

> I read the following info from kernel selftest documentation
> https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html

> "The kernel contains a set of “self tests” under the
> tools/testing/selftests/ directory. These are intended to be small tests
> to exercise individual code paths in the kernel. Tests are intended to
> be run after building, installing and booting a kernel."

> So, we can build older kernel(use older kernel source) if we want to
> test older kernel.

> IMO, kernel selftest is different from other testsuit(ie ltp, this
> shuould think about api changes because ltp may test on different kernel).
Yes, that's how I understand the difference with approach of in kselftest - the
kernel tree testsuite and LTP - the out-of-tree testsuite.

Kind regards,
Petr

> Also cc ltp co-maintainer Petr

> Or, did I miss something?

> Best Regards
> Yang Xu



> > thanks,
> > -- Shuah

2022-01-26 22:23:03

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

On 1/26/22 12:13 AM, Petr Vorel wrote:
> Hi all,
>
>> on 2022/1/26 4:33, Shuah Khan wrote :
>>> On 12/15/21 2:56 AM, Yang Xu wrote:
>>>> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams
>>>> internals"), zram has
>>>> switched to per-cpu streams. Even kernel still keep this interface for
>>>> some reasons, but
>>>> writing to max_comp_stream doesn't take any effect. So remove it.
>
>>> I get that max_comp_stream doesn't do anything since this referenced
>>> commit. Don't we need this test on older kernels since older kernels
>>> still support max_comp_stream?
>
>> I read the following info from kernel selftest documentation
>> https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html
>
>> "The kernel contains a set of “self tests” under the
>> tools/testing/selftests/ directory. These are intended to be small tests
>> to exercise individual code paths in the kernel. Tests are intended to
>> be run after building, installing and booting a kernel."
>
>> So, we can build older kernel(use older kernel source) if we want to
>> test older kernel.
>
>> IMO, kernel selftest is different from other testsuit(ie ltp, this
>> shuould think about api changes because ltp may test on different kernel).
> Yes, that's how I understand the difference with approach of in kselftest - the
> kernel tree testsuite and LTP - the out-of-tree testsuite.
>

Removing max_comp_stream test appears to be motivated by the fact it isn't
needed on newer kernels.

Kselftest from mainline can be run on older stable kernels. This is a use-case
for a lot test rings. The idea is that when a new test gets added for older
code to regression test a bug, we should be able to run that test on an older
kernel. This is the reason why we don't remove code that can still test an older
kernel and make sure it skips gracefully.

Hence, I won't be taking this patch.

thanks,
-- Shuah

2022-01-26 22:27:36

by Petr Vorel

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

> On 1/26/22 12:13 AM, Petr Vorel wrote:
> > Hi all,

> > > on 2022/1/26 4:33, Shuah Khan wrote :
> > > > On 12/15/21 2:56 AM, Yang Xu wrote:
> > > > > Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams
> > > > > internals"), zram has
> > > > > switched to per-cpu streams. Even kernel still keep this interface for
> > > > > some reasons, but
> > > > > writing to max_comp_stream doesn't take any effect. So remove it.

> > > > I get that max_comp_stream doesn't do anything since this referenced
> > > > commit. Don't we need this test on older kernels since older kernels
> > > > still support max_comp_stream?

> > > I read the following info from kernel selftest documentation
> > > https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html

> > > "The kernel contains a set of “self tests” under the
> > > tools/testing/selftests/ directory. These are intended to be small tests
> > > to exercise individual code paths in the kernel. Tests are intended to
> > > be run after building, installing and booting a kernel."

> > > So, we can build older kernel(use older kernel source) if we want to
> > > test older kernel.

> > > IMO, kernel selftest is different from other testsuit(ie ltp, this
> > > shuould think about api changes because ltp may test on different kernel).
> > Yes, that's how I understand the difference with approach of in kselftest - the
> > kernel tree testsuite and LTP - the out-of-tree testsuite.


> Removing max_comp_stream test appears to be motivated by the fact it isn't
> needed on newer kernels.

> Kselftest from mainline can be run on older stable kernels. This is a use-case
> for a lot test rings. The idea is that when a new test gets added for older
> code to regression test a bug, we should be able to run that test on an older
> kernel. This is the reason why we don't remove code that can still test an older
> kernel and make sure it skips gracefully.

Thanks for clarifying this approach. It might be worth of documenting it in
dev-tools/kselftest.rst.

Kind regards,
Petr

> Hence, I won't be taking this patch.

> thanks,
> -- Shuah

2022-01-26 22:29:26

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 1/3] selftests/zram: Remove obsolete max_comp_streams interface

On 1/26/22 11:24 AM, Petr Vorel wrote:
>> On 1/26/22 12:13 AM, Petr Vorel wrote:
>>> Hi all,
>
>>>> on 2022/1/26 4:33, Shuah Khan wrote :
>>>>> On 12/15/21 2:56 AM, Yang Xu wrote:
>>>>>> Since kernel commit 43209ea2d17a ("zram: remove max_comp_streams
>>>>>> internals"), zram has
>>>>>> switched to per-cpu streams. Even kernel still keep this interface for
>>>>>> some reasons, but
>>>>>> writing to max_comp_stream doesn't take any effect. So remove it.
>
>>>>> I get that max_comp_stream doesn't do anything since this referenced
>>>>> commit. Don't we need this test on older kernels since older kernels
>>>>> still support max_comp_stream?
>
>>>> I read the following info from kernel selftest documentation
>>>> https://www.kernel.org/doc/html/latest/dev-tools/kselftest.html
>
>>>> "The kernel contains a set of “self tests” under the
>>>> tools/testing/selftests/ directory. These are intended to be small tests
>>>> to exercise individual code paths in the kernel. Tests are intended to
>>>> be run after building, installing and booting a kernel."
>
>>>> So, we can build older kernel(use older kernel source) if we want to
>>>> test older kernel.
>
>>>> IMO, kernel selftest is different from other testsuit(ie ltp, this
>>>> shuould think about api changes because ltp may test on different kernel).
>>> Yes, that's how I understand the difference with approach of in kselftest - the
>>> kernel tree testsuite and LTP - the out-of-tree testsuite.
>
>
>> Removing max_comp_stream test appears to be motivated by the fact it isn't
>> needed on newer kernels.
>
>> Kselftest from mainline can be run on older stable kernels. This is a use-case
>> for a lot test rings. The idea is that when a new test gets added for older
>> code to regression test a bug, we should be able to run that test on an older
>> kernel. This is the reason why we don't remove code that can still test an older
>> kernel and make sure it skips gracefully.
>
> Thanks for clarifying this approach. It might be worth of documenting it in
> dev-tools/kselftest.rst.
>

I will send out a patch clarifying this.

thanks,
-- Shuah

2022-01-27 14:53:03

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: [PATCH v2 1/3] selftests/zram: Skip max_comp_streams interface on newer kernel

Since commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram
has switched to per-cpu streams. Even kernel still keep this interface for
some reasons, but writing to max_comp_stream doesn't take any effect. So
skip it on newer kernel ie 4.7.

The code that comparing kernel version is from xfstests testsuite ext4/053.

Signed-off-by: Yang Xu <[email protected]>
---
tools/testing/selftests/zram/zram_lib.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
index 6f872f266fd1..f47fc0f27e99 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -11,6 +11,9 @@ dev_mounted=-1

# Kselftest framework requirement - SKIP code is 4.
ksft_skip=4
+kernel_version=`uname -r | cut -d'.' -f1,2`
+kernel_major=${kernel_version%.*}
+kernel_minor=${kernel_version#*.}

trap INT

@@ -25,6 +28,20 @@ check_prereqs()
fi
}

+kernel_gte()
+{
+ major=${1%.*}
+ minor=${1#*.}
+
+ if [ $kernel_major -gt $major ]; then
+ return 0
+ elif [[ $kernel_major -eq $major && $kernel_minor -ge $minor ]]; then
+ return 0
+ fi
+
+ return 1
+}
+
zram_cleanup()
{
echo "zram cleanup"
@@ -86,6 +103,13 @@ zram_max_streams()
{
echo "set max_comp_streams to zram device(s)"

+ kernel_gte 4.7
+ if [ $? -eq 0 ]; then
+ echo "The device attribute max_comp_streams was"\
+ "deprecated in 4.7"
+ return 0
+ fi
+
local i=0
for max_s in $zram_max_streams; do
local sys_path="/sys/block/zram${i}/max_comp_streams"
--
2.23.0

2022-01-27 14:53:09

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: [PATCH v2 3/3] selftests/zram: Adapt the situation that /dev/zram0 is being used

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. So
even zram device is being used, we still can add zram device and remove
them in cleanup.

The two interface was introduced since kernel commit 6566d1a32bf7("zram:
add dynamic device add/remove functionality") in v4.2-rc1. If kernel
supports these two interface, we use hot_add/hot_remove to slove this
problem, if not, just check whether zram is being used or built in, then
skip it on old kernel.

Signed-off-by: Yang Xu <[email protected]>
---
fix this ie the following ltp commit does
https://github.com/linux-test-project/ltp/commit/caed7833
also removing useless zram_unload
tools/testing/selftests/zram/zram.sh | 15 +---
tools/testing/selftests/zram/zram01.sh | 3 +-
tools/testing/selftests/zram/zram02.sh | 1 -
tools/testing/selftests/zram/zram_lib.sh | 110 +++++++++++++----------
4 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/tools/testing/selftests/zram/zram.sh b/tools/testing/selftests/zram/zram.sh
index 232e958ec454..b0b91d9b0dc2 100755
--- a/tools/testing/selftests/zram/zram.sh
+++ b/tools/testing/selftests/zram/zram.sh
@@ -2,9 +2,6 @@
# SPDX-License-Identifier: GPL-2.0
TCID="zram.sh"

-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
-
. ./zram_lib.sh

run_zram () {
@@ -18,14 +15,4 @@ echo ""

check_prereqs

-# check zram module exists
-MODULE_PATH=/lib/modules/`uname -r`/kernel/drivers/block/zram/zram.ko
-if [ -f $MODULE_PATH ]; then
- run_zram
-elif [ -b /dev/zram0 ]; then
- run_zram
-else
- echo "$TCID : No zram.ko module or /dev/zram0 device file not found"
- echo "$TCID : CONFIG_ZRAM is not set"
- exit $ksft_skip
-fi
+run_zram
diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
index e9e9eb777e2c..8f4affe34f3e 100755
--- a/tools/testing/selftests/zram/zram01.sh
+++ b/tools/testing/selftests/zram/zram01.sh
@@ -33,7 +33,7 @@ zram_algs="lzo"

zram_fill_fs()
{
- for i in $(seq 0 $(($dev_num - 1))); do
+ for i in $(seq $dev_start $dev_end); do
echo "fill zram$i..."
local b=0
while [ true ]; do
@@ -67,7 +67,6 @@ zram_mount

zram_fill_fs
zram_cleanup
-zram_unload

if [ $ERR_CODE -ne 0 ]; then
echo "$TCID : [FAIL]"
diff --git a/tools/testing/selftests/zram/zram02.sh b/tools/testing/selftests/zram/zram02.sh
index e83b404807c0..2418b0c4ed13 100755
--- a/tools/testing/selftests/zram/zram02.sh
+++ b/tools/testing/selftests/zram/zram02.sh
@@ -36,7 +36,6 @@ zram_set_memlimit
zram_makeswap
zram_swapoff
zram_cleanup
-zram_unload

if [ $ERR_CODE -ne 0 ]; then
echo "$TCID : [FAIL]"
diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
index f47fc0f27e99..21ec1966de76 100755
--- a/tools/testing/selftests/zram/zram_lib.sh
+++ b/tools/testing/selftests/zram/zram_lib.sh
@@ -5,10 +5,12 @@
# Author: Alexey Kodanev <[email protected]>
# Modified: Naresh Kamboju <[email protected]>

-MODULE=0
dev_makeswap=-1
dev_mounted=-1
-
+dev_start=0
+dev_end=-1
+module_load=-1
+sys_control=-1
# Kselftest framework requirement - SKIP code is 4.
ksft_skip=4
kernel_version=`uname -r | cut -d'.' -f1,2`
@@ -46,57 +48,72 @@ zram_cleanup()
{
echo "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
rm -rf zram$i
done

-}
+ if [ $sys_control -eq 1 ]; then
+ for i in $(seq $dev_start $dev_end); do
+ echo $i > /sys/class/zram-control/hot_remove
+ done
+ fi

-zram_unload()
-{
- if [ $MODULE -ne 0 ] ; then
- echo "zram rmmod zram"
+ if [ $module_load -eq 1 ]; then
rmmod zram > /dev/null 2>&1
fi
}

zram_load()
{
- # check zram module exists
- MODULE_PATH=/lib/modules/`uname -r`/kernel/drivers/block/zram/zram.ko
- if [ -f $MODULE_PATH ]; then
- MODULE=1
- echo "create '$dev_num' zram device(s)"
- modprobe zram num_devices=$dev_num
- if [ $? -ne 0 ]; then
- echo "failed to insert zram module"
- exit 1
- fi
-
- dev_num_created=$(ls /dev/zram* | wc -w)
+ echo "create '$dev_num' zram device(s)"
+
+ # zram module loaded, new kernel
+ if [ -d "/sys/class/zram-control" ]; then
+ echo "zram modules already loaded, kernel supports" \
+ "zram-control interface"
+ dev_start=$(ls /dev/zram* | wc -w)
+ dev_end=$(($dev_start + $dev_num - 1))
+ sys_control=1
+
+ for i in $(seq $dev_start $dev_end); do
+ cat /sys/class/zram-control/hot_add > /dev/null
+ done
+
+ echo "all zram devices (/dev/zram$dev_start~$dev_end" \
+ "successfully created"
+ return 0
+ fi

- if [ "$dev_num_created" -ne "$dev_num" ]; then
- echo "unexpected num of devices: $dev_num_created"
- ERR_CODE=-1
+ # detect old kernel or built-in
+ modprobe zram num_devices=$dev_num
+ if [ ! -d "/sys/class/zram-control" ]; then
+ if grep -q '^zram' /proc/modules; then
+ rmmod zram > /dev/null 2>&1
+ if [ $? -ne 0 ]; then
+ echo "zram module is being used on old kernel" \
+ "without zram-control interface"
+ exit $ksft_skip
+ fi
else
- echo "zram load module successful"
+ echo "test needs CONFIG_ZRAM=m on old kernel without" \
+ "zram-control interface"
+ exit $ksft_skip
fi
- elif [ -b /dev/zram0 ]; then
- echo "/dev/zram0 device file found: OK"
- else
- echo "ERROR: No zram.ko module or no /dev/zram0 device found"
- echo "$TCID : CONFIG_ZRAM is not set"
- exit 1
+ modprobe zram num_devices=$dev_num
fi
+
+ module_load=1
+ dev_end=$(($dev_num - 1))
+ echo "all zram devices (/dev/zram0~$dev_end) successfully created"
}

zram_max_streams()
@@ -110,7 +127,7 @@ zram_max_streams()
return 0
fi

- 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"
echo $max_s > $sys_path || \
@@ -122,7 +139,7 @@ zram_max_streams()
echo "FAIL can't set max_streams '$max_s', get $max_stream"

i=$(($i + 1))
- echo "$sys_path = '$max_streams' ($i/$dev_num)"
+ echo "$sys_path = '$max_streams'"
done

echo "zram max streams: OK"
@@ -132,15 +149,16 @@ zram_compress_alg()
{
echo "test that we can set compression algorithm"

- local algs=$(cat /sys/block/zram0/comp_algorithm)
+ local i=$dev_start
+ local algs=$(cat /sys/block/zram${i}/comp_algorithm)
echo "supported algs: $algs"
- local i=0
+
for alg in $zram_algs; do
local sys_path="/sys/block/zram${i}/comp_algorithm"
echo "$alg" > $sys_path || \
echo "FAIL can't set '$alg' to $sys_path"
i=$(($i + 1))
- echo "$sys_path = '$alg' ($i/$dev_num)"
+ echo "$sys_path = '$alg'"
done

echo "zram set compression algorithm: OK"
@@ -149,14 +167,14 @@ zram_compress_alg()
zram_set_disksizes()
{
echo "set disk size to zram device(s)"
- local i=0
+ local i=$dev_start
for ds in $zram_sizes; do
local sys_path="/sys/block/zram${i}/disksize"
echo "$ds" > $sys_path || \
echo "FAIL can't set '$ds' to $sys_path"

i=$(($i + 1))
- echo "$sys_path = '$ds' ($i/$dev_num)"
+ echo "$sys_path = '$ds'"
done

echo "zram set disksizes: OK"
@@ -166,14 +184,14 @@ zram_set_memlimit()
{
echo "set memory limit to zram device(s)"

- local i=0
+ local i=$dev_start
for ds in $zram_mem_limits; do
local sys_path="/sys/block/zram${i}/mem_limit"
echo "$ds" > $sys_path || \
echo "FAIL can't set '$ds' to $sys_path"

i=$(($i + 1))
- echo "$sys_path = '$ds' ($i/$dev_num)"
+ echo "$sys_path = '$ds'"
done

echo "zram set memory limit: OK"
@@ -182,8 +200,8 @@ zram_set_memlimit()
zram_makeswap()
{
echo "make swap with zram device(s)"
- local i=0
- for i in $(seq 0 $(($dev_num - 1))); do
+ local i=$dev_start
+ for i in $(seq $dev_start $dev_end); do
mkswap /dev/zram$i > err.log 2>&1
if [ $? -ne 0 ]; then
cat err.log
@@ -206,7 +224,7 @@ zram_makeswap()
zram_swapoff()
{
local i=
- for i in $(seq 0 $dev_makeswap); do
+ for i in $(seq $dev_start $dev_end); do
swapoff /dev/zram$i > err.log 2>&1
if [ $? -ne 0 ]; then
cat err.log
@@ -220,7 +238,7 @@ zram_swapoff()

zram_makefs()
{
- local i=0
+ local i=$dev_start
for fs in $zram_filesystems; do
# if requested fs not supported default it to ext2
which mkfs.$fs > /dev/null 2>&1 || fs=ext2
@@ -239,7 +257,7 @@ zram_makefs()
zram_mount()
{
local i=0
- for i in $(seq 0 $(($dev_num - 1))); do
+ for i in $(seq $dev_start $dev_end); do
echo "mount /dev/zram$i"
mkdir zram$i
mount /dev/zram$i zram$i > /dev/null || \
--
2.23.0

2022-01-28 13:20:22

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/zram: Skip max_comp_streams interface on newer kernel

On 1/27/22 2:11 AM, Yang Xu wrote:
> Since commit 43209ea2d17a ("zram: remove max_comp_streams internals"), zram
> has switched to per-cpu streams. Even kernel still keep this interface for
> some reasons, but writing to max_comp_stream doesn't take any effect. So
> skip it on newer kernel ie 4.7.
>
> The code that comparing kernel version is from xfstests testsuite ext4/053.
>
> Signed-off-by: Yang Xu <[email protected]>
> ---

Thank you. This and the other 2 patches are in linux-kselftest fixes branch
for rc3.

thanks,
-- Shuah

2022-01-29 07:01:50

by Yang Xu (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] selftests/zram: Skip max_comp_streams interface on newer kernel

on 2022/1/28 2:05, Shuah Khan wrote:
> On 1/27/22 2:11 AM, Yang Xu wrote:
>> Since commit 43209ea2d17a ("zram: remove max_comp_streams internals"),
>> zram
>> has switched to per-cpu streams. Even kernel still keep this interface
>> for
>> some reasons, but writing to max_comp_stream doesn't take any effect. So
>> skip it on newer kernel ie 4.7.
>>
>> The code that comparing kernel version is from xfstests testsuite
>> ext4/053.
>>
>> Signed-off-by: Yang Xu <[email protected]>
>> ---
>
> Thank you. This and the other 2 patches are in linux-kselftest fixes branch
> for rc3.
Thanks for your review and merge.

Best Regards
Yang Xu
>
> thanks,
> -- Shuah