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

[Dave Chinner <david@fromorbit.com>]

On Wed, Apr 30, 2025 at 02:26:28PM +0530, Nirjhar Roy (IBM) wrote:
> On Thu, 2025-04-17 at 13:01 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > xfs/183 runs bulkstat_unlink_test to create 100 inodes, unlink on
> > and bulkstat them. It takes a ridiculously long time to run under
> > check-parallel because it runs sync() multiple times per iteration:
> > 
> > Ten slowest tests - runtime in seconds:
> > .....
> > xfs/183 328
> > 
> > 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.
> > 
> > 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:
> > 
> > xfs/183        4s
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  src/bulkstat_unlink_test.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/bulkstat_unlink_test.c b/src/bulkstat_unlink_test.c
> > index d78cc2ac2..62e5bb978 100644
> > --- a/src/bulkstat_unlink_test.c
> > +++ b/src/bulkstat_unlink_test.c
> > @@ -88,7 +88,7 @@ main(int argc, char *argv[])
> >  		}
> >  
> >  		if (chknb) { /* Get the original number of inodes
> > (lazy) */
> > -			sync();
> > +			syncfs(fd[nfiles]);
> This looks good to me. 
> Minor: Is it safe to use int fd[nfiles + 1];

Yes, it is. Variable size array declarations like this have
been supported by C and C compilers for a long time.

> Isn't it better to use
> malloc()?

See my previous comments about stack usage in userspace test code. :)

-Dave.

-- 
Dave Chinner
david@fromorbit.com