On 2020/11/19 上午3:48, David Sterba wrote:
> On Fri, Nov 13, 2020 at 08:51:38PM +0800, Qu Wenruo wrote:
>> --- a/fs/btrfs/ctree.c
>> +++ b/fs/btrfs/ctree.c
>> @@ -1686,10 +1686,11 @@ static noinline int generic_bin_search(struct extent_buffer *eb,
>>  		oip = offset_in_page(offset);
>>  
>>  		if (oip + key_size <= PAGE_SIZE) {
>> -			const unsigned long idx = offset >> PAGE_SHIFT;
>> +			const unsigned long idx = get_eb_page_index(offset);
>>  			char *kaddr = page_address(eb->pages[idx]);
>>  
>> -			tmp = (struct btrfs_disk_key *)(kaddr + oip);
>> +			tmp = (struct btrfs_disk_key *)(kaddr +
>> +					get_eb_page_offset(eb, offset));
> 
> Here offset_in_page(offset) == get_eb_page_offset(eb, offset) and does
> not need to be calculated again for both sector/page combinations. As
> this is a hot path in search it sould be kept optimizied.
> 
Now you fall in to the pitfall.

offset_in_page(offset) != get_eb_page_offset(eb, offset) at all.

get_eb_page_offset(eb, offset) will add eb->start into the
offset_in_page() call.
This is especially important for subpage.

So here we still need to call get_eb_page_offset().

Although I need to find a better name for it.

THanks,
Qu