Re: [PATCH v4 13/18] btrfs: introduce read_extent_buffer_subpage()

[Josef Bacik <josef@toxicpanda.com>]

On 1/16/21 2:15 AM, Qu Wenruo wrote:
> Introduce a new helper, read_extent_buffer_subpage(), to do the subpage
> extent buffer read.
> 
> The difference between regular and subpage routines are:
> - No page locking
>    Here we completely rely on extent locking.
>    Page locking can reduce the concurrency greatly, as if we lock one
>    page to read one extent buffer, all the other extent buffers in the
>    same page will have to wait.
> 
> - Extent uptodate condition
>    Despite the existing PageUptodate() and EXTENT_BUFFER_UPTODATE check,
>    We also need to check btrfs_subpage::uptodate_bitmap.
> 
> - No page loop
>    Just one page, no need to loop, this greately simplified the subpage
>    routine.
> 
> This patch only implemented the bio submit part, no endio support yet.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/extent_io.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 70 insertions(+)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 9414219fa28b..291ff76d5b2e 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -5718,6 +5718,73 @@ void set_extent_buffer_uptodate(struct extent_buffer *eb)
>   	}
>   }
>   
> +static int read_extent_buffer_subpage(struct extent_buffer *eb, int wait,
> +				      int mirror_num)
> +{
> +	struct btrfs_fs_info *fs_info = eb->fs_info;
> +	struct extent_io_tree *io_tree;
> +	struct page *page = eb->pages[0];
> +	struct bio *bio = NULL;
> +	int ret = 0;
> +
> +	ASSERT(!test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags));
> +	ASSERT(PagePrivate(page));
> +	io_tree = &BTRFS_I(fs_info->btree_inode)->io_tree;
> +
> +	if (wait == WAIT_NONE) {
> +		ret = try_lock_extent(io_tree, eb->start,
> +				      eb->start + eb->len - 1);
> +		if (ret <= 0)
> +			return ret;
> +	} else {
> +		ret = lock_extent(io_tree, eb->start, eb->start + eb->len - 1);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = 0;
> +	if (test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags) ||
> +	    PageUptodate(page) ||
> +	    btrfs_subpage_test_uptodate(fs_info, page, eb->start, eb->len)) {
> +		set_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags);
> +		unlock_extent(io_tree, eb->start, eb->start + eb->len - 1);
> +		return ret;
> +	}
> +
> +	clear_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
> +	eb->read_mirror = 0;
> +	atomic_set(&eb->io_pages, 1);
> +	check_buffer_tree_ref(eb);

We need btrfs_subpage_clear_error() here as well.  Thanks,

Josef