Re: [PATCH 3/3] btrfs: factor out reading of bg from find_frist_block_group

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 26.05.20 г. 17:21 ч., Johannes Thumshirn wrote:
> When find_first_block_group() finds a block group item in the extent-tree,
> it does a lookup of the object in the extent mapping tree and does further
> checks on the item.
> 
> Factor out this step from find_first_block_group() so we can further
> simplify the code.
> 
> As a bonus we even get a slight decrease in size:
> $ ./scripts/bloat-o-meter btrfs_old.ko btrfs.ko
> add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-2503 (-2503)
> Function                                     old     new   delta
> btrfs_read_block_groups.cold                 462     337    -125
> btrfs_read_block_groups                     4787    2409   -2378
> Total: Before=2369371, After=2366868, chg -0.11%

Always be careful of such results since this is likely due to less
inlining. The bulk of the size overhead is likely in the new function
and now those have been replaced by a 'call' and a function prologue
etc. So this doesn't always mean better performance is all I'm trying to
say ;)

> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>

Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>

Although look down for one minor discussion point.

> ---
>  fs/btrfs/block-group.c | 95 ++++++++++++++++++++++--------------------
>  1 file changed, 49 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index c4462e4c8413..3d9e0ee1d1be 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1522,6 +1522,52 @@ void btrfs_mark_bg_unused(struct btrfs_block_group *bg)
>  	spin_unlock(&fs_info->unused_bgs_lock);
>  }
>  
> +static int read_bg_from_eb(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
> +			   struct extent_buffer *leaf, int slot)

nit: I wonder if instead of passing leaf/slot it'll be better to pass
btrfs_path, since that function always refers to nodes/slots [0]. My gut
feeling is this provides better interface abstraction, but this is
effectively a private, function so I might be overthinking it.

<snip>



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux