Re: [PATCH] Revert "btrfs: fix repair of compressed extents" and "btrfs: pass a btrfs_bio to btrfs_repair_one_sector"

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

On 2022/8/13 14:53, Qu Wenruo wrote:
>
>
> On 2022/8/13 14:19, Christoph Hellwig wrote:
>> On Sat, Aug 13, 2022 at 02:00:25PM +0800, Qu Wenruo wrote:
>>> To fix the problem, we need to revert commit 7aa51232e204 ("btrfs: pass
>>> a btrfs_bio to btrfs_repair_one_sector"), but unfortunately later commit
>>> 81bd9328ab9f ("btrfs: fix repair of compressed extents") has a
>>> dependency on that commit.
>>
>> Let's try to sort this out properly intead of doing a blind revert before
>> -rc1.  I'll cook up a patch to pass an explicit offset ASAP as the quick
>> fix, but for the longer run:  is there such a huge benefit of having
>> these logically non-contigous bios?  They are so different from the
>> I/O stack in any other file systems that I think we'll keep running into
>> problems again an again.

To answer that, the reason I found so far is purely because of btrfs
RAID profiles.

Unlike all other fses, btrfs has the in-built raid profiles, thus only
requiring the logical bytenr to be continuous will reduce the amount of
bios submitted by btrfs.

(Especially for profiles like RAID56 and RAID0/10)

Although I'm not sure if the reduced amount of bio would really cause
any different, as block layer should have its own merge optimization.

Thanks,
Qu

>
> OK, I didn't consider the reason why we allow non-contigous page into a
> bio.
>
> But indeed, if we allow that, it would be a much simpler fix.
>
> Mind me to introduce a patch to add a new page offset contingous check
> to the related code path?
>
> Thanks,
> Qu