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

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

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]; Isn't it better to use
malloc()?
Other than this:
Feel free to add 
Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>  			if (xfsctl(dirname, fd[nfiles],
> XFS_IOC_FSBULKSTAT, &a) != 0) {
>  				printf("Warning (%s:%d),
> xfsctl(XFS_IOC_FSBULKSTAT) FAILED.\n", __FILE__, __LINE__);
>  			}
> @@ -118,7 +118,7 @@ main(int argc, char *argv[])
>  			 *The files are still opened (but unlink()ed) ,
>  			 * we should have more inodes than before
>  			 */
> -			sync();
> +			syncfs(fd[nfiles]);
>  			last_inode = 0;
>  			if (xfsctl(dirname, fd[nfiles],
> XFS_IOC_FSBULKSTAT, &a) != 0) {
>  				printf("Warning (%s:%d),
> xfsctl(XFS_IOC_FSBULKSTAT) FAILED.\n", __FILE__, __LINE__);
> @@ -139,7 +139,7 @@ main(int argc, char *argv[])
>  			 * The files are now closed, we should be back
> to our,
>  			 * previous inode count
>  			 */
> -			sync();
> +			syncfs(fd[nfiles]);
>  			last_inode = 0;
>  			if (xfsctl(dirname, fd[nfiles],
> XFS_IOC_FSBULKSTAT, &a) != 0) {
>  				printf("Warning (%s:%d),
> xfsctl(XFS_IOC_FSBULKSTAT) FAILED.\n", __FILE__, __LINE__);
> @@ -150,7 +150,7 @@ main(int argc, char *argv[])
>  			}
>  		}
>  
> -		sync();
> +		syncfs(fd[nfiles]);
>  		last_inode = 0;
>  		for (;;) {
>  			if ((e = xfsctl(dirname, fd[nfiles],
> XFS_IOC_FSBULKSTAT, &a)) < 0) {
> @@ -173,11 +173,11 @@ main(int argc, char *argv[])
>  			}
>  		}
>  
> -		close(fd[nfiles]);
>  		sprintf(fname, "rm -rf %s\n", dirname);
>  		system(fname);
>  
> -		sync();
> +		syncfs(fd[nfiles]);
> +		close(fd[nfiles]);
>  		sleep(2);
>  		printf("passed\n");
>  	}