Re: [PATCH 05/17] btrfs: don't allow tree block to cross page boundary for subpage support

[Nikolay Borisov <nborisov@suse.com>]

On 8.09.20 г. 10:52 ч., Qu Wenruo wrote:
> As a preparation for subpage sector size support (allow sector size
> smaller than page size to be mounted), if the sector size is smaller

nit: (allowing filesystem with sector size smaller than page size to be
mounted)

> than page size, we don't allow tree block to be read if it cross page
> boundary (normally 64K).

Why normally 64k ? I suspect you assume readers are familiar with the
motivation for this work which they don't necessarily need to be. Please
make your sentences explicit. Correct me if I'm wrong but you are
ensuring that an eb doesn't cross the largest possible sectorsize?
> 
> This ensures that, tree blocks are always contained in one page for 64K
> system, which can greatly simplify the handling.

"For system with 64k page size" the term "64k system" is ambiguous, heed
this when rewording other patches as well since I've seen this used in
multiple places.


> Or we need to do complex multi-page handling for tree blocks.
> 
> Currently the only way to create such tree blocks crossing 64K boundary
> is by btrfs-convert, which will get fixed soon and doesn't get
> wide-spread usage.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_io.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 5d969340275e..119193166cec 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -5232,6 +5232,13 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
>  		btrfs_err(fs_info, "bad tree block start %llu", start);
>  		return ERR_PTR(-EINVAL);
>  	}
> +	if (fs_info->sectorsize < PAGE_SIZE && round_down(start, PAGE_SIZE) !=
> +	    round_down(start + len - 1, PAGE_SIZE)) {
> +		btrfs_err(fs_info,
> +		"tree block crosses page boundary, start %llu nodesize %lu",
> +			  start, len);
> +		return ERR_PTR(-EINVAL);
> +	}
>  
>  	eb = find_extent_buffer(fs_info, start);
>  	if (eb)
>