| Message ID | e73cfe5310a8cee5f6c709d54b8c18ff52e39a0a.1739918100.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fstests: btrfs/226: fill in missing comments changes | expand |
On Wed, Feb 19, 2025 at 06:35:44AM +0800, Anand Jain wrote: > From: Qu Wenruo <wqu@suse.com> > > Update comments that were previously missed. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > 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. As a supplement patch, this is good to me. Reviewed-by: Zorro Lang <zlang@redhat.com> > _scratch_mount -o nodatasum > > # Test a write against COW file/extent - should fail with -EAGAIN. Disable the > -- > 2.47.0 > >
On Tue, Feb 18, 2025 at 10:36 PM Anand Jain <anand.jain@oracle.com> wrote: > > From: Qu Wenruo <wqu@suse.com> > > Update comments that were previously missed. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > 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@oracle.com/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. > _scratch_mount -o nodatasum > > # Test a write against COW file/extent - should fail with -EAGAIN. Disable the > -- > 2.47.0 > >
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@oracle.com> wrote: > > > > From: Qu Wenruo <wqu@suse.com> > > > > Update comments that were previously missed. > > > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > > --- > > 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@oracle.com/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 :) Thanks, Zorro > > > > > _scratch_mount -o nodatasum > > > > # Test a write against COW file/extent - should fail with -EAGAIN. Disable the > > -- > > 2.47.0 > > > > >
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. _scratch_mount -o nodatasum # Test a write against COW file/extent - should fail with -EAGAIN. Disable the