Re: [PATCH] block: virtio-blk: Fix handling of zone append command completion

[Damien Le Moal <dlemoal@kernel.org>]

On 6/23/23 07:19, Michael S. Tsirkin wrote:
> On Fri, Jun 23, 2023 at 06:55:24AM +0900, Damien Le Moal wrote:
>> On 6/22/23 23:32, Suwan Kim wrote:
>>> On Tue, Jun 20, 2023 at 5:39 PM Damien Le Moal <dlemoal@kernel.org> wrote:
>>>>
>>>> The introduction of completion batching with commit 07b679f70d73
>>>> ("virtio-blk: support completion batching for the IRQ path") overlloked
>>>> handling correctly the completion of zone append operations, which
>>>> require an update of the request __sector field, as is done in
>>>> virtblk_request_done(): the function virtblk_complete_batch() only
>>>> executes virtblk_unmap_data() and virtblk_cleanup_cmd() without doing
>>>> this update. This causes problems with zone append operations, e.g.
>>>> zonefs complains about invalid zone append locations.
>>>>
>>>
>>> Hi Damien Le Moal,
>>>
>>> Unfortunately, this commit was reverted due to io hang.
>>> (afd384f0dbea2229fd11159efb86a5b41051c4a9)
>>> You can see the mail thread at the block layer mailing list.
>>
>> There is no commit afd384f0dbea2229fd11159efb86a5b41051c4a9 in Linus tree. What
>> patch are you talking about ? Where is it ?
> 
> Either you didn't check recently enough, or  there's some
> breakage and your CDN's not updating. If the later try
> poking kernel.org admins.
> 
> This is the commit:
> 
> commit afd384f0dbea2229fd11159efb86a5b41051c4a9
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date:   Thu Jun 8 17:42:53 2023 -0400
> 
>     Revert "virtio-blk: support completion batching for the IRQ path"
> 
> you can get the patch from lore too:
>     Message-Id: <336455b4f630f329380a8f53ee8cad3868764d5c.1686295549.git.mst@redhat.com>

Yep. Got it after pulling from Linus master. Should have done that first :)

>>> We don't have a solution about io hang yet..
>>> So I have one question.
>>> Is there any possibility of virtblk-driver io hang on zoned devices
>>> without this patch?
>>
>> If you are talking about the batch completion support being reverted, then my
>> fix patch is not necessary. The issue I fixed is not about IO hang but the fact
>> that completion processing was not identical for batch case vs non batch. That
>> led to breakage of the zone append command completion. The original support for
>> zone append without batch completion is fine.
> 
> Yes that's great! I expect we'll reapply the batch completion
> down the road and then your patch would help!

OK, Thanks !

-- 
Damien Le Moal
Western Digital Research