diff mbox series

[1/5] tests/test enhance

Message ID 20240418102321.95384-2-xni@redhat.com (mailing list archive)
State Changes Requested
Headers show
Series mdadm tests fix and enhance | expand

Commit Message

Xiao Ni April 18, 2024, 10:23 a.m. UTC
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.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Mateusz Kusiak April 19, 2024, 7:16 a.m. UTC | #1
Hi Xiao,
one small note from me.

On 18.04.2024 12:23, Xiao Ni wrote:
> @@ -309,6 +322,7 @@ print_warning() {
>   main() {
>   	print_warning
>   	do_setup
> +	record_system_speed_limit
>   
>   	echo "Testing on linux-$(uname -r) kernel"
>   	[ "$savelogs" == "1" ] &&


I feel like record_system_speed_limit() should be called in do_setup() rather than main().
Saving current system settings is job of setup.

Thanks,
Mateusz
Mateusz Kusiak April 19, 2024, 7:30 a.m. UTC | #2
On 19.04.2024 09:16, Mateusz Kusiak wrote:
> Hi Xiao,
> one small note from me.
> 
> On 18.04.2024 12:23, Xiao Ni wrote:
>> @@ -309,6 +322,7 @@ print_warning() {
>>   main() {
>>       print_warning
>>       do_setup
>> +    record_system_speed_limit
>>       echo "Testing on linux-$(uname -r) kernel"
>>       [ "$savelogs" == "1" ] &&
> 
> 
> I feel like record_system_speed_limit() should be called in do_setup() rather than main().
> Saving current system settings is job of setup.
> 
> Thanks,
> Mateusz
> 

One more thing. Feel free to add tag "fixes".
I broke this behavior (lowering sync speed) in 4c12714d1ca0 ("test: run tests on system level mdadm").

Thanks,
Mateusz
Xiao Ni April 21, 2024, 2:46 a.m. UTC | #3
On Fri, Apr 19, 2024 at 3:30 PM Mateusz Kusiak
<mateusz.kusiak@linux.intel.com> wrote:
>
> On 19.04.2024 09:16, Mateusz Kusiak wrote:
> > Hi Xiao,
> > one small note from me.
> >
> > On 18.04.2024 12:23, Xiao Ni wrote:
> >> @@ -309,6 +322,7 @@ print_warning() {
> >>   main() {
> >>       print_warning
> >>       do_setup
> >> +    record_system_speed_limit
> >>       echo "Testing on linux-$(uname -r) kernel"
> >>       [ "$savelogs" == "1" ] &&
> >
> >
> > I feel like record_system_speed_limit() should be called in do_setup() rather than main().
> > Saving current system settings is job of setup.

Thanks for the suggestion.

> >
> > Thanks,
> > Mateusz
> >
>
> One more thing. Feel free to add tag "fixes".
> I broke this behavior (lowering sync speed) in 4c12714d1ca0 ("test: run tests on system level mdadm").

Ok, no problem.

Regards
Xiao
>
> Thanks,
> Mateusz
>
diff mbox series

Patch

diff --git a/test b/test
index 338c2db44fa7..2aa3bd75d503 100755
--- a/test
+++ b/test
@@ -6,7 +6,8 @@  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
 devlist=
 
 savelogs=0
@@ -39,8 +40,19 @@  ctrl_c() {
 	ctrl_c_error=1
 }
 
+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 $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
+}
+
 restore_system_speed_limit() {
-	echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
+	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
 }
 
 mdadm() {
@@ -103,6 +115,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
@@ -309,6 +322,7 @@  print_warning() {
 main() {
 	print_warning
 	do_setup
+	record_system_speed_limit
 
 	echo "Testing on linux-$(uname -r) kernel"
 	[ "$savelogs" == "1" ] &&