All of lore.kernel.org
 help / color / mirror / Atom feed
From: "xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>
To: Petr Vorel <pvorel@suse.cz>
Cc: "ltp@lists.linux.it" <ltp@lists.linux.it>
Subject: Re: [LTP] [PATCH v5 2/4] zram/zram_lib.sh: adapt the situation that zram device is being used
Date: Tue, 21 Dec 2021 01:33:24 +0000	[thread overview]
Message-ID: <61C12EF0.1020403@fujitsu.com> (raw)
In-Reply-To: <YcBeuHSm65VC8qmA@pevik>

Hi Petr
> Hi Xu,
>
>> 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[1]. 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 6566d1a32("zram: add dynamic
>> device add/remove functionality")[2] in 2015.6. If kernel supports these two interface,
> nit: instead of date (a bit non standard written) I'd just mention the kernel
> release, i.e. v4.2-rc1.
Will do.
>
>> 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.
>
>> Also, zram01,02 case are adjuested to adapt the situation that CONFIG_ZRAM=y and can
>> run zram01,02 simultaneously on new kernel.
>
>> [1]https://www.kernel.org/doc/html/latest/admin-guide/blockdev/zram.html#add-remove-zram-devices
>> [2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6566d1a32bf7
>
>> -	modprobe zram num_devices=$dev_num || \
>> -		tst_brk TBROK "failed to insert zram module"
>> +# On kernel that supports /sys/class/zram-control interface but doesn't load zram,
>> +# we dont' need to use hot_add/hot_remove interface. If system has loaded zram
>> +# or buitin, we need to use hot_add/hot_remove interface.
>> +# On old kernel that doesn't support /sys/class/zram-control interface, we just
>> +# check whether zram module is being used or it is built in kernel(we can't create
>> +# number of devices required). If so, skip it.
>> +	if [ ! -d "/sys/class/zram-control" ]; then
>> +		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 || \
>> +					tst_brk TCONF "zram module is being used"
> nit: I'd be more clear already in the output.
Yes.
>
>> +			else
>> +				tst_brk TCONF "test needs CONFIG_ZRAM=m"
> Also here I'd somehow mention the old kernel.
>> +			fi
>> +			modprobe zram num_devices=$dev_num
>> +		fi
>> +		module_load=1
>> +		dev_end=$(($dev_num - 1))
>> +		tst_res TPASS "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))
>> +	sys_control=1
>
>> -	if [ "$dev_num_created" -ne "$dev_num" ]; then
>> -		tst_brk TFAIL "unexpected num of devices: $dev_num_created"
>> -	fi
>> +	for i in $(seq  $dev_start $dev_end); do
>> +		cat /sys/class/zram-control/hot_add>  /dev/null
>> +	done
>
>> -	tst_res TPASS "all zram devices successfully created"
>> +	tst_res TPASS "all zram devices (/dev/zram$dev_start~$dev_end) successfully created"
>>   }
>
> IMHO this should work and be a bit more readable
> (put extra TINFO to help understand the problem on failure or what has been
> tested):
>
> 	tst_res TINFO "create '$dev_num' zram device(s)"
>
> 	# zram module loaded, new kernel
> 	if [ -d "/sys/class/zram-control" ]; then
> 		tst_res TINFO "zram module 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
>
> 		tst_res TPASS "all zram devices (/dev/zram$dev_start~$dev_end) successfully created"
> 		return
> 	fi
>
> 	modprobe zram num_devices=$dev_num
>
> 	# detect old kernel or built-in
> 	if [ ! -d "/sys/class/zram-control" ]; then
> 		if grep -q '^zram' /proc/modules; then
> 			rmmod zram>  /dev/null 2>&1 || \
> 				tst_brk TCONF "zram module is being used on old kernel without zram-control interface"
> 		else
> 			tst_brk TCONF "test needs CONFIG_ZRAM=m on old kernel without zram-control interface"
> 		fi
> 		modprobe zram num_devices=$dev_num
> 	fi
>
> 	module_load=1
> 	dev_end=$(($dev_num - 1))
> 	tst_res TPASS "all zram devices (/dev/zram0~$dev_end) successfully created"

Nice, it is easy to understand.

Best Regards
Yang Xu
> }
>
>
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

  reply	other threads:[~2021-12-21  1:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20  6:52 [LTP] [PATCH v5 1/4] swapping01: skip test if zram-swap is being used Yang Xu
2021-12-20  6:52 ` [LTP] [PATCH v5 2/4] zram/zram_lib.sh: adapt the situation that zram device " Yang Xu
2021-12-20 10:45   ` Petr Vorel
2021-12-21  1:33     ` xuyang2018.jy [this message]
2021-12-20  6:52 ` [LTP] [PATCH v5 3/4] zram/zram03: Convert into new api Yang Xu
2021-12-20  6:52 ` [LTP] [PATCH v5 4/4] zram/zram01.sh: replacing data stored in this disk with allocated for this disk Yang Xu
2021-12-20 10:49   ` Petr Vorel
2021-12-21  2:18     ` xuyang2018.jy
2021-12-21  8:41       ` Petr Vorel
2021-12-22  9:33         ` xuyang2018.jy
2021-12-22 14:12           ` Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=61C12EF0.1020403@fujitsu.com \
    --to=xuyang2018.jy@fujitsu.com \
    --cc=ltp@lists.linux.it \
    --cc=pvorel@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.