* [LTP] [PATCH 0/1] Fix zram01.sh
@ 2019-11-04 12:52 Martin Doucha
2019-11-04 12:52 ` [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01 Martin Doucha
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-04 12:52 UTC (permalink / raw)
To: ltp
The compression ratio check in zram01.sh is completely broken:
- It uses `free -m` to *guess* zram memory usage which is distorted by all
other processes running on the system.
- It calculates memory usage backwards ($before - $after; which should
produce a negative value if the two values weren't nonsense).
- It mixes bytes with megabytes when calculating the final ratio.
Here's a patch to calculate the compression ratio correctly using sysfs memory
usage statistics for each zram device. I currently calculate it as
$bytes_written / $mm_stat_compr_data_size.
One question for debate is whether I should use a different formula:
- $bytes_written / $mm_stat_mem_used_total
(mem_used_total includes internal zram memory management overhead)
- $mm_stat_orig_data_size / $mm_stat_compr_data_size
(zram01.sh fills the zram device with binary zeroes so the data will be
compacted even before compression; orig_data_size << bytes_written)
- $mm_stat_orig_data_size / $mm_stat_mem_used_total)
(this could easily produce ratio <100% due to page compacting)
See https://www.kernel.org/doc/Documentation/blockdev/zram.txt
The mm_stat sysfs file is available since kernel 4.1. Unfortunately,
I couldn't test the TCONF branch when mm_stat doesn't exist because there's
no SLE release with kernel version between 3.14 and 4.1.
Verifcation run: https://openqa.suse.de/tests/3552880#step/zram01/6
Martin Doucha (1):
Fix compression ratio calculation in zram01
.../kernel/device-drivers/zram/zram01.sh | 36 +++++++++----------
1 file changed, 16 insertions(+), 20 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01
2019-11-04 12:52 [LTP] [PATCH 0/1] Fix zram01.sh Martin Doucha
@ 2019-11-04 12:52 ` Martin Doucha
2019-11-04 15:16 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-04 12:52 UTC (permalink / raw)
To: ltp
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(-)
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
--
2.23.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01
2019-11-04 12:52 ` [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01 Martin Doucha
@ 2019-11-04 15:16 ` Petr Vorel
2019-11-04 15:51 ` Martin Doucha
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2019-11-04 15:16 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01
2019-11-04 15:16 ` Petr Vorel
@ 2019-11-04 15:51 ` Martin Doucha
2019-11-05 8:23 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-04 15:51 UTC (permalink / raw)
To: ltp
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.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01
2019-11-04 15:51 ` Martin Doucha
@ 2019-11-05 8:23 ` Petr Vorel
2019-11-05 8:46 ` Martin Doucha
2019-11-05 9:03 ` [LTP] [PATCH v2 0/1] " Martin Doucha
0 siblings, 2 replies; 11+ messages in thread
From: Petr Vorel @ 2019-11-05 8:23 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01
2019-11-05 8:23 ` Petr Vorel
@ 2019-11-05 8:46 ` Martin Doucha
2019-11-05 9:03 ` [LTP] [PATCH v2 0/1] " Martin Doucha
1 sibling, 0 replies; 11+ messages in thread
From: Martin Doucha @ 2019-11-05 8:46 UTC (permalink / raw)
To: ltp
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.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 0/1] Fix compression ratio calculation in zram01
2019-11-05 8:23 ` Petr Vorel
2019-11-05 8:46 ` Martin Doucha
@ 2019-11-05 9:03 ` Martin Doucha
2019-11-05 9:03 ` [LTP] [PATCH v2 1/1] " Martin Doucha
1 sibling, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-05 9:03 UTC (permalink / raw)
To: ltp
Changes from v1:
- TINFO about amount of data written to zram device changed to TPASS
Shortened patch description from v1:
The compression ratio check in zram01.sh is completely broken. Here's a patch
to calculate the compression ratio correctly using sysfs memory usage
statistics for each zram device. I currently calculate it as
$bytes_written / $mm_stat_compr_data_size.
One question for debate is whether I should use a different formula:
- $bytes_written / $mm_stat_mem_used_total
(mem_used_total includes internal zram memory management overhead)
- $mm_stat_orig_data_size / $mm_stat_compr_data_size
(zram01.sh fills the zram device with binary zeroes so the data will be
compacted even before compression; orig_data_size << bytes_written)
- $mm_stat_orig_data_size / $mm_stat_mem_used_total)
(this could easily produce ratio <100% due to page compacting)
See https://www.kernel.org/doc/Documentation/blockdev/zram.txt
Martin Doucha (1):
Fix compression ratio calculation in zram01
.../kernel/device-drivers/zram/zram01.sh | 38 +++++++++----------
1 file changed, 17 insertions(+), 21 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/1] Fix compression ratio calculation in zram01
2019-11-05 9:03 ` [LTP] [PATCH v2 0/1] " Martin Doucha
@ 2019-11-05 9:03 ` Martin Doucha
2019-11-08 15:29 ` Cyril Hrubis
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-05 9:03 UTC (permalink / raw)
To: ltp
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 | 38 +++++++++----------
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index 9508211ab..4c9adee7b 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..."
@@ -74,30 +73,27 @@ zram_fill_fs()
[ -s err.txt ] && tst_resm TWARN "dd error: $(cat err.txt)"
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
+ tst_resm TPASS "zram$i can be filled with '$b' KB"
- [ $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
--
2.23.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/1] Fix compression ratio calculation in zram01
2019-11-05 9:03 ` [LTP] [PATCH v2 1/1] " Martin Doucha
@ 2019-11-08 15:29 ` Cyril Hrubis
2019-11-08 15:31 ` Martin Doucha
0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2019-11-08 15:29 UTC (permalink / raw)
To: ltp
Hi!
> + 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}'`
Why not just:
awk '{print $2}' /sys/block/zram$i/mm_stat
Other than this it looks good. Also no need to send v3, I can fix this
when applying.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/1] Fix compression ratio calculation in zram01
2019-11-08 15:29 ` Cyril Hrubis
@ 2019-11-08 15:31 ` Martin Doucha
2019-11-08 15:49 ` Cyril Hrubis
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2019-11-08 15:31 UTC (permalink / raw)
To: ltp
On 11/8/19 4:29 PM, Cyril Hrubis wrote:
> Hi!
>> + local compr_size=`cat "/sys/block/zram$i/mm_stat" | awk '{print $2}'`
>
> Why not just:
>
> awk '{print $2}' /sys/block/zram$i/mm_stat
>
> Other than this it looks good. Also no need to send v3, I can fix this
> when applying.
You're right, please fix it.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/1] Fix compression ratio calculation in zram01
2019-11-08 15:31 ` Martin Doucha
@ 2019-11-08 15:49 ` Cyril Hrubis
0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2019-11-08 15:49 UTC (permalink / raw)
To: ltp
Hi!
> > Other than this it looks good. Also no need to send v3, I can fix this
> > when applying.
>
> You're right, please fix it.
Pushed with this change, thanks.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-11-08 15:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 12:52 [LTP] [PATCH 0/1] Fix zram01.sh Martin Doucha
2019-11-04 12:52 ` [LTP] [PATCH 1/1] Fix compression ratio calculation in zram01 Martin Doucha
2019-11-04 15:16 ` Petr Vorel
2019-11-04 15:51 ` Martin Doucha
2019-11-05 8:23 ` Petr Vorel
2019-11-05 8:46 ` Martin Doucha
2019-11-05 9:03 ` [LTP] [PATCH v2 0/1] " Martin Doucha
2019-11-05 9:03 ` [LTP] [PATCH v2 1/1] " Martin Doucha
2019-11-08 15:29 ` Cyril Hrubis
2019-11-08 15:31 ` Martin Doucha
2019-11-08 15:49 ` Cyril Hrubis
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.