diff mbox series

[2/3] selftests/zram01.sh: Fix compression ratio calculation

Message ID 1639562171-4434-2-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive)
State Changes Requested
Headers show
Series [1/3] selftests/zram: Remove obsolete max_comp_streams interface | expand

Commit Message

Yang Xu (Fujitsu) Dec. 15, 2021, 9:56 a.m. UTC
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 <xuyang2018.jy@fujitsu.com>
---
 tools/testing/selftests/zram/zram01.sh | 30 +++++++-------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

Comments

Shuah Khan Jan. 25, 2022, 8:37 p.m. UTC | #1
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
Yang Xu (Fujitsu) Jan. 26, 2022, 6:08 a.m. UTC | #2
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
diff mbox series

Patch

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