Re: [PATCH] generic: add missing $FSX_AVOID to fsx invocations

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

On Tue, Nov 08, 2022 at 10:44:55AM +0800, Zorro Lang wrote:
> > If it's collapse/insert range you're specifically worried about, perhaps
> > its time to implement _get_file_block_size for ext4 so that
> > _test_congruent_file_oplen can exclude those tests that will get the
> > alignment wrong?
> 
> Thanks Darrick, I'm thinking about this helper you wrote recently :)
> (The real name is _require_congruent_file_oplen in common/rc.)
> 
> Hi Ted, is this helpful if you write a _ext4_get_file_block_size (refer to
> _xfs_get_file_block_size in common/xfs), then call it in _get_file_block_size
> to help _require_congruent_file_oplen to get the correct length which is an
> integer multiple of the file's allocation unit size ?

Well, it's a bit more complicated than that, since there's a
distinction between the cluster size and block size.  The cluster size
is the allocation unit.  However, other things are done in units of
the block size, including how we report fiemap results, etc.

For example, take a look at generic/206.  It will try to create a file
system where the file system block size is the one fourth of the page
size --- so for x86, 1k.  However, for ext4, the default cluster size
for 16 times the block size, so for this file system the allocation
unit size is 16k.  If we make _get_file_block_size return 64k for all
files, then generic/206 will fail:

    pagesz=$(getconf PAGE_SIZE)
    blksz=$((pagesz / 4))
    ...
    _scratch_mkfs_blocksized $blksz > $seqres.full 2>&1
    ...
    real_blksz=$(_get_file_block_size $testdir)
    test $real_blksz != $blksz && _notrun "Failed to format with small blocksize."

I assume this works for xfs because only files in the real-time block
size will have a larger allocation unit size, but the root directory
has a allocation unit size of the "real" block size?

I suppose I could make a hypothetical _ext4_get_file_block_size lie
and return the "real" block size when it's called on the mount point,
but that seems kinda of gross, and it's also a lie, since the
allocation unit size for the root directory actually is the cluster
size, not the block size.

If I were going to be doing things from scratch, I'd make a
distinction between _get_file_allocation_size and
_get_file_system_block_size, which would be a lot *clearer* about what
is going on.  Even then, I could imagine some tests getting confused
with how, say, fiemap behaves with an ext4 file system with a 4k block
size and a 64k allocation unit size, so I'm not sure it's a complete
solution.  And doing this now would require quite a bit of code churn
in xfstests.

> If this's helpful for your first concern [1], please tell me. Then we can talk
> about your second concern [2], if it's still your main concern now :)
> 
> "This might *[1]* either be because the test didn't know about ext4
>  bigalloc's cluster alignment requirements, *[2]* or because a particular
>  operation might just be *buggy* and being able to run tests as if a
>  particular command wasn't supported was useful."

So [1] is a real concern, and at the moment, we just suppress all of
the tests that try to use collapse/insert range.  For example, from
the ext4/bigalloc config file:

    # Until we can teach xfstests the difference between cluster size and
    # block size, avoid collapse_range and insert_range since these will
    # fail due the fact that these operations require cluster-aligned
    # ranges.
    export FSX_AVOID="-C -I"
    export FSSTRESS_AVOID="-f collapse=0 -f insert=0"
    export XFS_IO_AVOID="fcollapse finsert"
    TEST_SET_EXCLUDE="-x collapse,insert"

That's not ideal, and it's been on my todo list to try to fix it, when
I could get one of those round tuit's.  However, I had assumed that we
would split _get_file_block_size somehow, given the observation I've
made above.  So this was always been something that has put me off,
because it looked like a much larger project than say, "gee, I have an
hour or two on a weekend, let me see if I can fix this".

[2] is practically not _really_ a concern any more.  It used to be
that one of the ways that I would root cause a failing test was to
suppress one of the advanced fallocate modes, whether it be
collapse/insert range, or going even further back, punch hole.  If
test then passed, then I could say, "ah, hah; the problem can probably
be localized to a certain part of the fs code."

However, that's mainly tests that are using fsstress and fsx, and we
have solutions for that.  And for other tests, I can examine the test
and see whether or not it's using collapse/insert range by inspection,
so it's really not that big of a deal.  I could imagine other file
systems who might find this useful in the future, if they were trying
to growing support for the more advanced fallocate modes --- but that
wouldn't *my* concern, and arguably those file systems could solve
problems alternate ways, such as having mount options that suppress
those fallocate modes entirely which could be used when running
xfstests.

						- Ted

P.S.  I have noticed some tests that use collapse/insert range but
don't declare that they are in the collapse or insert groups.
Fortunately, all of these tests are also in the clone group, and so
they don't apply to ext4, so _I_ don't care.  :-)