Re: [PATCH RFC 0/2] btrfs: remove the metadata readahead mechanism

Qu Wenruo
2021-12-08T00:08:46Z

On 2021/12/7 23:53, Filipe Manana wrote:
> On Tue, Dec 7, 2021 at 3:41 PM David Sterba <dsterba@suse.cz> wrote:
>>
>> On Tue, Dec 07, 2021 at 03:53:29PM +0100, David Sterba wrote:
>>> On Tue, Dec 07, 2021 at 08:01:04PM +0800, Qu Wenruo wrote:
>>>> On 2021/12/7 19:56, Filipe Manana wrote:
>>>>> On Tue, Dec 07, 2021 at 07:43:49PM +0800, Qu Wenruo wrote:
>>>>>> On 2021/12/7 19:02, Filipe Manana wrote:
>>>>>>> On Tue, Dec 07, 2021 at 03:43:58PM +0800, Qu Wenruo wrote:
>>>>>>>> This is originally just my preparation for scrub refactors, but when the
>>>>>>>> readahead is involved, it won't just be a small cleanup.
>>>>>>>>
>>>>>>>> The metadata readahead code is introduced in 2011 (surprisingly, the
>>>>>>>> commit message even contains changelog), but now only one user for it,
>>>>>>>> and even for the only one user, the readahead mechanism can't provide
>>>>>>>> much help in fact.
>>>>>>>>
>>>>>>>> Scrub needs readahead for commit root, but the existing one can only do
>>>>>>>> current root readahead.
>>>>>>>
>>>>>>> If support for the commit root is added, is there a noticeable speedup?
>>>>>>> Have you tested that?
>>>>>>
>>>>>> Will craft a benchmark for that.
>>>>>>
>>>>>> Although I don't have any HDD available for benchmark, thus would only
>>>>>> have result from SATA SSD.
>>>
>>> I'm doing some tests, in a VM on a dedicated HDD.
>>
>> There's some measurable difference:
>>
>> With readahead:
>>
>> Duration: 0:00:20
>> Total to scrub: 7.02GiB
>> Rate: 236.92MiB/s
>>
>> Duration: 0:00:48
>> Total to scrub: 12.02GiB
>> Rate: 198.02MiB/s
>>
>> Without readahead:
>>
>> Duration: 0:00:22
>> Total to scrub: 7.02GiB
>> Rate: 215.10MiB/s
>>
>> Duration: 0:00:50
>> Total to scrub: 12.02GiB
>> Rate: 190.66MiB/s
>>
>> The setup is: data/single, metadata/dup, no-holes, free-space-tree,
>> there are 8 backing devices but all reside on one HDD.
>>
>> Data generated by fio like
>>
>> fio --rw=randrw --randrepeat=1 --size=3000m \
>> --bsrange=512b-64k --bs_unaligned \
>> --ioengine=libaio --fsync=1024 \
>> --name=job0 --name=job1 \
>>
>> and scrub starts right away this. VM has 4G or memory and 4 CPUs.
>
> How about using bare metal? And was it a debug kernel, or a default
> kernel config from a distro?
> Those details often make all the difference (either for the best or
> for the worse).
>
> I'm curious to see as well the results when:
>
> 1) The reada.c code is changed to work with commit roots;

In fact I have a pretty simple patch for that already.

It's pushed to reada_commit_root branch.
(just misc-next with one patch)

>
> 2) The standard btree readahead (struct btrfs_path::reada) is used
> instead of the reada.c code.

It's already used for csum lookup, see btrfs_lookup_csums_range().

The missing extent tree part is pretty easy to do.

This is pushed to remove_reada branch.
(the patchset with one new one-line patch).

Thanks,
Qu
>
>>
>> The difference is 2 seconds, roughly 4% but the sample is not large
>> enough to be conclusive.
>
> A bit too small.
>
> Thanks.
>