[PATCH] xfs/298: Add 100ms sleep before scratch_umount

[PATCH] xfs/298: Add 100ms sleep before scratch_umount

[Yang Xu]

When testing this case on my machine, it reports the following error:
umount: /mnt/xfstests/scratch: target is busy.
xfs_db: /dev/sda11 contains a mounted filesystem

scratch_unmount failed, so  _scratch_xfs_db reports scratch_dev is a
mounted filesystem. It seems filesystem has something to be doing.

To avoid this, just add a 100ms sleep before scratch_umount.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 tests/xfs/298 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/xfs/298 b/tests/xfs/298
index b0153ebf..17510379 100755
--- a/tests/xfs/298
+++ b/tests/xfs/298
@@ -51,6 +51,7 @@ while [ $SIZE -lt 1024 ];do
 	rm $SYMLINK_FILE
 # umount and check the number of extents on the inode. Should be 0.
 	cd /
+	sleep 0.1
 	_scratch_unmount >/dev/null 2>&1
 	_scratch_xfs_db  -c "inode $inode" -c "p core.nextents"
 
-- 
2.23.0

Re: [PATCH] xfs/298: Add 100ms sleep before scratch_umount

[Zorro Lang]

On Mon, May 09, 2022 at 02:56:57PM +0800, Yang Xu wrote:
> When testing this case on my machine, it reports the following error:
> umount: /mnt/xfstests/scratch: target is busy.
> xfs_db: /dev/sda11 contains a mounted filesystem
> 
> scratch_unmount failed, so  _scratch_xfs_db reports scratch_dev is a
> mounted filesystem. It seems filesystem has something to be doing.
> 
> To avoid this, just add a 100ms sleep before scratch_umount.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  tests/xfs/298 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/xfs/298 b/tests/xfs/298
> index b0153ebf..17510379 100755
> --- a/tests/xfs/298
> +++ b/tests/xfs/298
> @@ -51,6 +51,7 @@ while [ $SIZE -lt 1024 ];do
>  	rm $SYMLINK_FILE
>  # umount and check the number of extents on the inode. Should be 0.
>  	cd /
> +	sleep 0.1

Hi

Actually I was wondering if we can add a while loop trying in _scratch_unmount,
to avoid some random "device busy" problem, likes:

Change _scratch_unmount to __scratch_unmount, then:

_scratch_unmount()
{
	local n
	local is_unmounted=1

	__scratch_unmount > $tmp.scratch_unmount.err 2>&1
	if [ $? -ne 0 -a "$MULTI_SCRATCH_UNMOUNT" = "yes" ];then
		for ((n=0; n<5; n++));do
			sleep 0.1
			__scratch_unmount >> $tmp.scratch_unmount.err 2>&1
			if [ $? -eq 0 ];then
				is_unmounted=0
				break
			fi
		done
	else
		is_unmounted=0
	fi
	if [ $is_unmounted -ne 0 ];then
		cat $tmp.scratch_unmount.err
	fi
	rm -f $tmp.scratch_unmount.err
	return $is_unmounted
}

This's just an idea out of my head, hope to hear more suggestions from other
forks.

Thanks,
Zorro

>  	_scratch_unmount >/dev/null 2>&1
>  	_scratch_xfs_db  -c "inode $inode" -c "p core.nextents"
>  
> -- 
> 2.23.0
> 

Re: [PATCH] xfs/298: Add 100ms sleep before scratch_umount

[xuyang2018.jy]

on 2022/5/9 21:42, Zorro Lang wrote:
> On Mon, May 09, 2022 at 02:56:57PM +0800, Yang Xu wrote:
>> When testing this case on my machine, it reports the following error:
>> umount: /mnt/xfstests/scratch: target is busy.
>> xfs_db: /dev/sda11 contains a mounted filesystem
>>
>> scratch_unmount failed, so  _scratch_xfs_db reports scratch_dev is a
>> mounted filesystem. It seems filesystem has something to be doing.
>>
>> To avoid this, just add a 100ms sleep before scratch_umount.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   tests/xfs/298 | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/xfs/298 b/tests/xfs/298
>> index b0153ebf..17510379 100755
>> --- a/tests/xfs/298
>> +++ b/tests/xfs/298
>> @@ -51,6 +51,7 @@ while [ $SIZE -lt 1024 ];do
>>   	rm $SYMLINK_FILE
>>   # umount and check the number of extents on the inode. Should be 0.
>>   	cd /
>> +	sleep 0.1
>
> Hi
>
> Actually I was wondering if we can add a while loop trying in _scratch_unmount,
> to avoid some random "device busy" problem, likes:
>
> Change _scratch_unmount to __scratch_unmount, then:
>
> _scratch_unmount()
> {
> 	local n
> 	local is_unmounted=1
>
> 	__scratch_unmount>  $tmp.scratch_unmount.err 2>&1
> 	if [ $? -ne 0 -a "$MULTI_SCRATCH_UNMOUNT" = "yes" ];then
> 		for ((n=0; n<5; n++));do
> 			sleep 0.1
> 			__scratch_unmount>>  $tmp.scratch_unmount.err 2>&1
> 			if [ $? -eq 0 ];then
> 				is_unmounted=0
> 				break
> 			fi
> 		done
> 	else
> 		is_unmounted=0
> 	fi
> 	if [ $is_unmounted -ne 0 ];then
> 		cat $tmp.scratch_unmount.err
> 	fi
> 	rm -f $tmp.scratch_unmount.err
> 	return $is_unmounted
> }
>
> This's just an idea out of my head, hope to hear more suggestions from other
> forks.

I think this suggestion is meaningful and ltp also has a similar C api[1].

But we don't use this api in everywhere in ltp. So I don't use this api 
in everywhere in xfstests and just create a new function name to use it 
in situation that use many times mount/umount.

Also, xfstests some cases need to filter the stderr ie xfs/444.

So I think we can create a api that try to umount many times(5 or10) and
people can use this api if they meet the random ebusy problem instead of 
breaking the now _scratch_unmount api.

What do you think about it?

[1]https://github.com/linux-test-project/ltp/blob/master/lib/tst_device.c#L382

Best Regards
Yang Xu
>
> Thanks,
> Zorro
>
>>   	_scratch_unmount>/dev/null 2>&1
>>   	_scratch_xfs_db  -c "inode $inode" -c "p core.nextents"
>>
>> --
>> 2.23.0
>>

Re: [PATCH] xfs/298: Add 100ms sleep before scratch_umount

[xuyang2018.jy]

on 2022/5/9 21:42, Zorro Lang wrote:
> On Mon, May 09, 2022 at 02:56:57PM +0800, Yang Xu wrote:
>> When testing this case on my machine, it reports the following error:
>> umount: /mnt/xfstests/scratch: target is busy.
>> xfs_db: /dev/sda11 contains a mounted filesystem
>>
>> scratch_unmount failed, so  _scratch_xfs_db reports scratch_dev is a
>> mounted filesystem. It seems filesystem has something to be doing.
>>
>> To avoid this, just add a 100ms sleep before scratch_umount.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   tests/xfs/298 | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/xfs/298 b/tests/xfs/298
>> index b0153ebf..17510379 100755
>> --- a/tests/xfs/298
>> +++ b/tests/xfs/298
>> @@ -51,6 +51,7 @@ while [ $SIZE -lt 1024 ];do
>>   	rm $SYMLINK_FILE
>>   # umount and check the number of extents on the inode. Should be 0.
>>   	cd /
>> +	sleep 0.1
>
> Hi
>
> Actually I was wondering if we can add a while loop trying in _scratch_unmount,
> to avoid some random "device busy" problem, likes:
>
> Change _scratch_unmount to __scratch_unmount, then:
>
> _scratch_unmount()
> {
> 	local n
> 	local is_unmounted=1
>
> 	__scratch_unmount>  $tmp.scratch_unmount.err 2>&1
> 	if [ $? -ne 0 -a "$MULTI_SCRATCH_UNMOUNT" = "yes" ];then
> 		for ((n=0; n<5; n++));do
> 			sleep 0.1
> 			__scratch_unmount>>  $tmp.scratch_unmount.err 2>&1
> 			if [ $? -eq 0 ];then
> 				is_unmounted=0
> 				break
> 			fi
> 		done
> 	else
> 		is_unmounted=0
> 	fi
> 	if [ $is_unmounted -ne 0 ];then
> 		cat $tmp.scratch_unmount.err
> 	fi
> 	rm -f $tmp.scratch_unmount.err
> 	return $is_unmounted
> }
>
> This's just an idea out of my head, hope to hear more suggestions from other
> forks.

Another way is that introdued a environment variable, so people can 
enable this functionality in _scratch_unmount when meet ebusy problem.
By default, _scratch_unmount work as it did before.

Best Regards
Yang Xu
>
> Thanks,
> Zorro
>
>>   	_scratch_unmount>/dev/null 2>&1
>>   	_scratch_xfs_db  -c "inode $inode" -c "p core.nextents"
>>
>> --
>> 2.23.0
>>

Re: [PATCH] xfs/298: Add 100ms sleep before scratch_umount

[Dave Chinner]

On Mon, May 09, 2022 at 02:56:57PM +0800, Yang Xu wrote:
> When testing this case on my machine, it reports the following error:
> umount: /mnt/xfstests/scratch: target is busy.
> xfs_db: /dev/sda11 contains a mounted filesystem
> 
> scratch_unmount failed, so  _scratch_xfs_db reports scratch_dev is a
> mounted filesystem. It seems filesystem has something to be doing.
> 
> To avoid this, just add a 100ms sleep before scratch_umount.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  tests/xfs/298 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/xfs/298 b/tests/xfs/298
> index b0153ebf..17510379 100755
> --- a/tests/xfs/298
> +++ b/tests/xfs/298
> @@ -51,6 +51,7 @@ while [ $SIZE -lt 1024 ];do
>  	rm $SYMLINK_FILE
>  # umount and check the number of extents on the inode. Should be 0.
>  	cd /
> +	sleep 0.1
>  	_scratch_unmount >/dev/null 2>&1

What? No.

Never put random undocument sleeps in tests to hide failures - they
are almost always covering up a problem that needs to be understood
and fixed.

First thing to do here is understand why the filesystem is busy and
can't unmount. What is holding a reference that prevents unmount?

Is systemd or udev doing something stupid on your system and so
racing with unmount? Or is something else going on?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com