diff mbox series

[1/2] fstests: move fs-module reload to earlier in the run_section function

Message ID 7af8f80173fab5408da86f5b3393e787659a81d6.1731683116.git.anand.jain@oracle.com (mailing list archive)
State New
Headers show
Series [1/2] fstests: move fs-module reload to earlier in the run_section function | expand

Commit Message

Anand Jain Nov. 15, 2024, 3:20 p.m. UTC
Reload the module before each test, instead of later in run_section.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 check | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Darrick J. Wong Nov. 15, 2024, 4:39 p.m. UTC | #1
On Fri, Nov 15, 2024 at 11:20:51PM +0800, Anand Jain wrote:
> Reload the module before each test, instead of later in run_section.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  check | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/check b/check
> index 9222cd7e4f81..d8ee73f48c77 100755
> --- a/check
> +++ b/check
> @@ -935,6 +935,15 @@ function run_section()
>  			continue
>  		fi
>  
> +		# Reload the module after each test to check for leaks or
> +		# other problems.

Hrmm.  The nice thing about doing the reload /after/ each test is that
unloading the module will purge any per-fs slab caches that the module
creates.  The slab teardown logs any forgotten objects while $seq still
points to the test that leaked those objects.  Granted it's not a 100%
solution (that job falls to kmemcheck) but it's a cheap check.

This change makes it so that if test N leaks something, fstests reports
them for test N+1.

--D

> +		if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
> +			_test_unmount 2> /dev/null
> +			_scratch_unmount 2> /dev/null
> +			modprobe -r fs-$FSTYP
> +			modprobe fs-$FSTYP
> +		fi
> +
>  		# record that we really tried to run this test.
>  		if ((!${#loop_status[*]})); then
>  			try+=("$seqnum")
> @@ -1033,15 +1042,6 @@ function run_section()
>  			done
>  		fi
>  
> -		# Reload the module after each test to check for leaks or
> -		# other problems.
> -		if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
> -			_test_unmount 2> /dev/null
> -			_scratch_unmount 2> /dev/null
> -			modprobe -r fs-$FSTYP
> -			modprobe fs-$FSTYP
> -		fi
> -
>  		# Scan for memory leaks after every test so that associating
>  		# a leak to a particular test will be as accurate as possible.
>  		_check_kmemleak || tc_status="fail"
> -- 
> 2.46.1
> 
>
Filipe Manana Nov. 15, 2024, 4:48 p.m. UTC | #2
On Fri, Nov 15, 2024 at 3:22 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Reload the module before each test, instead of later in run_section.

Why?
The change log should give a justification for the change.
It's just stating what the code changes are doing but without any reasoning.

Thanks.

>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  check | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/check b/check
> index 9222cd7e4f81..d8ee73f48c77 100755
> --- a/check
> +++ b/check
> @@ -935,6 +935,15 @@ function run_section()
>                         continue
>                 fi
>
> +               # Reload the module after each test to check for leaks or
> +               # other problems.
> +               if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
> +                       _test_unmount 2> /dev/null
> +                       _scratch_unmount 2> /dev/null
> +                       modprobe -r fs-$FSTYP
> +                       modprobe fs-$FSTYP
> +               fi
> +
>                 # record that we really tried to run this test.
>                 if ((!${#loop_status[*]})); then
>                         try+=("$seqnum")
> @@ -1033,15 +1042,6 @@ function run_section()
>                         done
>                 fi
>
> -               # Reload the module after each test to check for leaks or
> -               # other problems.
> -               if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
> -                       _test_unmount 2> /dev/null
> -                       _scratch_unmount 2> /dev/null
> -                       modprobe -r fs-$FSTYP
> -                       modprobe fs-$FSTYP
> -               fi
> -
>                 # Scan for memory leaks after every test so that associating
>                 # a leak to a particular test will be as accurate as possible.
>                 _check_kmemleak || tc_status="fail"
> --
> 2.46.1
>
>
diff mbox series

Patch

diff --git a/check b/check
index 9222cd7e4f81..d8ee73f48c77 100755
--- a/check
+++ b/check
@@ -935,6 +935,15 @@  function run_section()
 			continue
 		fi
 
+		# Reload the module after each test to check for leaks or
+		# other problems.
+		if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
+			_test_unmount 2> /dev/null
+			_scratch_unmount 2> /dev/null
+			modprobe -r fs-$FSTYP
+			modprobe fs-$FSTYP
+		fi
+
 		# record that we really tried to run this test.
 		if ((!${#loop_status[*]})); then
 			try+=("$seqnum")
@@ -1033,15 +1042,6 @@  function run_section()
 			done
 		fi
 
-		# Reload the module after each test to check for leaks or
-		# other problems.
-		if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
-			_test_unmount 2> /dev/null
-			_scratch_unmount 2> /dev/null
-			modprobe -r fs-$FSTYP
-			modprobe fs-$FSTYP
-		fi
-
 		# Scan for memory leaks after every test so that associating
 		# a leak to a particular test will be as accurate as possible.
 		_check_kmemleak || tc_status="fail"