Re: [PATCH v2] fstests: btrfs: Add regression test for reserved space leak.

[Tsutomu Itoh <t-itoh@jp.fujitsu.com>]

On 2015/08/05 12:10, Dave Chinner wrote:
> On Wed, Aug 05, 2015 at 11:52:36AM +0900, Tsutomu Itoh wrote:
>> On 2015/08/05 10:57, Dave Chinner wrote:
>>> On Wed, Aug 05, 2015 at 09:39:35AM +0800, Qu Wenruo wrote:
>>>> Tsutomu Itoh wrote on 2015/08/05 10:26 +0900:
>>>>> On 2015/08/05 10:08, Qu Wenruo wrote:
>>>>>> +# As the reserved space freeing happens at commit_transaction time,
>>>>>> +# without a transaction commit, no reserved space needs freeing and
>>>>>> +# won't trigger the bug.
>>>>>> +sync
>>>>>
>>>>> Isn't '$BTRFS_UTIL_PROG filesystem sync' better instead of 'sync'?
>>>>>
>>>>> Thanks,
>>>>> Tsutomu
>>>> Hi, Tsutomu-san,
>>>>
>>>> Yes, I did use such method before, but Dave said it's better to use
>>>> unified interface to sync a filesystem other than the specialized
>>>> one.
>>>>
>>>> So I still use sync as Dave said.
>>>
>>> Mainly because "sync" is what users will use to make sure their data
>>> is safe. filesystem specific tools have a habit of doing "special
>>> stuff" to sync a filesystem, so it may not reflect the way users
>>> expect the system to behaviour when they run sync.
>>>
>>> The other option is this:
>>>
>>> _syncfs()
>>> {
>>> 	mntpt=$1
>>>
>>> 	$XFS_IO_PROG -c syncfs $mntpt
>>> }
>>>
>>> _sync_test()
>>> {
>>> 	_syncfs $TEST_DIR
>>> }
>>>
>>> _sync_scratch()
>>> {
>>> 	_syncfs $SCRATCH_MNT
>>> }
>>>
>>> which only runs sync on the filesystem that needs syncing (via the
>>> syncfs() syscall)
>>
>> I think that syncfs is better instead of sync because the syncing of
>> the specified filesystem is necessary.
>
> sync guarantees that. IOWs, there's no difference between sync and
> syncfs for the persepctive of this test and so the test does not
> need really need changing.

I agree that sync and syncfs is not different for this test.

However, sync is syncing of all filesystems, and, syncfs is syncing
of the specified filesystem only.
Therefor, I think that it is better to use sync and syncfs properly
according to the necessity.

Thanks,
Tsutomu