On Sat, Feb 22, 2025 at 11:17 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote: > > > > On 21/2/25 23:03, Zorro Lang wrote: > > On Fri, Feb 21, 2025 at 12:04:32PM +0000, Filipe Manana wrote: > >> On Tue, Feb 18, 2025 at 10:36 PM Anand Jain <anand.jain@xxxxxxxxxx> wrote: > >>> > >>> From: Qu Wenruo <wqu@xxxxxxxx> > >>> > >>> Update comments that were previously missed. > >>> > >>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx> > >>> --- > >>> tests/btrfs/226 | 6 ++---- > >>> 1 file changed, 2 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/tests/btrfs/226 b/tests/btrfs/226 > >>> index 359813c4f394..ce53b7d48c49 100755 > >>> --- a/tests/btrfs/226 > >>> +++ b/tests/btrfs/226 > >>> @@ -22,10 +22,8 @@ _require_xfs_io_command fpunch > >>> > >>> _scratch_mkfs >>$seqres.full 2>&1 > >>> > >>> -# This test involves RWF_NOWAIT direct IOs, but for inodes with data checksum, > >>> -# btrfs will fall back to buffered IO unconditionally to prevent data checksum > >>> -# mimsatch, and that will break RWF_NOWAIT with -EAGAIN. > >>> -# So here we have to go with nodatasum mount option. > >>> +# RWF_NOWAIT works only with direct I/O and requires an inode with nodatasum > >>> +# to avoid checksum mismatches. Otherwise, it falls back to buffered I/O. > >> > >> Btw, this is different from what I suggested before here: > >> > >> https://lore.kernel.org/fstests/68aa436b-4ddd-4ee7-ad5a-8eca55aae176@xxxxxxxxxx/T/#mb2369802d2e33c9778c62fcb3c0ee47de28b773b > >> > >> Which is: > >> > >> # RWF_NOWAIT only works with direct IO, which requires an inode with > >> nodatasum (otherwise it falls back to buffered IO). > >> > >> What is being added in this patch: > >> > >> +# RWF_NOWAIT works only with direct I/O and requires an inode with nodatasum > >> +# to avoid checksum mismatches. Otherwise, it falls back to buffered I/O. > >> > >> Is confusing because: > >> > >> 1) It gives the suggestion RWF_NOWAIT requires nodatasum. > >> > >> 2) The part that says "to avoid checksum mismatches", that's not > >> related to RWF_NOWAIT at all. > >> That's the reason why direct IO writes against inodes without > >> nodatasum fallback to buffered IO. > >> We don't have to explain that - this is not a test to exercise the > >> fallback after all, all we have to say > >> is that RWF_NOWAIT needs direct IO and direct IO can only be done > >> against inodes with nodatasum. > >> > >> So you didn't pick my suggestion after all, you just added your own > >> rephrasing which IMO is confusing. > > > > Your sentence missed the consequence part (checksum mismatches) that > Qu's sentence included. And that's totally irrelevant to this test case. Preventing checksum mismatches is why direct IO writes fallback to buffered IO if the inode doesn't have the nodatasum flag - it has nothing to do with RWF_NOWAIT writes. Besides that, such mismatches only happen for cases where an app writes to the write buffer while the direct IO write is in progress - which is not the case of this test case. > > How about, > > # RWF_NOWAIT only works with direct IO, which requires an inode with > nodatasum to avoid checksum-mismatches (otherwise it falls back to > buffered IO). Just stick it to the original - simple and to the point. Thanks. > > Thx, Anand > > > Hi Anand, please talk with Filipe (or more btrfs folks) and make a final > > decision about how to write this comment. I'll drop this patch from > > patches-in-queue branch temporarily, until you reach a consensus :) > > > > Thanks, > > Zorro > > > >> > >> > >> > >>> _scratch_mount -o nodatasum > >>> > >>> # Test a write against COW file/extent - should fail with -EAGAIN. Disable the > >>> -- > >>> 2.47.0 > >>> > >>> > >> > > >
