Re: [PATCH] ext4/054,ext4/055: don't run when using DAX

["Theodore Ts'o" <tytso@mit.edu>]

On Thu, Apr 28, 2022 at 01:19:23AM +0800, Zorro Lang wrote:
> I just noticed that _scratch_mkfs_sized() and _scratch_mkfs_blocksized() both use
> _scratch_mkfs_xfs for XFS, I'm wondering if ext4 would like to use _scratch_mkfs_ext4()
> or even use _scratch_mkfs() directly in these two functions. Then you can do something
> likes:
>   MKFS_OPTIONS="$MKFS_OPTIONS -F -O quota"
>   _scratch_mkfs_blocksized 1024
> or:
>   MKFS_OPTIONS="$MKFS_OPTIONS -F -O quota" _scratch_mkfs_blocksized 1024

I'd prefer to keep changing _scratch_mkfs_sized and
_scatch_mkfs_blocksized to use _scratch_mfks_ext4 as a separate
commit.  It makes sense to do that, but it does mean some behavioral
changes; specifically in the external log case,
"_scratch_mkfs_blocksized" will now create a file system using an
external log.  It's probably a good change, but there is some testing
I'd like to do first before makinig that change and I don't have time
for it.

> We just provide a helper to avoid someone forget 'dax', I don't object someone would
> like to "exclude dax" by explicit method :) So if you don't have much time to do this
> change, you can just do what you said above, then I'll take another time/chance to
> change _scratch_mkfs_* things.

Hmm, one thing which I noticed when searching through things.  xfs/432
does this:

_scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1

So in {gce,kvm}-xfstests we have an exclude file entry in
.../fs/xfs/cfg/dax.exclude:

# This test formats a file system with a 1k block size, which is not
# compatible with DAX (at least with systems with a 4k page size).
xfs/432

... in order to suppress a test failure.

Arguably we should add an "_exclude_scratch_mount_option dax" to this
test, as opposed to having an explicit test exclusion in my test
runner.  Or we figure out how to change xfs/432 to use
_scratch_mkfs_blocksized.  So there is a lot of cleanup that can be
done here, and I suspect we should do this work incrementally.  :-)

> Maybe we should think about let all _scratch_mkfs_*[1] helpers use _scratch_mkfs
> consistently. But that will change and affect too many things. I don't want to break
> fundamental code too much, might be better to let each fs help to change and test
> that bit by bit, when they need :)

Yep.   :-)

						- Ted

P.S.  Here's something else that should probably be moved from my test
runner into xfstests.  Again from .../xfs/cfg/dax.exclude:

# mkfs.xfs options which now includes reflink, and reflink is not
# compatible with DAX
xfs/032
xfs/205
xfs/294

Maybe _scratch_mkfs_xfs should be parsing the output of mkfs.xfs to
see if reflink is enabled, and then automatically asserting an
"_exclude_scratch_mount_option dax", perhaps?