Re: [PATCH 05/10] btrfs: precalculate checksums per leaf once

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2923 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> btrfs_csum_bytes_to_leaves shows up in system profiles, which makes it a
> candidate for optimizations. After the 64bit division has been replaced
> by shift, there's still a calculation done each time the function is
> called: checksums per leaf.
> 
> As this is a constanat value for the entire filesystem lifetime, we
> can calculate it once at mount time and reuse. This also allows to
> reduce the division to 64bit/32bit as we know the constant will always
> fit to 32bit type.
> 
> Replace the open-coded rounding up with a macro that internally handles
> the 64bit division.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h       | 1 +
>  fs/btrfs/disk-io.c     | 1 +
>  fs/btrfs/extent-tree.c | 9 +--------
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 1f2162fc1daa..8c4cd79b2810 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -933,6 +933,7 @@ struct btrfs_fs_info {
>  	u32 sectorsize;
>  	u32 sectorsize_bits;
>  	u32 csum_size;
> +	u32 csums_per_leaf;
>  	u32 stripesize;
>  
>  	/* Block groups and devices containing active swapfiles. */
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 25dbdfa8bc4b..f870e252aa37 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3079,6 +3079,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  	fs_info->sectorsize = sectorsize;
>  	fs_info->sectorsize_bits = ilog2(sectorsize);
>  	fs_info->csum_size = btrfs_super_csum_size(disk_super);
> +	fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;

I guess here we don't need any macro for division right?
The BTRFS_MAX_ITEM_SIZE() should follow the type of
BTRFS_MAX_ITEM_SIZE() which is u32, thus u32/u32, we're safe even on
32bit systems, right?

>  	fs_info->stripesize = stripesize;
>  
>  	/*
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 29ac97248942..81440a0ba106 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2138,17 +2138,10 @@ static u64 find_middle(struct rb_root *root)
>   */
>  u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
>  {
> -	u64 csum_size;
> -	u64 num_csums_per_leaf;
>  	u64 num_csums;
>  
> -	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
> -	num_csums_per_leaf = div64_u64(csum_size,
> -			(u64)btrfs_super_csum_size(fs_info->super_copy));
>  	num_csums = csum_bytes >> fs_info->sectorsize_bits;
> -	num_csums += num_csums_per_leaf - 1;
> -	num_csums = div64_u64(num_csums, num_csums_per_leaf);
> -	return num_csums;
> +	return DIV_ROUND_UP_ULL(num_csums, fs_info->csums_per_leaf);

Since it's just a DIV_ROUND_UP_ULL() call, can we make it inline?

Thanks,
Qu
>  }
>  
>  /*
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]