Re: [PATCH 04/19] mdadm/tests: test enhance

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Wed, 22 May 2024 16:50:41 +0800
Xiao Ni <xni@redhat.com> wrote:

> There are two changes.
> First, if md module is not loaded, it gives error when reading
> speed_limit_max. So read the value after loading md module which
> is done in do_setup
> 
> Second, sometimes the test reports error sync action doesn't
> happen. But dmesg shows sync action is done. So limit the sync
> speed before test. It doesn't affect the test run time. Because
> check wait sets the max speed before waiting sync action. And
> recording speed_limit_max/min in do_setup.
> 
> Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>  test          | 10 +++++-----
>  tests/func.sh | 26 +++++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/test b/test
> index 338c2db44fa7..ff403293d60b 100755
> --- a/test
> +++ b/test
> @@ -6,7 +6,10 @@ targetdir="/var/tmp"
>  logdir="$targetdir"
>  config=/tmp/mdadm.conf
>  testdir=$PWD/tests
> -system_speed_limit=`cat /proc/sys/dev/raid/speed_limit_max`
> +system_speed_limit_max=0
> +system_speed_limit_min=0
> +test_speed_limit_min=100
> +test_speed_limit_max=500
>  devlist=
>  
>  savelogs=0
> @@ -39,10 +42,6 @@ ctrl_c() {
>  	ctrl_c_error=1
>  }
>  
> -restore_system_speed_limit() {
> -	echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
> -}
> -
>  mdadm() {
>  	rm -f $targetdir/stderr
>  	case $* in
> @@ -103,6 +102,7 @@ do_test() {
>  		do_clean
>  		# source script in a subshell, so it has access to our
>  		# namespace, but cannot change it.
> +		control_system_speed_limit
>  		echo -ne "$_script... "
>  		if ( set -ex ; . $_script ) &> $targetdir/log
>  		then
> diff --git a/tests/func.sh b/tests/func.sh
> index b474442b6abe..221cff158f8c 100644
> --- a/tests/func.sh
> +++ b/tests/func.sh
> @@ -136,6 +136,23 @@ check_env() {
>  	fi
>  }
>  
> +record_system_speed_limit() {
> +	system_speed_limit_max=`cat /proc/sys/dev/raid/speed_limit_max`
> +	system_speed_limit_min=`cat /proc/sys/dev/raid/speed_limit_min`
> +}
> +
> +# To avoid sync action finishes before checking it, it needs to limit
> +# the sync speed
> +control_system_speed_limit() {
> +	echo $test_speed_limit_min > /proc/sys/dev/raid/speed_limit_min
> +	echo $test_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +
> +restore_system_speed_limit() {
> +	echo $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
> +	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +

Hi Xiao,
control and restore functions are not used or in this patch. if you want to use
them in next patches please highlight it in description.
Beside that LGTM.

I will loop over the patches and take as much I can. Looks like there are no
conflicts.

Thanks,
Mariusz