On Wed, Mar 02, 2022 at 02:57:00PM -0800, Darrick J. Wong wrote:
> On Wed, Mar 02, 2022 at 03:00:22AM +0530, Nitesh Shetty wrote:
> > At present block size of 1024 is hardcoded for mkfs. This creates
> > problem when device block size is more than 1024. So we use sysfs values
>
> What kinds of problems?
>
I was running the test on NVMe device, which has a LBA of 4096 by default,
so these test fails, not becuase of functionality issues, mainly because of
mkfs failure. mkfs failure is mainly because of assumption of device
LBA is 512 bytes, which is mostly right incase of scsi device.
> > of SCRATCH_DEV, if not found then default to 1024.
> >
> > Signed-off-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx>
> > ---
> > tests/xfs/205 | 3 ++-
> > tests/xfs/432 | 3 ++-
> > tests/xfs/516 | 3 ++-
> > 3 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/xfs/205 b/tests/xfs/205
> > index 104f1f45..73e32c4d 100755
> > --- a/tests/xfs/205
> > +++ b/tests/xfs/205
> > @@ -22,7 +22,8 @@ _require_scratch_nocheck
> > # bitmap consuming all the free space in our small data device.
> > unset SCRATCH_RTDEV
> >
> > -fsblksz=1024
> > +physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
>
> For a disk with 512 byte sectors, does this set physical=512?
>
> The code within the $() really ought to be turned into a helper
> _scratch_dev_phys_block_size or something.
>
Yes it does, you are right, I see a new failure, since minimum block size for CRC
enabled filesystem is 1K. I will make it minimum of 1024 and actual
device LBA and send v2.
> > +fsblksz=${physical:-1024}
>
> Filesystem blocksize isn't supposed to be smaller than the physical
> blocksize, but why do you change the fs block size to the physical size?
>
>
adressed above. next patch will have fsblksz=min(1024, physical blocksize)
> > _scratch_mkfs_xfs -d size=$((32768*fsblksz)) -b size=$fsblksz >> $seqres.full 2>&1
> > _scratch_mount
> >
> > diff --git a/tests/xfs/432 b/tests/xfs/432
> > index 86012f0b..cd6367e2 100755
> > --- a/tests/xfs/432
> > +++ b/tests/xfs/432
> > @@ -49,7 +49,8 @@ echo "Format and mount"
> > # block. 8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
> > # dablock. 33 dirblocks * 64k mean that we can expand a directory by
> > # 2112k before we have to allocate another da btree block.
> > -_scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
> > +physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
> > +_scratch_mkfs -b size=${physical:-1k} -n size=64k > "$seqres.full" 2>&1
>
> This test formats a very specific geometry because it needs precise
> calculations to generate a directory with 1000 consecutively mapped
> blocks. Does it still do that if the blocksize isn't 1k?
>
yes, agree its geometric specific. But with above change test exits as
not run, instead of failure. Reason being unable to create >1000 data
block extents.
Yes, you are right about needing precise calculation to generate
that 1000 data block. My knowledge on file system is limited at present.
Does it makes sense to put those tests which run with 512 bytes
assumption to not_run state, instead of fail for 4096 block size
devices, as a temporary fix ? or should I just simply report them so
that persons with fs expertise fix it ?
> > _scratch_mount >> "$seqres.full" 2>&1
> >
> > metadump_file="$TEST_DIR/meta-$seq"
> > diff --git a/tests/xfs/516 b/tests/xfs/516
> > index 9e1b9931..b9d4f0c9 100755
> > --- a/tests/xfs/516
> > +++ b/tests/xfs/516
> > @@ -84,7 +84,8 @@ test_su_opts()
> > local mounted=0
> >
> > echo "Format with 256k stripe unit; 4x stripe width" >> $seqres.full
> > - _scratch_mkfs -b size=1k -d su=256k,sw=4 >> $seqres.full 2>&1
> > + physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
> > + _scratch_mkfs -b size=${physical:-1k} -d su=256k,sw=4 >> $seqres.full 2>&1
> This is a test of sunit/swidth. Do you need to scale those up as well?
>
Agreed, they should be scaled.
> --D
>
> >
> > __test_mount_opts "$@"
> > }
> >
> > base-commit: 2ea74ba4e70b546279896e2a733c8c7f4b206193
> > --
> > 2.25.1
> >
>
--
Nitesh Shetty
