| Message ID | 20191104125228.17173-2-mdoucha@suse.cz |
|---|---|
| State | Superseded |
| Delegated to: | Alexey Kodanev |
| Headers | show |
| Series | Fix zram01.sh | expand |
Hi Martin, > 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 /sys/block/zram<id>/mm_stat to measure memory usage instead. The file is > available since kernel 4.1. +1 even older kernels won't be covered. Thanks! ... > --- a/testcases/kernel/device-drivers/zram/zram01.sh > zram_fill_fs() > { > - tst_test_cmds awk bc dd free > - local mem_free0=$(free -m | awk 'NR==2 {print $4}') > + tst_test_cmds awk bc dd > for i in $(seq 0 $(($dev_num - 1))); do > tst_resm TINFO "fill zram$i..." > @@ -75,29 +74,26 @@ zram_fill_fs() > tst_brkm TBROK "cannot fill zram" > fi > tst_resm TINFO "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 total_size=0 > - for sm in $zram_sizes; do > - local s=$(echo $sm | sed 's/M//') > - total_size=$(($total_size + $s)) > - done > - [ $used_mem -eq 0 ] && tst_brkm TBROK "no memory used by zram" > + if [ ! -f "/sys/block/zram$i/mm_stat" ]; then > + if [ $i -eq 0 ]; then > + tst_resm TCONF "zram compression ratio test requires zram mm_stat sysfs file" I wonder if /sys/block/zram0/mm_stat is missing whether it can be on any other zram<id>. Is it it's presence file system specific? Shouldn't we just quit whole test wit tst_brk TCONF. Kind regards, Petr
On 11/4/19 4:16 PM, Petr Vorel wrote: > I wonder if /sys/block/zram0/mm_stat is missing whether it can be on any other > zram<id>. Is it it's presence file system specific? Shouldn't we just quit whole > test wit tst_brk TCONF. If /sys/block/zram0/mm_stat is missing then all /sys/block/zram*/mm_stat files should be missing. But I don't want to terminate the test there because the remaining 3 write tests could still find a regression. So print a TCONF message on the first pass and silently skip the remaining compression ratio checks. I was also thinking about checking whether the write test filled the test file at least up to 50% of memory limit if mm_stat doesn't exist. But it'd mostly add unnecessary complexity.
Hi Martin, > On 11/4/19 4:16 PM, Petr Vorel wrote: > > I wonder if /sys/block/zram0/mm_stat is missing whether it can be on any other > > zram<id>. Is it it's presence file system specific? Shouldn't we just quit whole > > test wit tst_brk TCONF. > If /sys/block/zram0/mm_stat is missing then all /sys/block/zram*/mm_stat > files should be missing. But I don't want to terminate the test there > because the remaining 3 write tests could still find a regression. So > print a TCONF message on the first pass and silently skip the remaining > compression ratio checks. Do you mean that dd filling zram could find a regression? I'm asking because it's a bit strange to have test, which doesn't lead to any result (TPASS/TFAIL/TBROK/TCONF), which will be If this part is also a test, maybe following TINFO should be changed to TPASS. + Also new shell API allows to use loop in API (code simplify), but that requires for each run to produce a result. > I was also thinking about checking whether the write test filled the > test file at least up to 50% of memory limit if mm_stat doesn't exist. > But it'd mostly add unnecessary complexity. Agree. Kind regards, Petr
On 11/5/19 9:23 AM, Petr Vorel wrote: > Do you mean that dd filling zram could find a regression? > > I'm asking because it's a bit strange to have test, > which doesn't lead to any result (TPASS/TFAIL/TBROK/TCONF), which will be > > If this part is also a test, maybe following TINFO should be changed to TPASS. > + Also new shell API allows to use loop in API (code simplify), but that > requires for each run to produce a result. Note the TBROK if `dd` fails to write anything at all. But I'll change the TINFO to TPASS and resubmit.
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh index 9508211ab..2a8189de2 100755 --- a/testcases/kernel/device-drivers/zram/zram01.sh +++ b/testcases/kernel/device-drivers/zram/zram01.sh @@ -58,8 +58,7 @@ TST_CLEANUP="zram_cleanup" zram_fill_fs() { - tst_test_cmds awk bc dd free - local mem_free0=$(free -m | awk 'NR==2 {print $4}') + tst_test_cmds awk bc dd for i in $(seq 0 $(($dev_num - 1))); do tst_resm TINFO "fill zram$i..." @@ -75,29 +74,26 @@ zram_fill_fs() tst_brkm TBROK "cannot fill zram" fi tst_resm TINFO "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 total_size=0 - for sm in $zram_sizes; do - local s=$(echo $sm | sed 's/M//') - total_size=$(($total_size + $s)) - done - [ $used_mem -eq 0 ] && tst_brkm TBROK "no memory used by zram" + if [ ! -f "/sys/block/zram$i/mm_stat" ]; then + if [ $i -eq 0 ]; then + tst_resm TCONF "zram compression ratio test requires zram mm_stat sysfs file" + fi - tst_resm TINFO "zram used ${used_mem}M, zram disk sizes ${total_size}M" + continue + fi - local v=$((100 * $total_size / $used_mem)) + local compr_size=`cat "/sys/block/zram$i/mm_stat" | awk '{print $2}'` + local v=$((100 * 1024 * $b / $compr_size)) + local r=`echo "scale=2; $v / 100 " | bc` - if [ "$v" -lt 100 ]; then - tst_resm TFAIL "compression ratio: 0.$v:1" - return - fi + if [ "$v" -lt 100 ]; then + tst_resm TFAIL "compression ratio: $r:1" + break + fi - tst_resm TPASS "compression ratio: $(echo "scale=2; $v / 100 " | bc):1" + tst_resm TPASS "compression ratio: $r:1" + done } zram_load
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 /sys/block/zram<id>/mm_stat to measure memory usage instead. The file is available since kernel 4.1. Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- .../kernel/device-drivers/zram/zram01.sh | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-)