Re: [PATCH v2 09/24] btrfs: extent_io: calculate inline extent buffer page size based on page size

[Josef Bacik <josef@toxicpanda.com>]

On 11/13/20 7:51 AM, Qu Wenruo wrote:
> Btrfs only support 64K as max node size, thus for 4K page system, we
> would have at most 16 pages for one extent buffer.
> 
> For a system using 64K page size, we would really have just one
> single page.
> 
> While we always use 16 pages for extent_buffer::pages[], this means for
> systems using 64K pages, we are wasting memory for the 15 pages which
> will never be utilized.
> 
> So this patch will change how the extent_buffer::pages[] array size is
> calclulated, now it will be calculated using
> BTRFS_MAX_METADATA_BLOCKSIZE and PAGE_SIZE.
> 
> For systems using 4K page size, it will stay 16 pages.
> For systems using 64K page size, it will be just 1 page.
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/extent_io.c | 7 +------
>   fs/btrfs/extent_io.h | 8 +++++---
>   2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 37dd103213f9..ca3eb095a298 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -5042,12 +5042,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
>   	atomic_set(&eb->refs, 1);
>   	atomic_set(&eb->io_pages, 0);
>   
> -	/*
> -	 * Sanity checks, currently the maximum is 64k covered by 16x 4k pages
> -	 */
> -	BUILD_BUG_ON(BTRFS_MAX_METADATA_BLOCKSIZE
> -		> MAX_INLINE_EXTENT_BUFFER_SIZE);
> -	BUG_ON(len > MAX_INLINE_EXTENT_BUFFER_SIZE);
> +	ASSERT(len <= BTRFS_MAX_METADATA_BLOCKSIZE);
>   
>   	return eb;
>   }
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index c76697fc3120..dfdef9c5c379 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -73,9 +73,11 @@ typedef blk_status_t (submit_bio_hook_t)(struct inode *inode, struct bio *bio,
>   
>   typedef blk_status_t (extent_submit_bio_start_t)(struct inode *inode,
>   		struct bio *bio, u64 bio_offset);
> -
> -#define INLINE_EXTENT_BUFFER_PAGES 16
> -#define MAX_INLINE_EXTENT_BUFFER_SIZE (INLINE_EXTENT_BUFFER_PAGES * PAGE_SIZE)
> +/*
> + * The SZ_64K is BTRFS_MAX_METADATA_BLOCKSIZE, here just to avoid circle
> + * including "ctree.h".
> + */

Just a passing thought, maybe we should move that definition into btrfs_tree.h.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef