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.

Reply via email to