On Fri, Mar 22, 2024 at 11:08:00AM -0400, Josef Bacik wrote: > 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, Frankly I'd have been ok with a per-test exclusion with some documentation: test $FSTYP = btrfs && \ _notrun 'FIXME: btrfs does not support concurrent read+ficlone' There's nothing in the FICLONE spec about implementations needing to support concurrent reads and clones on the source file. --D > Josef >