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

[Qu Wenruo]

在 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