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

[Anand Jain <anand.jain@oracle.com>]

On 16/11/24 00:39, Darrick J. Wong wrote:
> 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.
> 

Agreed. It's better to report the issue at test N. I'll add this comment 
to the code in v2.

Thanks!
Anand

> --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
>>
>>