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

[Dave Chinner <david@fromorbit.com>]

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.

-Dave.
-- 
Dave Chinner
david@fromorbit.com