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

David Sterba
2020-09-16T21:05:12Z

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.