| Message ID | 1639983142-2247-4-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v5,1/4] swapping01: skip test if zram-swap is being used | expand |
> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we nit: we can drop [1] > used free -m changes to calculate the compression ratio. > After the above patch, we used compr_data_size to calculate. kernel documentation[2] has nit: I'd add link to the doc also in this commit message. Kind regards, Petr > the following info: > orig_data_size: uncompressed size of data stored in this disk. > compr_data_size: compressed size of data stored in this disk > mem_used_total: the amount of memory allocated for this disk > We should calculate the compression ratio by used disk size divided into used mem size. > It can also avoid the situation that division by 0 as below: > zram01 7 TINFO: filling zram4 (it can take long time) > zram01 7 TPASS: zram4 was filled with '25568' KB > zram01 7 TINFO: compr_size 0 > /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0") > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
Hi Petr >> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we > nit: we can drop [1] >> used free -m changes to calculate the compression ratio. > >> After the above patch, we used compr_data_size to calculate. kernel documentation[2] has > nit: I'd add link to the doc also in this commit message. I don't see the link on your ltp fork: https://github.com/pevik/ltp/tree/yang_xu/zram-swap.v5.fixes I have tried this branch and these change seems fine. I guess I don't need to send a v6 patch and you can merge it directly(by removing [1][2] linke and adding fixes tag ). Is it right? ps: I want to add a fixes tag for pointing to commit 4372f7a2156 ("Fix compression ratio calculation in zram01"). Best Regards Yang Xu > > Kind regards, > Petr > >> the following info: >> orig_data_size: uncompressed size of data stored in this disk. >> compr_data_size: compressed size of data stored in this disk >> mem_used_total: the amount of memory allocated for this disk > >> We should calculate the compression ratio by used disk size divided into used mem size. >> It can also avoid the situation that division by 0 as below: >> zram01 7 TINFO: filling zram4 (it can take long time) >> zram01 7 TPASS: zram4 was filled with '25568' KB >> zram01 7 TINFO: compr_size 0 >> /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0") > >> Reviewed-by: Petr Vorel<pvorel@suse.cz> >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
Hi Xu, > Hi Petr > >> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we > > nit: we can drop [1] > >> used free -m changes to calculate the compression ratio. > >> After the above patch, we used compr_data_size to calculate. kernel documentation[2] has > > nit: I'd add link to the doc also in this commit message. > I don't see the link on your ltp fork: > https://github.com/pevik/ltp/tree/yang_xu/zram-swap.v5.fixes I haven't fixed it there, I pushed it to verify there is no build failure. > I have tried this branch and these change seems fine. Yes, code should have my suggestions. > I guess I don't need to send a v6 patch and you can merge it directly(by > removing [1][2] linke and adding fixes tag ). Is it right? Ah, sure, go ahead and merge. I'm sorry to pick on details. > ps: I want to add a fixes tag for pointing to commit 4372f7a2156 ("Fix > compression ratio calculation in zram01"). +1, thanks! Kind regards, Petr > Best Regards > Yang Xu
Hi Petr Thanks for your review, I have pushed this patchset. Also adding some debuginfo in zram03.c like you do in zram_lib.sh. ps: I have sent a patchset in kernel selftest to update zram case a week ago, but doesn't get any reponse... https://patchwork.kernel.org/project/linux-kselftest/list/?series=595877 Best Regards Yang Xu > Hi Xu, > >> Hi Petr >>>> Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we >>> nit: we can drop [1] >>>> used free -m changes to calculate the compression ratio. > >>>> After the above patch, we used compr_data_size to calculate. kernel documentation[2] has >>> nit: I'd add link to the doc also in this commit message. >> I don't see the link on your ltp fork: >> https://github.com/pevik/ltp/tree/yang_xu/zram-swap.v5.fixes > I haven't fixed it there, I pushed it to verify there is no build failure. > >> I have tried this branch and these change seems fine. > Yes, code should have my suggestions. > >> I guess I don't need to send a v6 patch and you can merge it directly(by >> removing [1][2] linke and adding fixes tag ). Is it right? > Ah, sure, go ahead and merge. I'm sorry to pick on details. > >> ps: I want to add a fixes tag for pointing to commit 4372f7a2156 ("Fix >> compression ratio calculation in zram01"). > +1, thanks! > > Kind regards, > Petr > >> Best Regards >> Yang Xu
Hi Xu, > Hi Petr > Thanks for your review, I have pushed this patchset. > Also adding some debuginfo in zram03.c like you do in zram_lib.sh. +1 > ps: I have sent a patchset in kernel selftest to update zram case a week > ago, but doesn't get any reponse... > https://patchwork.kernel.org/project/linux-kselftest/list/?series=595877 Great thanks! patchwork is temporarily down (502 Bad Gateway), thus posting lore link: https://lore.kernel.org/linux-kselftest/1639562171-4434-1-git-send-email-xuyang2018.jy@fujitsu.com/#b Although it's meant to be for kernel developers I might find a time to have a look. Kind regards, Petr
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh index 5e13f387c..5b4c05434 100755 --- a/testcases/kernel/device-drivers/zram/zram01.sh +++ b/testcases/kernel/device-drivers/zram/zram01.sh @@ -125,8 +125,8 @@ zram_fill_fs() continue fi - local compr_size=`awk '{print $2}' "/sys/block/zram$i/mm_stat"` - local v=$((100 * 1024 * $b / $compr_size)) + local mem_used_total=`awk '{print $3}' "/sys/block/zram$i/mm_stat"` + local v=$((100 * 1024 * $b / $mem_used_total)) local r=`echo "scale=2; $v / 100 " | bc` if [ "$v" -lt 100 ]; then
Before ltp commit 4372f7a2156 ("Fix compression ratio calculation in zram01")[1], we used free -m changes to calculate the compression ratio. After the above patch, we used compr_data_size to calculate. kernel documentation[2] has the following info: orig_data_size: uncompressed size of data stored in this disk. compr_data_size: compressed size of data stored in this disk mem_used_total: the amount of memory allocated for this disk We should calculate the compression ratio by used disk size divided into used mem size. It can also avoid the situation that division by 0 as below: zram01 7 TINFO: filling zram4 (it can take long time) zram01 7 TPASS: zram4 was filled with '25568' KB zram01 7 TINFO: compr_size 0 /opt/ltp/testcases/bin/zram01.sh: line 131: 100 * 1024 * 25568 / 0: division by 0 (error token is "0") Reviewed-by: Petr Vorel <pvorel@suse.cz> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- testcases/kernel/device-drivers/zram/zram01.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)