Re: [PATCH v2 04/19] btrfs: remove the open-code to read disk-key

Qu Wenruo
2020-09-17T08:10:15Z

[-- Attachment #1.1: Type: text/plain, Size: 2051 bytes --]

On 2020/9/17 上午12:01, David Sterba wrote:
> On Tue, Sep 15, 2020 at 01:35:17PM +0800, Qu Wenruo wrote:
>> generic_bin_search() distinguishes between reading a key which doesn't
>> cross a page and one which does. However this distinction is not
>> necessary since read_extent_buffer handles both cases transparently.
>>
>> Just use read_extent_buffer to streamline the code.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/ctree.c | 13 ++-----------
>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
>> index cd1cd673bc0b..e204e1320745 100644
>> --- a/fs/btrfs/ctree.c
>> +++ b/fs/btrfs/ctree.c
>> @@ -1697,7 +1697,6 @@ static noinline int generic_bin_search(struct extent_buffer *eb,
>> }
>>
>> while (low < high) {
>> - unsigned long oip;
>> unsigned long offset;
>> struct btrfs_disk_key *tmp;
>> struct btrfs_disk_key unaligned;
>> @@ -1705,17 +1704,9 @@ static noinline int generic_bin_search(struct extent_buffer *eb,
>>
>> mid = (low + high) / 2;
>> offset = p + mid * item_size;
>> - oip = offset_in_page(offset);
>>
>> - if (oip + key_size <= PAGE_SIZE) {
>> - const unsigned long idx = offset >> PAGE_SHIFT;
>> - char *kaddr = page_address(eb->pages[idx]);
>> -
>> - tmp = (struct btrfs_disk_key *)(kaddr + oip);
>> - } else {
>> - read_extent_buffer(eb, &unaligned, offset, key_size);
>> - tmp = &unaligned;
>> - }
>> + read_extent_buffer(eb, &unaligned, offset, key_size);
>> + tmp = &unaligned;
>
> Reading from the first page is a performance optimization on systems
> with 4K pages, ie. the majority. I'm not in favor removing it just to
> make the code look nicer.
>

For 4K system, with the optimization it only saves one
read_extent_buffer() call cost.

In read_extent_buffer() it's still doing the same thing.

Or we will need to manually call get_eb_page_offset() here to make it
work for subpage.

I wonder if it really worthy.

Thanks,
Qu

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]