Re: [PATCH v2 3/4] btrfs: introduce btrfs_subpage_bitmap_info

[David Sterba <dsterba@suse.cz>]

On Tue, Aug 17, 2021 at 05:38:51PM +0800, Qu Wenruo wrote:
> Currently we use fixed size u16 bitmap for subpage bitmap.
> This is fine for 4K sectorsize with 64K page size.
> 
> But for 4K sectorsize and larger page size, the bitmap is too small,
> while for smaller page size like 16K, u16 bitmaps waste too much space.
> 
> Here we introduce a new helper structure, btrfs_subpage_bitmap_info, to
> record the proper bitmap size, and where each bitmap should start at.
> 
> By this, we can later compact all subpage bitmaps into one u32 bitmap.
> 
> This patch is the first step towards such compact bitmap.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ctree.h   |  1 +
>  fs/btrfs/disk-io.c | 12 +++++++++---
>  fs/btrfs/subpage.c | 35 +++++++++++++++++++++++++++++++++++
>  fs/btrfs/subpage.h | 36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index f07c82fafa04..a5297748d719 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -899,6 +899,7 @@ struct btrfs_fs_info {
>  	struct btrfs_workqueue *scrub_workers;
>  	struct btrfs_workqueue *scrub_wr_completion_workers;
>  	struct btrfs_workqueue *scrub_parity_workers;
> +	struct btrfs_subpage_info *subpage_info;
>  
>  	struct btrfs_discard_ctl discard_ctl;
>  
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 2f9515dccce0..3355708919d0 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1644,6 +1644,7 @@ void btrfs_free_fs_info(struct btrfs_fs_info *fs_info)
>  	btrfs_extent_buffer_leak_debug_check(fs_info);
>  	kfree(fs_info->super_copy);
>  	kfree(fs_info->super_for_commit);
> +	kfree(fs_info->subpage_info);
>  	kvfree(fs_info);
>  }
>  
> @@ -3392,12 +3393,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  		goto fail_alloc;
>  	}
>  
> -	if (sectorsize != PAGE_SIZE) {
> +	if (sectorsize < PAGE_SIZE) {
> +		struct btrfs_subpage_info *subpage_info;
> +
>  		btrfs_warn(fs_info,
>  		"read-write for sector size %u with page size %lu is experimental",
>  			   sectorsize, PAGE_SIZE);
> -	}
> -	if (sectorsize != PAGE_SIZE) {
>  		if (btrfs_super_incompat_flags(fs_info->super_copy) &
>  			BTRFS_FEATURE_INCOMPAT_RAID56) {
>  			btrfs_err(fs_info,
> @@ -3406,6 +3407,11 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  			err = -EINVAL;
>  			goto fail_alloc;
>  		}
> +		subpage_info = kzalloc(sizeof(*subpage_info), GFP_NOFS);
> +		if (!subpage_info)
> +			goto fail_alloc;
> +		btrfs_init_subpage_info(subpage_info, sectorsize);
> +		fs_info->subpage_info = subpage_info;
>  	}
>  
>  	ret = btrfs_init_workqueues(fs_info, fs_devices);
> diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
> index ae6c68370a95..c4fb2ce52207 100644
> --- a/fs/btrfs/subpage.c
> +++ b/fs/btrfs/subpage.c
> @@ -63,6 +63,41 @@
>   *   This means a slightly higher tree locking latency.
>   */
>  
> +void btrfs_init_subpage_info(struct btrfs_subpage_info *subpage_info,
> +			     u32 sectorsize)
> +{
> +	unsigned int cur = 0;
> +	unsigned int nr_bits;
> +
> +	/*
> +	 * Just in case we have super large PAGE_SIZE that unsigned int is not
> +	 * enough to contain the number of sectors for the minimal sectorsize.
> +	 */
> +	BUILD_BUG_ON(UINT_MAX * SZ_4K < PAGE_SIZE);

Do you seriously expect such hardware to exist? We know there are arches
with 256K page and that's perhaps the maximum we should care about now
but UINT_MAX * 4K is 16TiB, 2^44. CPUs are barely capable of addressing
2^40 physical address space, making assertions about page size orders of
magnitude larger than that is insane.