[PATCH 1/2] xfs/614: query correct direct I/O alignment

[PATCH 1/2] xfs/614: query correct direct I/O alignment

[Christoph Hellwig]

When creating XFS file systems on files, mkfs will query the file system
for the minimum alignment, which can be larger than that of the
underlying device.  Do the same to link the right output file.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/614 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/614 b/tests/xfs/614
index 0f8952e50b9a..06cc2384f38c 100755
--- a/tests/xfs/614
+++ b/tests/xfs/614
@@ -26,7 +26,7 @@ _require_loop
 $MKFS_XFS_PROG 2>&1 | grep -q concurrency || \
 	_notrun "mkfs does not support concurrency options"
 
-test_dev_lbasize=$(blockdev --getss $TEST_DEV)
+test_dev_lbasize=$($here/src/min_dio_alignment $TEST_DIR $TEST_DEV)
 seqfull=$0
 _link_out_file "lba${test_dev_lbasize}"
 
-- 
2.45.2

[PATCH 2/2] xfs/614: remove the _require_loop call

[Christoph Hellwig]

This test only creates file system images as regular files, but never
actually uses the kernel loop driver.  Remove the extra requirement.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/614 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/xfs/614 b/tests/xfs/614
index 06cc2384f38c..f2ea99edb342 100755
--- a/tests/xfs/614
+++ b/tests/xfs/614
@@ -22,7 +22,6 @@ _cleanup()
 
 
 _require_test
-_require_loop
 $MKFS_XFS_PROG 2>&1 | grep -q concurrency || \
 	_notrun "mkfs does not support concurrency options"
 
-- 
2.45.2

Re: [PATCH 2/2] xfs/614: remove the _require_loop call

[Darrick J. Wong]

On Tue, Feb 04, 2025 at 02:46:55PM +0100, Christoph Hellwig wrote:
> This test only creates file system images as regular files, but never
> actually uses the kernel loop driver.  Remove the extra requirement.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Oh, heh, yeah, loopdev not necessary!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  tests/xfs/614 | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/xfs/614 b/tests/xfs/614
> index 06cc2384f38c..f2ea99edb342 100755
> --- a/tests/xfs/614
> +++ b/tests/xfs/614
> @@ -22,7 +22,6 @@ _cleanup()
>  
>  
>  _require_test
> -_require_loop
>  $MKFS_XFS_PROG 2>&1 | grep -q concurrency || \
>  	_notrun "mkfs does not support concurrency options"
>  
> -- 
> 2.45.2
> 
> 

Re: [PATCH 1/2] xfs/614: query correct direct I/O alignment

[Darrick J. Wong]

On Tue, Feb 04, 2025 at 02:46:54PM +0100, Christoph Hellwig wrote:
> When creating XFS file systems on files, mkfs will query the file system
> for the minimum alignment, which can be larger than that of the
> underlying device.  Do the same to link the right output file.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/xfs/614 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/614 b/tests/xfs/614
> index 0f8952e50b9a..06cc2384f38c 100755
> --- a/tests/xfs/614
> +++ b/tests/xfs/614
> @@ -26,7 +26,7 @@ _require_loop
>  $MKFS_XFS_PROG 2>&1 | grep -q concurrency || \
>  	_notrun "mkfs does not support concurrency options"
>  
> -test_dev_lbasize=$(blockdev --getss $TEST_DEV)
> +test_dev_lbasize=$($here/src/min_dio_alignment $TEST_DIR $TEST_DEV)

Hmmmm... I have a patch with a similar aim in my dev tree that
determines the lba size from whatever mkfs decides is the sector size:

# Figure out what sector size mkfs will use to format, which might be dependent
# upon the directio write geometry of the test filesystem.
loop_file=$TEST_DIR/$seq.loop
rm -f "$loop_file"
truncate -s 16M "$loop_file"
$MKFS_XFS_PROG -f -N "$loop_file" | _filter_mkfs 2>$tmp.mkfs >/dev/null
. $tmp.mkfs
seqfull=$0
_link_out_file "lba${sectsz}"

What do you think of that approach?

--D

>  seqfull=$0
>  _link_out_file "lba${test_dev_lbasize}"
>  
> -- 
> 2.45.2
> 
> 

Re: [PATCH 1/2] xfs/614: query correct direct I/O alignment

[Christoph Hellwig]

On Tue, Feb 04, 2025 at 09:12:58AM -0800, Darrick J. Wong wrote:
> Hmmmm... I have a patch with a similar aim in my dev tree that
> determines the lba size from whatever mkfs decides is the sector size:
> 
> # Figure out what sector size mkfs will use to format, which might be dependent
> # upon the directio write geometry of the test filesystem.
> loop_file=$TEST_DIR/$seq.loop
> rm -f "$loop_file"
> truncate -s 16M "$loop_file"
> $MKFS_XFS_PROG -f -N "$loop_file" | _filter_mkfs 2>$tmp.mkfs >/dev/null
> . $tmp.mkfs
> seqfull=$0
> _link_out_file "lba${sectsz}"
> 
> What do you think of that approach?

That sound sensible.  Where is that patch, it doesn't seem to be
in the realtime-reflink branch that I'm usually working against.

Re: [PATCH 1/2] xfs/614: query correct direct I/O alignment

[Darrick J. Wong]

On Wed, Feb 05, 2025 at 04:44:22PM +0100, Christoph Hellwig wrote:
> On Tue, Feb 04, 2025 at 09:12:58AM -0800, Darrick J. Wong wrote:
> > Hmmmm... I have a patch with a similar aim in my dev tree that
> > determines the lba size from whatever mkfs decides is the sector size:
> > 
> > # Figure out what sector size mkfs will use to format, which might be dependent
> > # upon the directio write geometry of the test filesystem.
> > loop_file=$TEST_DIR/$seq.loop
> > rm -f "$loop_file"
> > truncate -s 16M "$loop_file"
> > $MKFS_XFS_PROG -f -N "$loop_file" | _filter_mkfs 2>$tmp.mkfs >/dev/null
> > . $tmp.mkfs
> > seqfull=$0
> > _link_out_file "lba${sectsz}"
> > 
> > What do you think of that approach?
> 
> That sound sensible.  Where is that patch, it doesn't seem to be
> in the realtime-reflink branch that I'm usually working against.

It's one of the few zoned changes I have in the fstests branch:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=zoned_2025-02-04

--D

Re: [PATCH 1/2] xfs/614: query correct direct I/O alignment

[Christoph Hellwig]

On Wed, Feb 05, 2025 at 07:51:11AM -0800, Darrick J. Wong wrote:
> It's one of the few zoned changes I have in the fstests branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=zoned_2025-02-04

Oh, you probably want to pull this in to make your life easier:

http://git.infradead.org/?p=users/hch/xfstests-dev.git;a=shortlog;h=refs/heads/xfs-zoned-rebase

Note that the dio size mismatch also happens when using a 4k LBA rt
device with a 512 byte LBA data device without any zoned code.

Re: [PATCH 2/2] xfs/614: remove the _require_loop call

[Zorro Lang]

On Tue, Feb 04, 2025 at 02:46:55PM +0100, Christoph Hellwig wrote:
> This test only creates file system images as regular files, but never
> actually uses the kernel loop driver.  Remove the extra requirement.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

I think this one doesn't depend on the [PATCH 1/2], so I'll
merge this one at first. About the other one, I'll wait you
or Darrick's next patch :)

Thanks,
Zorro

>  tests/xfs/614 | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/xfs/614 b/tests/xfs/614
> index 06cc2384f38c..f2ea99edb342 100755
> --- a/tests/xfs/614
> +++ b/tests/xfs/614
> @@ -22,7 +22,6 @@ _cleanup()
>  
>  
>  _require_test
> -_require_loop
>  $MKFS_XFS_PROG 2>&1 | grep -q concurrency || \
>  	_notrun "mkfs does not support concurrency options"
>  
> -- 
> 2.45.2
>