Re: [PATCH 11/17] btrfs: extract the extent buffer verification from btree_readpage_end_io_hook()

Nikolay Borisov
2020-09-11T17:21:20Z

On 8.09.20 г. 10:52 ч., Qu Wenruo wrote:
> Currently btree_readpage_end_io_hook() only needs to handle one extent
> buffer as currently one page only maps to one extent buffer.
>
> But for incoming subpage support, one page can be mapped to multiple
> extent buffers, thus we can no longer use current code.
>
> This refactor would allow us to call btrfs_check_extent_buffer() on
> all involved extent buffers at btree_readpage_end_io_hook() and other
> locations.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
> fs/btrfs/disk-io.c | 78 ++++++++++++++++++++++++++--------------------
> 1 file changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 62dbd9bbd381..f6e562979682 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -574,60 +574,37 @@ static int check_tree_block_fsid(struct extent_buffer *eb)
> return ret;
> }
>

<snip>
> +static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
> + u64 phy_offset, struct page *page,
> + u64 start, u64 end, int mirror)
> +{
> + struct extent_buffer *eb;
> + int ret = 0;
> + int reads_done;

nit: while at it just turn it into a bool.

> +
> + if (!page->private)
> + goto out;
> +

nit: metadata pages are guaranteed to have their ->private val set to
the pointer of extent buffer so this check can go away.

> + eb = (struct extent_buffer *)page->private;
> +
> + /*
> + * The pending IO might have been the only thing that kept this buffer
> + * in memory. Make sure we have a ref for all this other checks
> + */
> + atomic_inc(&eb->refs);
> +
> + reads_done = atomic_dec_and_test(&eb->io_pages);
> + if (!reads_done)
> + goto err;
> +
> + eb->read_mirror = mirror;
> + if (test_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags)) {
> + ret = -EIO;
> + goto err;
> + }
> +
> + ret = btrfs_check_extent_buffer(eb);
> +
> err:
> if (reads_done &&
> test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
>