Re: [PATCH 1/2] xfs/032: try running on blocksize > pagesize filesystems

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

On Sun, Dec 22, 2024 at 08:44:21PM +0800, Zorro Lang wrote:
> On Tue, Dec 03, 2024 at 07:45:49PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Now that we're no longer limited to blocksize <= pagesize, let's make
> > sure that mkfs, fsstress, and copy work on such things.  This is also a
> > subtle way to get more people running at least one test with that
> > config.
> > 
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> 
> Hi Darrick, sorry for missing this patchset long time :-D

No worries.

> >  tests/xfs/032 |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > 
> > diff --git a/tests/xfs/032 b/tests/xfs/032
> > index 75edf0e9c7268d..52d66ea182d47e 100755
> > --- a/tests/xfs/032
> > +++ b/tests/xfs/032
> > @@ -25,6 +25,17 @@ IMGFILE=$TEST_DIR/${seq}_copy.img
> >  
> >  echo "Silence is golden."
> >  
> > +# Can we mount blocksize > pagesize filesystems?
> > +for ((blocksize = PAGESIZE; blocksize <= 65536; blocksize *= 2)); do
> > +	_scratch_mkfs -b size=$blocksize -d size=1g >> $seqres.full 2>&1 || \
> > +		continue
> > +
> > +	_try_scratch_mount || continue
> > +	mounted_blocksize="$(stat -f -c '%S' $SCRATCH_MNT)"
> 
> _get_block_size $SCRATCH_MNT

Fixed, thanks.

> > +	_scratch_unmount
> > +	test "$blocksize" -eq "$mounted_blocksize" && PAGESIZE=$blocksize
> > +done
> 
> I'm wondering if we can have a helper likes _has_lbs_support(), if it
> returns 0, then set PAGESIZE to 65536 directly? (and we'd better to
> change name of PAGESIZE, e.g. MAX_BLOCKSIZE)

I suppose we could, though how do we detect large block size support?
If it's just mkfs+mount then that's not a lot better than the loop that
exists now.

Another approach might be to change the loop to:

while [ $SECTORSIZE -le 65536 ]; do
	while [ $BLOCKSIZE -le 65536 ]; do
		...
	done
done

But break out of the loop if _scratch_mount fails and BLOCKSIZE >
PAGESIZE?  Then we don't need the detector loop.

--D

> Thanks,
> Zorro
> 
> > +
> >  do_copy()
> >  {
> >  	local opts="$*"
> > 
> 
>