Re: [PATCH 3/4] xfs/259: try to force loop device block size

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, May 22, 2025 at 11:21:26AM +1000, Dave Chinner wrote:
> On Wed, May 21, 2025 at 03:41:36PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Starting with 6.15-rc1, loop devices created with directio mode enabled
> > will set their logical block size to whatever STATX_DIO_ALIGN on the
> > host filesystem reports.  If you happen to be running a kernel that
> > always sets up loop devices in directio mode and TEST_DEV is a block
> > device with 4k sectors, this will cause conflicts with this test's usage
> > of mkfs with different block sizes.  Add a helper to force the loop
> > device block size to 512 bytes, which is implied by scenarios such as
> > "device size is 4T - 2048 bytes".
> > 
> > Also fix xfs/078 which simply needs the blocksize to be set.
> > 
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  common/rc         |   22 ++++++++++++++++++++++
> >  tests/generic/563 |    1 +
> >  tests/xfs/078     |    2 ++
> >  tests/xfs/259     |    1 +
> >  tests/xfs/613     |    1 +
> >  5 files changed, 27 insertions(+)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index 657772e73db86b..4e3917a298e072 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -4526,6 +4526,28 @@ _create_loop_device()
> >  	echo $dev
> >  }
> >  
> > +# Configure the loop device however needed to support the given block size.
> > +_force_loop_device_blocksize()
> > +{
> > +	local loopdev="$1"
> > +	local blksize="$2"
> > +	local is_dio
> > +	local logsec
> > +
> > +	if [ ! -b "$loopdev" ] || [ -z "$blksize" ]; then
> > +		echo "_force_loop_device_blocksize requires loopdev and blksize" >&2
> > +		return 1
> > +	fi
> > +
> > +	curr_blksize="$(losetup --list --output LOG-SEC --noheadings --raw "$loopdev")"
> > +	if [ "$curr_blksize" -gt "$blksize" ]; then
> > +		losetup --direct-io=off "$loopdev"
> > +		losetup --sector-size "$blksize" "$loopdev"
> > +	fi
> > +
> > +	#losetup --raw --list "$loopdev" >> $seqres.full
> > +}
> 
> I think it would make more sense to use a
> _create_loop_device_blocksize() wrapper function and change the call
> sites to use it that to add this function that requires error
> checking of the parameters even though it is only called directly
> after loop device creation.
> 
> _create_loop_device_blocksize()
> {
> 	local file=$1
> 	local blksize=$2
> 
> 	dev=`losetup -f --show $file --sector-size=$blksize`
> 
> 	# If the loop device sector size is incompatible with doing
> 	# direct IO on the backing file, attempting to turn on
> 	# direct-io will fail with an -EINVAL error. However, the
> 	# device will still work correctly using buffered IO, so we
> 	# ignore the error.
> 	test -b "$dev" && losetup --direct-io=on $dev 2> /dev/null
> 	echo $dev

Yeah, I guess that works too.

--D

> }
> 
> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
>