Re: new scrub code vs zoned file systems

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2023/6/1 13:17, Naohiro Aota wrote:
> On Thu, Jun 01, 2023 at 01:00:40PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2023/6/1 12:40, Christoph Hellwig wrote:
>>> On Thu, Jun 01, 2023 at 10:09:24AM +0800, Qu Wenruo wrote:
>>>> So far the various wrapper around the write operations work as expected,
>>>> and hide the detailed well enough that most of us didn't even notice.
>>>>
>>>> E.g. all the zoned code is already handled in scrub_write_sectors().
>>>>
>>>> The crash itself is caused by the fact that end io part is relying on
>>>> the inode pointer, that itself is a simple fix.
>>>
>>> But the reason why it is relying on the inode pointer is that it needs
>>> to record the actual written LBA after I/O completion.  So it's not
>>> just a case of just add a NULL check, it needs a way to adjust the
>>> logical to physical mapping from the dummy added before the I/O.
>>
>> That's all handled by scrub.
>>
>> For scrub we're doing the writes just like metadata, with QD=1, aka,
>> always write and wait (and know where the write would land), and for the
>> gaps we would call fill_writer_pointer_gap() to fill them.
>>
>> Thus we don't need to do any adjustment (unless you're talking about
>> RST, but I believe that's a different beast).
>
> True. For the dev_replace, we need to place the moved data at the same
> address on the destination device as the source device. Thus, we need to
> use WRITE command to ensure that.

Oh, that looks like the cause.

In btrfs_submit_repair_write() we set the bi_opf to ZONE_APPEND instead,
which later would trigger btrfs_record_physical_zoned().

So this means, we should not change the WRITE into ZONE_APPEND for
btrfs_submit_repair_write() for dev-replace case at all.

I stupidly thought zoned device can not accept WRITE command at all but
only ZONE_APPEND.

Let me try it locally first.

Thanks,
Qu
>
> So, calling into the record_physical function looks strange to me. It
> misses some condition to use ZONE_APPEND?
>
>>>
>>>> But I'm more concerned about why we have a full zone before that crash.
>>>
>>> I think this is happening because we can't account for the zone filling
>>> without the proper context.
>>
>> I believe it's a different problem, maybe some de-sync between scrub
>> write_pointer and the real zoned pointer inside the device.
>>
>> My current guess is, the target zone inside the target device is not
>> properly reset before dev-replace.
>
> This must be a different issue. Are we choosing that zone for zone finish
> to free the active zone resource?