Re: [PATCH v4 3/7] btrfs: split RAID stripes on deletion

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

在 2024/7/8 14:26, Johannes Thumshirn 写道:
> On 06.07.24 01:26, Qu Wenruo wrote:
>>
>>
>> 在 2024/7/6 00:43, Johannes Thumshirn 写道:
>>> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>>
>>> The current RAID stripe code assumes, that we will always remove a
>>> whole stripe entry.
>>>
>>> But if we're only removing a part of a RAID stripe we're hitting the
>>> ASSERT()ion checking for this condition.
>>>
>>> Instead of assuming the complete deletion of a RAID stripe, split the
>>> stripe if we need to.
>>
>> Sorry to be so critical, but if I understand correctly,
>> btrfs_insert_one_raid_extent() does not do any merge of stripe extent.
>
> No problem at all. I want to solve bugs, not increase my patch count ;).
>
>>
>> Thus one stripe extent always means part of a data extent.
>>
>> In that case a removal of a data extent should always remove all its
>> stripe extents.
>>
>> Furthermore due to the COW nature on zoned/rst devices, the space of a
>> deleted data extent should not be re-allocated until a transaction
>> commitment.
>>
>> Thus I'm wonder if this split is masking some bigger problems.
>
> Hmm now that you're saying it. The reason I wrote this path is, that I
> did hit the following ASSERT() in my testing:
>
>>> -		ASSERT(found_start >= start && found_end <= end);
>
> This indicates a partial delete of a stripe extent. But I agree as
> stripe extents are tied to extent items, this shouldn't really happen.
>
> So maybe most of this series (apart from the deadlock fix) masks problems?
>
> I'm back to the drawing board :(.

Can the ASSERT() be reproduced without a zoned device? (I'm really not a
fan of the existing tcmu emulated solution, meanwhile libvirt still
doesn't support ZNS devices)

If it can be reproduced just with RST feature, I may provide some help
digging into the ASSERT().

Thanks,
Qu