* [PATCH] fstests: btrfs/226: fill in missing comments changes
@ 2025-02-18 22:35 Anand Jain
2025-02-19 5:58 ` Zorro Lang
2025-02-21 12:04 ` Filipe Manana
0 siblings, 2 replies; 9+ messages in thread
From: Anand Jain @ 2025-02-18 22:35 UTC (permalink / raw)
To: zlang, fstests; +Cc: linux-btrfs
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.
_scratch_mount -o nodatasum
# Test a write against COW file/extent - should fail with -EAGAIN. Disable the
--
2.47.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-18 22:35 [PATCH] fstests: btrfs/226: fill in missing comments changes Anand Jain
@ 2025-02-19 5:58 ` Zorro Lang
2025-02-21 12:04 ` Filipe Manana
1 sibling, 0 replies; 9+ messages in thread
From: Zorro Lang @ 2025-02-19 5:58 UTC (permalink / raw)
To: Anand Jain; +Cc: zlang, fstests, linux-btrfs
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
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-18 22:35 [PATCH] fstests: btrfs/226: fill in missing comments changes Anand Jain
2025-02-19 5:58 ` Zorro Lang
@ 2025-02-21 12:04 ` Filipe Manana
2025-02-21 15:03 ` Zorro Lang
2025-02-23 22:30 ` [PATCH v2] " Anand Jain
1 sibling, 2 replies; 9+ messages in thread
From: Filipe Manana @ 2025-02-21 12:04 UTC (permalink / raw)
To: Anand Jain; +Cc: zlang, fstests, linux-btrfs
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
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-21 12:04 ` Filipe Manana
@ 2025-02-21 15:03 ` Zorro Lang
2025-02-22 11:16 ` Anand Jain
2025-02-23 22:31 ` Anand Jain
2025-02-23 22:30 ` [PATCH v2] " Anand Jain
1 sibling, 2 replies; 9+ messages in thread
From: Zorro Lang @ 2025-02-21 15:03 UTC (permalink / raw)
To: Anand Jain; +Cc: zlang, fstests, linux-btrfs, Filipe Manana
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
> >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-21 15:03 ` Zorro Lang
@ 2025-02-22 11:16 ` Anand Jain
2025-02-22 11:29 ` Filipe Manana
2025-02-23 22:31 ` Anand Jain
1 sibling, 1 reply; 9+ messages in thread
From: Anand Jain @ 2025-02-22 11:16 UTC (permalink / raw)
To: Filipe Manana; +Cc: zlang, fstests, linux-btrfs, Zorro Lang
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@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.
>
Your sentence missed the consequence part (checksum mismatches) that
Qu's sentence included.
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).
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
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-22 11:16 ` Anand Jain
@ 2025-02-22 11:29 ` Filipe Manana
2025-02-23 22:30 ` Anand Jain
0 siblings, 1 reply; 9+ messages in thread
From: Filipe Manana @ 2025-02-22 11:29 UTC (permalink / raw)
To: Anand Jain; +Cc: zlang, fstests, linux-btrfs, Zorro Lang
On Sat, Feb 22, 2025 at 11:17 AM Anand Jain <anand.jain@oracle.com> 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@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.
> >
>
> 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
> >>>
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-22 11:29 ` Filipe Manana
@ 2025-02-23 22:30 ` Anand Jain
0 siblings, 0 replies; 9+ messages in thread
From: Anand Jain @ 2025-02-23 22:30 UTC (permalink / raw)
To: Filipe Manana; +Cc: zlang, fstests, linux-btrfs, Zorro Lang
On 22/2/25 19:29, Filipe Manana wrote:
> On Sat, Feb 22, 2025 at 11:17 AM Anand Jain <anand.jain@oracle.com> 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@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.
>>>
>>
>> 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.
Done. Thx.
Anand
>
>>
>> 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
>>>>>
>>>>>
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] fstests: btrfs/226: fill in missing comments changes
2025-02-21 12:04 ` Filipe Manana
2025-02-21 15:03 ` Zorro Lang
@ 2025-02-23 22:30 ` Anand Jain
1 sibling, 0 replies; 9+ messages in thread
From: Anand Jain @ 2025-02-23 22:30 UTC (permalink / raw)
To: fstests, fdmanana; +Cc: zlang, linux-btrfs
From: Qu Wenruo <wqu@suse.com>
Adds the comments that were previously missed.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
v2: fix comments - drop reason for the nodatasum requisite.
tests/btrfs/226 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/btrfs/226 b/tests/btrfs/226
index 359813c4f394..afab5868b224 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 only works with direct IO, which requires an inode with
+# nodatasum (otherwise it falls back to buffered IO).
_scratch_mount -o nodatasum
# Test a write against COW file/extent - should fail with -EAGAIN. Disable the
--
2.47.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fstests: btrfs/226: fill in missing comments changes
2025-02-21 15:03 ` Zorro Lang
2025-02-22 11:16 ` Anand Jain
@ 2025-02-23 22:31 ` Anand Jain
1 sibling, 0 replies; 9+ messages in thread
From: Anand Jain @ 2025-02-23 22:31 UTC (permalink / raw)
To: Zorro Lang; +Cc: zlang, fstests, linux-btrfs, Filipe Manana
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@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 :)
>
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
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-02-23 22:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-18 22:35 [PATCH] fstests: btrfs/226: fill in missing comments changes Anand Jain
2025-02-19 5:58 ` Zorro Lang
2025-02-21 12:04 ` Filipe Manana
2025-02-21 15:03 ` Zorro Lang
2025-02-22 11:16 ` Anand Jain
2025-02-22 11:29 ` Filipe Manana
2025-02-23 22:30 ` Anand Jain
2025-02-23 22:31 ` Anand Jain
2025-02-23 22:30 ` [PATCH v2] " Anand Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox