Re: [RFC PATCH 2/2] libfs: Improve behavior when directory offset values wrap

[Chuck Lever III <chuck.lever@oracle.com>]

> On Nov 22, 2024, at 3:49 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> 
> On Thu, Nov 21, 2024 at 10:30 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>> On Thu, Nov 21, 2024 at 01:18:05PM -0800, Hugh Dickins wrote:
>>> On Thu, 21 Nov 2024, Chuck Lever III wrote:
>>>> 
>>>> I will note that tmpfs hangs during generic/449 for me 100%
>>>> of the time; the failure appears unrelated to renames. Do you
>>>> know if there is regular CI being done for tmpfs? I'm planning
>>>> to add it to my nightly test rig once I'm done here.
>>> 
>>> For me generic/449 did not hang, just took a long time to discover
>>> something uninteresting and eventually declare "not run".  Took
>>> 14 minutes six years ago, when I gave up on it and short-circuited
>>> the "not run" with the patch below.
>>> 
>>> (I carry about twenty patches for my own tmpfs fstests testing; but
>>> many of those are just for ancient 32-bit environment, or to suit the
>>> "huge=always" option. I never have enough time/priority to review and
>>> post them, but can send you a tarball if they might of use to you.)
>>> 
>>> generic/449 is one of those tests which expects metadata to occupy
>>> space inside the "disk", in a way which it does not on tmpfs (and a
>>> quick glance at its history suggests btrfs also had issues with it).
>>> 
>>> [PATCH] generic/449: not run on tmpfs earlier
>>> 
>>> Do not waste 14 minutes to discover that tmpfs succeeds in
>>> setting acls despite running out of space for user attrs.
>>> 
>>> Signed-off-by: Hugh Dickins <hughd@google.com>
>>> ---
>>> tests/generic/449 | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/tests/generic/449 b/tests/generic/449
>>> index 9cf814ad..a52a992b 100755
>>> --- a/tests/generic/449
>>> +++ b/tests/generic/449
>>> @@ -22,6 +22,11 @@ _require_test
>>> _require_acls
>>> _require_attrs trusted
>>> 
>>> +if [ "$FSTYP" = "tmpfs" ]; then
>>> +     # Do not waste 14 minutes to discover this:
>>> +     _notrun "$FSTYP succeeds in setting acls despite running out of space for user attrs"
>>> +fi
>>> +
>>> _scratch_mkfs_sized $((256 * 1024 * 1024)) >> $seqres.full 2>&1
>>> _scratch_mount || _fail "mount failed"
>>> 
>>> --
>>> 2.35.3
>> 
>> My approach (until I could look into the failure more) has been
>> similar:
>> 
>> diff --git a/tests/generic/449 b/tests/generic/449
>> index 9cf814ad326c..8307a43ce87f 100755
>> --- a/tests/generic/449
>> +++ b/tests/generic/449
>> @@ -21,6 +21,7 @@ _require_scratch
>> _require_test
>> _require_acls
>> _require_attrs trusted
>> +_supported_fs ^nfs ^overlay ^tmpfs
>> 
> 
> nfs and overlay are _notrun because they do not support _scratch_mkfs_sized
> 
>> _scratch_mkfs_sized $((256 * 1024 * 1024)) >> $seqres.full 2>&1
>> _scratch_mount || _fail "mount failed"
>> 
>> 
>> I stole it from somewhere else, so it's not tmpfs-specific.
> 
> I think opt-out for a certain fs makes sense in some tests, but it is
> prefered to describe the requirement that is behind the opt-out.
> 
> For example, you thought that nfs,overlay,tmpfs should all opt-out
> from this test. Why? Which property do they share in common and
> how can it be described in a generic way?
> 
> I am not talking about a property that can be checked.
> Sometimes we need to make groups of filesystems that share a common
> property that cannot be tested, to better express the requirements.
> 
> _fstyp_has_non_default_seek_data_hole() is the only example that
> comes to mind but there could be others.

Adding a rationale is sensible. I don't have one, but
here is the limit of my thinking:

"^nfs" is in this mix because I test NFSD with a tmpfs
export semi-regularly, and it suffers from the same
problem.

"^overlay" doesn't have any reason to be here except
that it was part of the line I stole from elsewhere.

But the top-level question is "does it make sense to
exclude tmpfs from generic/449, or is there something
that should be fixed to make it pass quickly?"


--
Chuck Lever