Re: [PATCH 0/3] btrfs: use btrfs_path::reada to replace the

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

On 2021/12/14 20:41, David Sterba wrote:
> On Mon, Dec 13, 2021 at 09:10:51PM +0800, Qu Wenruo wrote:
>> [PROBLEMS]
>> The metadata readahead code is introduced in 2011 (surprisingly, the
>> commit message even contains changelog), but now there is only one user
>> for it, and even for the only one user, the readahead mechanism can't
>> provide all it needs:
>>
>> - No support for commit tree readahead
>>    Only support readahead on current tree.
>>
>> - Bad layer separation
>>    To manage on-fly bios, btrfs_reada_add() mechanism internally manages
>>    a kinda complex zone system, and it's bind to per-device.
>>
>>    This is against the common layer separation, as metadata should all be
>>    in btrfs logical address space, should not be bound to device physical
>>    layer.
>>
>>    In fact, this is the cause of all recent reada related bugs.
>>
>> - No caller for asynchronous metadata readahead
>>    Even btrfs_reada_add() is designed to be fully asynchronous, scrub
>>    only calls it in a synchronous way (call btrfs_reada_add() and
>>    immediately call btrfs_reada_wait()).
>>    Thus rendering a lot of code unnecessary.
>
> I agree with removing the reada.c code, it's overengineered perhaps with
> intentions to use it in more places but this hasn't happened and nobody
> is interested doing the work. We have the path readahead so it's not
> we'd lose readahead capabilities at all.
>
> Thanks for benchmarking it, the drop is acceptable and we know people
> are more interested in limiting the load anyway.
>
>> [BENCHMARK]
>> The conclusion looks like this:
>>
>> For the worst case (no dirty metadata, slow HDD), there will be around 5%
>> performance drop for scrub.
>> For other cases (even SATA SSD), there is no distinguishable performance
>> difference.
>>
>> The number is reported scrub speed, in MiB/s.
>> The resolution is limited by the reported duration, which only has a
>> resolution of 1 second.
>>
>> 	Old		New		Diff
>> SSD	455.3		466.332		+2.42%
>> HDD	103.927 	98.012		-5.69%
>
> I'll copy this information to the last patch changelog.

Since I found a bug in the first patch, let me have the chance to
re-order the patches, and put this into the patch removing reada.

Thanks,
Qu