Re: [PATCH] fstests: btrfs/226: fill in missing comments changes

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





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.

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 :)


pls pull for-next. (force updated).

Thx. Anand

Thanks,
Zorro




  _scratch_mount -o nodatasum

  # Test a write against COW file/extent - should fail with -EAGAIN. Disable the
--
2.47.0









[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux