On Feb 08, 2022 / 16:57, Darrick J. Wong wrote: > On Mon, Feb 07, 2022 at 12:09:58PM +0900, Shin'ichiro Kawasaki wrote: > > The test case generic/204 formats the scratch device to get block size > > as a part of preparation. However, this preparation is required only for > > xfs. To simplify preparation for other filesystems, do the preparation > > only for xfs. > > > > Suggested-by: Naohiro Aota <naohiro.aota@xxxxxxx> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > > --- > > tests/generic/204 | 23 ++++++++++++++--------- > > 1 file changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/tests/generic/204 b/tests/generic/204 > > index 40d524d1..ea267760 100755 > > --- a/tests/generic/204 > > +++ b/tests/generic/204 > > @@ -16,17 +16,18 @@ _cleanup() > > sync > > } > > > > -# Import common functions. > > -. ./common/filter > > - > > # real QA test starts here > > _supported_fs generic > > > > _require_scratch > > > > -# get the block size first > > -_scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > -. $tmp.mkfs > > +dbsize=4096 > > +isize=256 > > +if [ $FSTYP = "xfs" ]; then > > + # get the block size first > > + _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + . $tmp.mkfs > > +fi > > > > # For xfs, we need to handle the different default log sizes that different > > # versions of mkfs create. All should be valid with a 16MB log, so use that. > > @@ -37,11 +38,15 @@ _scratch_mkfs 2> /dev/null | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > SIZE=`expr 115 \* 1024 \* 1024` > > _scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw \ > > || _fail "mkfs failed" Hi Darrick, thank you for valuable review comments. > > Getting back to why generic/204 calls mkfs twice -- > > The first time is to get dbsize (aka the data block size) and the inode > size. AFAICT neither mkfs call add any mkfs options that would change > the block size, so I wonder why we need the first call at all? Oh, this question looks valid. I think both _scratch_mkfs and _scratch_mkfs_sized handle block size and i-node size options in MKFS_OPTIONS. So the first call does not look required. > > Would the following work for g/204, assuming that _filter_mkfs some day > provides correct output for dbsize and isize? Thank you for this idea. As you suggested, I removed the first _filter_mkfs call and $dbsize option for _scratch_mkfs_sized. With this change, I tried some MKFS_OPTIONS variations for mkfs.xfs (-b size=X, -i size=Y), and observed the test condition of g/204 ($files) is preserved. Will revise the series with this fix approach. > > _require_scratch > > # For xfs, we need to handle the different default log sizes that > # different versions of mkfs create. All should be valid with a 16MB > # log, so use that. And v4/512 v5/1k xfs don't have enough free inodes, > # set imaxpct=50 at mkfs time solves this problem. > [ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50" > > SIZE=`expr 115 \* 1024 \* 1024` > _scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw || \ > _fail "mkfs failed" > > cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null > _scratch_mount > > # Source $tmp.mkfs to get geometry > . $tmp.mkfs > > --D > > > -cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + > > +if [ $FSTYP = "xfs" ]; then > > + cat $tmp.mkfs.raw | _xfs_filter_mkfs 2> $tmp.mkfs > /dev/null > > + # Source $tmp.mkfs to get geometry > > + . $tmp.mkfs > > +fi > > + > > _scratch_mount > > > > -# Source $tmp.mkfs to get geometry > > -. $tmp.mkfs > > > > # fix the reserve block pool to a known size so that the enospc calculations > > # work out correctly. Space usages is based 22500 files and 1024 reserved blocks > > -- > > 2.34.1 > > -- Best Regards, Shin'ichiro Kawasaki
