Re: [PATCH] generic: test concurrent direct IO writes and fsync using same fd

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/9/3 19:11, Filipe Manana 写道:
> On Tue, Sep 3, 2024 at 5:09 AM Zorro Lang <zlang@redhat.com> wrote:
[...]
>>
>> Oh, so this test depends on CONFIG_BTRFS_ASSERT=y, or it always test passed? I tested
>> on general kernel of fedora 42, which doesn't enable this config.
>>
>> There's a _require_xfs_debug helper in common/xfs (and _require_no_xfs_debug
>> too), does btrfs have similar method to check that?
>
> No, we don't.
>
> I don't think we should make the test run only if the kernel config
> has CONFIG_BTRFS_ASSERT=y.
>
> This particular regression is easily detected with
> CONFIG_BTRFS_ASSERT=y, and btrfs developers and the CI we use have
> that setting enabled.
>
> The fact that this regression happened shows that fstests didn't have
> this type of scenario covered, which happens in practice with qemu's
> workload (from two user reports).
>
> I'm adding the test not just to prove the assertion can be triggered
> for this particular regression, but also to make sure no other types
> of regressions happen in the future which may be unrelated to the
> reason for the current regression.

I agree with Filipe, yes I know it's frustrating if one can not
reproduce the bug.

But considering filesystems can have different configs to expose
different behavior, limiting the test case for certain configs doesn't
really improve coverage.

And I believe for most fs development teams, we all have different
kernel configs for prod and develop coverage.

For this particular case, I believe just mentioning what is needed to
trigger the original failure (and what the original symptom) is good enough.
Although I guess that's what is missing and caused the initial confusion.

So mind to slightly update the commit message to mention the needed
config to trigger the original bug?

With such a small update, it should help us to communicate with other
people who mostly works out of our realm.

Thanks,
Qu
>
> Hope that makes sense.
>
>>
>> Thanks,
>> Zorro
>>

>