diff mbox series

[3/3] seek_sanity_test: use XFS ioctls to determine file allocation unit size

Message ID 165903224646.2338516.11839049913536195078.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series fstests: random fixes for v2022.07.24 | expand

Commit Message

Darrick J. Wong July 28, 2022, 6:17 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

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
Tested-by: liuyd.fnst@fujitsu.com
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 src/seek_sanity_test.c |   36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

Comments

Zorro Lang July 31, 2022, 2:11 p.m. UTC | #1
On Thu, Jul 28, 2022 at 11:17:26AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> 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
> Tested-by: liuyd.fnst@fujitsu.com
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

This patch looks good to me, and I can reproduce the regression on nfs, then
test passed after merge this patch. So I'd like to merge this patch at first,
to fix that regression for nfs. Others 2 patches are still under discussion,
I'll wait.

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  src/seek_sanity_test.c |   36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 1030d0c5..78f835e8 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -40,6 +40,28 @@ static void get_file_system(int fd)
>  	}
>  }
>  
> +/* 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, XFS_IOC_FSGETXATTR, &fsx);
> +	if (ret)
> +		return -1;
> +
> +	alloc_size = fsgeom.blocksize;
> +	if (fsx.fsx_xflags & XFS_XFLAG_REALTIME)
> +		alloc_size *= fsgeom.rtextsize;
> +
> +	return 0;
> +}
> +
>  static int get_io_sizes(int fd)
>  {
>  	off_t pos = 0, offset = 1;
> @@ -47,6 +69,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 +80,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) {
>
diff mbox series

Patch

diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index 1030d0c5..78f835e8 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -40,6 +40,28 @@  static void get_file_system(int fd)
 	}
 }
 
+/* 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, XFS_IOC_FSGETXATTR, &fsx);
+	if (ret)
+		return -1;
+
+	alloc_size = fsgeom.blocksize;
+	if (fsx.fsx_xflags & XFS_XFLAG_REALTIME)
+		alloc_size *= fsgeom.rtextsize;
+
+	return 0;
+}
+
 static int get_io_sizes(int fd)
 {
 	off_t pos = 0, offset = 1;
@@ -47,6 +69,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 +80,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) {