Re: [PATCH 5/5] generic/733: disable for btrfs

[Josef Bacik <josef@toxicpanda.com>]

On Thu, Mar 21, 2024 at 05:52:33PM -0400, Kent Overstreet wrote:
> On Thu, Mar 21, 2024 at 02:36:47PM -0700, Christoph Hellwig wrote:
> > On Wed, Mar 20, 2024 at 04:58:24PM +0100, David Sterba wrote:
> > > It is a bug in the test, it should have been xfs specific and never
> > > promoted to generic/ and not affect btrfs unless explained how
> > > in the first place.
> > 
> > Well, the concept that reflinks are reasonably fast and are not live
> > locked by ongoing I/O seems pretty natural.  But I guess we can't
> > just asusme quality of implementation everywhere, and do an opt-in
> > like _require_non_sucky_reflink.  Either way we need to document
> > the assumptions and not add a magic exclude for a single fs.
> 
> generic/733 runs just fine on bcachefs, sounds like btrfs folks just
> need to fix their filesystem

Neither of these comments are particularly helpful or relevant to the
conversation.

Btrfs range locks the extent during the clone operation, and also range locks
the area that it reads.  This doesn't make it "sucky" or "worse", simply
different.  I'm not entirely sure why protecting a range of extents that's
currently being modified is considered "bad" or "broken".

In any case, I can accept that we need to have a different option for skipping
this test, but this is sort of an argument for the shared/ directory or some
other mechanism, or at the very least validating the test passes on all the
major file systems before including it as a generic test.

Adding an opt in _require feels like the best option, or we can simply keep
excluding it in our own fstests branch and ignore the upstream branch.  I'm good
with either solution.  Thanks,

Josef