[PATCH] btrfs: do not account twice for inode ref when reserving metadata units

[PATCH] btrfs: do not account twice for inode ref when reserving metadata units

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When reserving metadata units for creating an inode, we don't need to
reserve one extra unit for the inode ref item because when creating the
inode, at btrfs_create_new_inode(), we always insert the inode item and
the inode ref item in a single batch (a single btree insert operation,
and both ending up in the same leaf).

As we have accounted already one unit for the inode item, the extra unit
for the inode ref item is superfluous, it only makes us reserve more
metadata than necessary and often adding more reclaim pressure if we are
low on available metadata space.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b42d6e7e4049..adc8b684fe31 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6138,12 +6138,15 @@ int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
 		(*trans_num_items)++;
 	} else {
 		/*
-		 * 1 to add inode ref
 		 * 1 to add dir item
 		 * 1 to add dir index
 		 * 1 to update parent inode item
+		 *
+		 * No need for 1 unit for the inode ref item because it is
+		 * inserted in a batch together with the inode item at
+		 * btrfs_create_new_inode().
 		 */
-		*trans_num_items += 4;
+		*trans_num_items += 3;
 	}
 	return 0;
 }
-- 
2.35.1

Re: [PATCH] btrfs: do not account twice for inode ref when reserving metadata units

[David Sterba]

On Mon, May 09, 2022 at 04:29:14PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When reserving metadata units for creating an inode, we don't need to
> reserve one extra unit for the inode ref item because when creating the
> inode, at btrfs_create_new_inode(), we always insert the inode item and
> the inode ref item in a single batch (a single btree insert operation,
> and both ending up in the same leaf).
> 
> As we have accounted already one unit for the inode item, the extra unit
> for the inode ref item is superfluous, it only makes us reserve more
> metadata than necessary and often adding more reclaim pressure if we are
> low on available metadata space.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Added to misc-next, thanks.

Re: [PATCH] btrfs: do not account twice for inode ref when reserving metadata units

[Nikolay Borisov]

On 9.05.22 г. 18:29 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When reserving metadata units for creating an inode, we don't need to
> reserve one extra unit for the inode ref item because when creating the
> inode, at btrfs_create_new_inode(), we always insert the inode item and
> the inode ref item in a single batch (a single btree insert operation,
> and both ending up in the same leaf).
> 
> As we have accounted already one unit for the inode item, the extra unit
> for the inode ref item is superfluous, it only makes us reserve more
> metadata than necessary and often adding more reclaim pressure if we are
> low on available metadata space.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

That's a neat little optimisation.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>   fs/btrfs/inode.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b42d6e7e4049..adc8b684fe31 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6138,12 +6138,15 @@ int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
>   		(*trans_num_items)++;
>   	} else {
>   		/*
> -		 * 1 to add inode ref
>   		 * 1 to add dir item
>   		 * 1 to add dir index
>   		 * 1 to update parent inode item
> +		 *
> +		 * No need for 1 unit for the inode ref item because it is
> +		 * inserted in a batch together with the inode item at
> +		 * btrfs_create_new_inode().
>   		 */
> -		*trans_num_items += 4;
> +		*trans_num_items += 3;
>   	}
>   	return 0;
>   }