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

From: Qu Wenruo
Date: Mon Jul 08 2024 - 01:20:39 EST




在 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@xxxxxxx>

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