Re: [PoC PATCH 00/11] btrfs: scrub: rework to get rid of the complex bio formshaping

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

On 2022/12/6 16:40, Christoph Hellwig wrote:
> On Tue, Dec 06, 2022 at 04:23:27PM +0800, Qu Wenruo wrote:
>> TL;DR
>> The current scrub code is way too complex for future expansion.
>>
>> Current scrub code has a complex system to manage its in-flight bios.
> 
>  From my own ventures into that code I have to agree.
> 
>> This behavior is designed to improve scrub performance, but has a lot of
>> disadvantage too:
> 
> Just curious:  any idea how it was supposed to improve performance?
> Because the code does not actually look particularly optimized in terms
> of I/O patterns.

I'm not 100% sure, but my educated guess is, it's just merging bios.

- Merge physically adjacent read/writes into one scrub_bio
   This will mostly help RAID0/RAID10/RAID5 scrubbing.
   As although logically each stripe is only 64K, this allows scrub
   to assemble all read/writes into a larger bio instead always split
   them at stripe boundary.

   However the effectiveness should not be that huge, as the scrub_bio
   size is only slightly increased from 64K to 128K.

I'd argue that if the extents are all interleaved, then my 
always-read-64K behavior would be better.

> 
>> Furthermore, all work will done in a submit-and-wait fashion, reducing
>> the delayed calls/jumps to minimal.
> 
> I think even with this overall scheme we could do a bit of async
> state machine if needed.

Yes, the infrastructure is already here.

In raid56, we need to scrub the data stripes first.
In that case, scrub2_stripe_group is introduced, and use workqueue for 
each stripe to scrub.

So if later we determine the current read-64K-then-next behavior is not 
fast enough, we can use scrub2_stripe_group to run several stripes at once.

>  But then again scrube is not the main
> I/O fast path, so in doubt we can just throw more threads at the
> problem if that becomes too complicated.

Exactly.

I'd do some benchmark later to show the difference.

Thanks,
Qu