Re: [PATCH 1/9] seek_sanity_test: fix allocation unit detection on XFS realtime

["liuyd.fnst@fujitsu.com" <liuyd.fnst@fujitsu.com>]

Hi, Darrick.


On 7/28/22 10:37, Darrick J. Wong wrote:
> Does this patch fix NFS for you?

It works for me. Thanks.
BTW. It has conflict during "git am". Looks like your branch are ahead 
of master.

> 
> --D
> ---
> From: Darrick J. Wong <djwong@kernel.org>
> 
> seek_sanity_test: use XFS ioctls to determine file allocation unit size
> 
> liuyd.fnst@fujitsu.com reported that my recent change to the seek sanity
> test broke NFS.  I foolishly thought that st_blksize was sufficient to
> find the file allocation unit size so that applications could figure out
> the SEEK_HOLE granularity.  Replace that with an explicit callout to XFS
> ioctls so that xfs realtime will work again.
> 
> Fixes: e861a302 ("seek_sanity_test: fix allocation unit detection on XFS realtime")
> Reported-by: liuyd.fnst@fujitsu.com
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>   src/Makefile           |    4 ++++
>   src/seek_sanity_test.c |   40 +++++++++++++++++++++++++++++++---------
>   2 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/src/Makefile b/src/Makefile
> index 38628a22..b89a7a5e 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -82,6 +82,10 @@ ifeq ($(HAVE_LIBCAP), true)
>   LLDLIBS += -lcap
>   endif
>   
> +ifeq ($(HAVE_FSXATTR_XFLAG_HASATTR), yes)
> +LCFLAGS += -DHAVE_FSXATTR_XFLAG_HASATTR
> +endif
> +
>   ifeq ($(HAVE_SEEK_DATA), yes)
>    ifeq ($(HAVE_FSXATTR_XFLAG_HASATTR), yes)
>     ifeq ($(HAVE_NFTW), yes)
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 1030d0c5..b53f4862 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -40,6 +40,32 @@ static void get_file_system(int fd)
>   	}
>   }
>   
> +#ifdef HAVE_FSXATTR_XFLAG_HASATTR
> +/* Compute the file allocation unit size for an XFS file. */
> +static int detect_xfs_alloc_unit(int fd)
> +{
> +	struct fsxattr fsx;
> +	struct xfs_fsop_geom fsgeom;
> +	int ret;
> +
> +	ret = ioctl(fd, XFS_IOC_FSGEOMETRY, &fsgeom);
> +	if (ret)
> +		return -1;
> +
> +	ret = ioctl(fd, FS_IOC_FSGETXATTR, &fsx);
> +	if (ret)
> +		return -1;
> +
> +	alloc_size = fsgeom.blocksize;
> +	if (fsx.fsx_xflags & FS_XFLAG_REALTIME)
> +		alloc_size *= fsgeom.rtextsize;
> +
> +	return 0;
> +}
> +#else
> +# define detect_xfs_alloc_unit(fd) (-1)
> +#endif
> +
>   static int get_io_sizes(int fd)
>   {
>   	off_t pos = 0, offset = 1;
> @@ -47,6 +73,10 @@ static int get_io_sizes(int fd)
>   	int shift, ret;
>   	int pagesz = sysconf(_SC_PAGE_SIZE);
>   
> +	ret = detect_xfs_alloc_unit(fd);
> +	if (!ret)
> +		goto done;
> +
>   	ret = fstat(fd, &buf);
>   	if (ret) {
>   		fprintf(stderr, "  ERROR %d: Failed to find io blocksize\n",
> @@ -54,16 +84,8 @@ static int get_io_sizes(int fd)
>   		return ret;
>   	}
>   
> -	/*
> -	 * st_blksize is typically also the allocation size.  However, XFS
> -	 * rounds this up to the page size, so if the stat blocksize is exactly
> -	 * one page, use this iterative algorithm to see if SEEK_DATA will hint
> -	 * at a more precise answer based on the filesystem's (pre)allocation
> -	 * decisions.
> -	 */
> +	/* st_blksize is typically also the allocation size */
>   	alloc_size = buf.st_blksize;
> -	if (alloc_size != pagesz)
> -		goto done;
>   
>   	/* try to discover the actual alloc size */
>   	while (pos == 0 && offset < alloc_size) {