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
