Re: [PATCH 23/28] open-by-handle.c: use syncfs() rather than sync()

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 5/21/25 08:02, Dave Chinner wrote:
> On Wed, Apr 30, 2025 at 02:32:33PM +0530, Nirjhar Roy (IBM) wrote:
>> On Thu, 2025-04-17 at 13:01 +1000, Dave Chinner wrote:
>>> From: Dave Chinner <dchinner@redhat.com>
>>>
>>> generic/467 runs open_by_handle at least 15 times. Each
>>> execution runs sync() at least once, sometimes three times.
>>>
>>> When running check-parallel, sync() can take a -long- time to
>>> run as there can be dozens of filesystems that need to be synced,
>>> not to mention sync getting hung up behind all the mount and
>>> unmounts that are also being run. This results in:
>>>
>>> Ten slowest tests - runtime in seconds:
>>> generic/467 442
>>> .....
>>>
>>> This test running for a really long time.
>>>
>>> Convert the sync() calls to syncfs() so that they only try to sync
>>> the filesystem under test and not the entire system. This avoids
>>> interactions and delays with other tests and mount/unmount
>>> operations, hence allowing both the test and the overall
>>> check-parallel operation to run faster:
>>>
>>> generic/467        6s
>>>
>>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>>> ---
>>>   src/open_by_handle.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
>>> index a99cce4b3..816bb5d12 100644
>>> --- a/src/open_by_handle.c
>>> +++ b/src/open_by_handle.c
>>> @@ -430,7 +430,7 @@ int main(int argc, char **argv)
>>>   	}
>>>   
>>>   	/* sync to get the new inodes to hit the disk */
>>> -	sync();
>>> +	syncfs(mount_fd);
>>>   
>>>   	/*
>>>   	 * encode the file handles or read them from file (-i) and
>>> maybe store
>>> @@ -563,10 +563,10 @@ int main(int argc, char **argv)
>>>   	}
>>>   
>>>   	/* sync to get log forced for unlink transactions to hit the
>>> disk */
>>> -	sync();
>>> +	syncfs(mount_fd);
>>>   
>>>   	/* sync once more FTW */
>>> -	sync();
>>> +	syncfs(mount_fd);
>> I don't see mount_fd being closed at the end. Maybe all the file
>> descriptors are closed after the program finishes execution?
> Yes, they are closed on exit. Again, simple test code that isn't
> running out of open files, so there's not a lot of reason for this
> patch series to care about such existing issues in the code right
> now.

Yeah, makes sense.

--NR

>
> -Dave.

-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore