[PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[Johannes Thumshirn]

In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
we're submitting a DIO write on a zoned filesystem but are not using
REQ_OP_ZONE_APPEND to submit the IO to the block device.

This is a left over from a previous version where btrfs_dio_iomap_begin()
didn't use btrfs_use_zone_append() to check for sequential write only
zones.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/inode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 618ec195985b..288c7ce63a32 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8179,10 +8179,6 @@ static blk_qc_t btrfs_submit_direct(struct inode *inode, struct iomap *iomap,
 		bio->bi_end_io = btrfs_end_dio_bio;
 		btrfs_io_bio(bio)->logical = file_offset;
 
-		WARN_ON_ONCE(write && btrfs_is_zoned(fs_info) &&
-			     fs_info->max_zone_append_size &&
-			     bio_op(bio) != REQ_OP_ZONE_APPEND);
-
 		if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
 			status = extract_ordered_extent(BTRFS_I(inode), bio,
 							file_offset);
-- 
2.30.0

Re: [PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[David Sterba]

On Wed, Mar 17, 2021 at 05:57:31PM +0900, Johannes Thumshirn wrote:
> In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
> we're submitting a DIO write on a zoned filesystem but are not using
> REQ_OP_ZONE_APPEND to submit the IO to the block device.
> 
> This is a left over from a previous version where btrfs_dio_iomap_begin()
> didn't use btrfs_use_zone_append() to check for sequential write only
> zones.

I can't identify the patch where this got changed. I've landed on
544d24f9de73 ("btrfs: zoned: enable zone append writing for direct IO")
but that adds the btrfs_use_zone_append, the append flag and also the
warning.

Re: [PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[Johannes Thumshirn]

On 17/03/2021 11:54, David Sterba wrote:
> On Wed, Mar 17, 2021 at 05:57:31PM +0900, Johannes Thumshirn wrote:
>> In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
>> we're submitting a DIO write on a zoned filesystem but are not using
>> REQ_OP_ZONE_APPEND to submit the IO to the block device.
>>
>> This is a left over from a previous version where btrfs_dio_iomap_begin()
>> didn't use btrfs_use_zone_append() to check for sequential write only
>> zones.
> 
> I can't identify the patch where this got changed. I've landed on
> 544d24f9de73 ("btrfs: zoned: enable zone append writing for direct IO")
> but that adds the btrfs_use_zone_append, the append flag and also the
> warning.
> 

It is an oversight from the development phase. In v11 (I think) I've added
08f455593fff ("btrfs: zoned: cache if block group is on a sequential zone")
and forgot to remove the WARN_ON_ONCE() for 544d24f9de73 ("btrfs: zoned: 
enable zone append writing for direct IO").

When developing auto relocation I got hit by the WARN as a block groups
where relocated to conventional zone and the dio code calls
btrfs_use_zone_append() introduced by 08f455593fff to check if it can use
zone append (a.k.a. if it's a sequential zone) or not and sets the appropriate
flags for iomap.

I've never hit it in testing before, as I was relying on emulation to test
the conventional zones code but this one case wasn't hit, because on emulation
fs_info->max_zone_append_size is 0 and the WARN doesn't trigger either.

Re: [PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[Johannes Thumshirn]

On 17/03/2021 14:20, Johannes Thumshirn wrote:
> On 17/03/2021 11:54, David Sterba wrote:
>> On Wed, Mar 17, 2021 at 05:57:31PM +0900, Johannes Thumshirn wrote:
>>> In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
>>> we're submitting a DIO write on a zoned filesystem but are not using
>>> REQ_OP_ZONE_APPEND to submit the IO to the block device.
>>>
>>> This is a left over from a previous version where btrfs_dio_iomap_begin()
>>> didn't use btrfs_use_zone_append() to check for sequential write only
>>> zones.
>>
>> I can't identify the patch where this got changed. I've landed on
>> 544d24f9de73 ("btrfs: zoned: enable zone append writing for direct IO")
>> but that adds the btrfs_use_zone_append, the append flag and also the
>> warning.
>>
> 
> It is an oversight from the development phase. In v11 (I think) I've added
> 08f455593fff ("btrfs: zoned: cache if block group is on a sequential zone")
> and forgot to remove the WARN_ON_ONCE() for 544d24f9de73 ("btrfs: zoned: 
> enable zone append writing for direct IO").
> 
> When developing auto relocation I got hit by the WARN as a block groups
> where relocated to conventional zone and the dio code calls
> btrfs_use_zone_append() introduced by 08f455593fff to check if it can use
> zone append (a.k.a. if it's a sequential zone) or not and sets the appropriate
> flags for iomap.
> 
> I've never hit it in testing before, as I was relying on emulation to test
> the conventional zones code but this one case wasn't hit, because on emulation
> fs_info->max_zone_append_size is 0 and the WARN doesn't trigger either.
> 

I just realized that explanation should have gone into the commit message, do you
want me to resend with a more elaborate commit message?

Re: [PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[David Sterba]

On Wed, Mar 17, 2021 at 01:22:11PM +0000, Johannes Thumshirn wrote:
> On 17/03/2021 14:20, Johannes Thumshirn wrote:
> > On 17/03/2021 11:54, David Sterba wrote:
> >> On Wed, Mar 17, 2021 at 05:57:31PM +0900, Johannes Thumshirn wrote:
> >>> In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
> >>> we're submitting a DIO write on a zoned filesystem but are not using
> >>> REQ_OP_ZONE_APPEND to submit the IO to the block device.
> >>>
> >>> This is a left over from a previous version where btrfs_dio_iomap_begin()
> >>> didn't use btrfs_use_zone_append() to check for sequential write only
> >>> zones.
> >>
> >> I can't identify the patch where this got changed. I've landed on
> >> 544d24f9de73 ("btrfs: zoned: enable zone append writing for direct IO")
> >> but that adds the btrfs_use_zone_append, the append flag and also the
> >> warning.
> >>
> > 
> > It is an oversight from the development phase. In v11 (I think) I've added
> > 08f455593fff ("btrfs: zoned: cache if block group is on a sequential zone")
> > and forgot to remove the WARN_ON_ONCE() for 544d24f9de73 ("btrfs: zoned: 
> > enable zone append writing for direct IO").
> > 
> > When developing auto relocation I got hit by the WARN as a block groups
> > where relocated to conventional zone and the dio code calls
> > btrfs_use_zone_append() introduced by 08f455593fff to check if it can use
> > zone append (a.k.a. if it's a sequential zone) or not and sets the appropriate
> > flags for iomap.
> > 
> > I've never hit it in testing before, as I was relying on emulation to test
> > the conventional zones code but this one case wasn't hit, because on emulation
> > fs_info->max_zone_append_size is 0 and the WARN doesn't trigger either.
> 
> I just realized that explanation should have gone into the commit message, do you
> want me to resend with a more elaborate commit message?

I'll append the explanation above to the changelog, no need to resend
unless you want to add/adjust something. Thanks.

Re: [PATCH] btrfs: zoned: remove outdated WARN_ON in direct IO

[Johannes Thumshirn]

On 17/03/2021 15:29, David Sterba wrote:
> On Wed, Mar 17, 2021 at 01:22:11PM +0000, Johannes Thumshirn wrote:
>> On 17/03/2021 14:20, Johannes Thumshirn wrote:
>>> On 17/03/2021 11:54, David Sterba wrote:
>>>> On Wed, Mar 17, 2021 at 05:57:31PM +0900, Johannes Thumshirn wrote:
>>>>> In btrfs_submit_direct() there's a WAN_ON_ONCE() that will trigger if
>>>>> we're submitting a DIO write on a zoned filesystem but are not using
>>>>> REQ_OP_ZONE_APPEND to submit the IO to the block device.
>>>>>
>>>>> This is a left over from a previous version where btrfs_dio_iomap_begin()
>>>>> didn't use btrfs_use_zone_append() to check for sequential write only
>>>>> zones.
>>>>
>>>> I can't identify the patch where this got changed. I've landed on
>>>> 544d24f9de73 ("btrfs: zoned: enable zone append writing for direct IO")
>>>> but that adds the btrfs_use_zone_append, the append flag and also the
>>>> warning.
>>>>
>>>
>>> It is an oversight from the development phase. In v11 (I think) I've added
>>> 08f455593fff ("btrfs: zoned: cache if block group is on a sequential zone")
>>> and forgot to remove the WARN_ON_ONCE() for 544d24f9de73 ("btrfs: zoned: 
>>> enable zone append writing for direct IO").
>>>
>>> When developing auto relocation I got hit by the WARN as a block groups
>>> where relocated to conventional zone and the dio code calls
>>> btrfs_use_zone_append() introduced by 08f455593fff to check if it can use
>>> zone append (a.k.a. if it's a sequential zone) or not and sets the appropriate
>>> flags for iomap.
>>>
>>> I've never hit it in testing before, as I was relying on emulation to test
>>> the conventional zones code but this one case wasn't hit, because on emulation
>>> fs_info->max_zone_append_size is 0 and the WARN doesn't trigger either.
>>
>> I just realized that explanation should have gone into the commit message, do you
>> want me to resend with a more elaborate commit message?
> 
> I'll append the explanation above to the changelog, no need to resend
> unless you want to add/adjust something. Thanks.
> 

Thanks