[PATCH v2] btrfs: Batch up release of reserved metadata for delayed items used for deletion

[PATCH v2] btrfs: Batch up release of reserved metadata for delayed items used for deletion

[Nikolay Borisov]

With Filipe's recent rework of the delayed inode code one aspect which
isn't batched is the release of the reserved metadata of delayed inode's
delete items. With this patch on top of Filipe's rework and running the
same test as provided in the description of a patch titled
"btrfs: improve batch deletion of delayed dir index items" I observe
the following change of the number of calls to btrfs_block_rsv_release:

Before this change:
@block_rsv_release: 1004
@btrfs_delete_delayed_items_total_time: 14602
@delete_batches: 505

After:
@block_rsv_release: 510
@btrfs_delete_delayed_items_total_time: 13643
@delete_batches: 507

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

V2:
 * Improved subject wording to make it more clear (Filipe)

 * Print the inode number in the tracepoint (Filipe)

 * More explicit referal to the test case in Filipe's initial patch to make it
 more clear how those numbers are derived.

 fs/btrfs/delayed-inode.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index e1e856436ad5..6c06ddba5a7a 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -800,11 +800,13 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
 				    struct btrfs_path *path,
 				    struct btrfs_delayed_item *item)
 {
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_delayed_item *curr, *next;
 	struct extent_buffer *leaf = path->nodes[0];
 	LIST_HEAD(batch_list);
 	int nitems, slot, last_slot;
 	int ret;
+	u64 total_reserved_size = item->bytes_reserved;

 	ASSERT(leaf != NULL);

@@ -841,14 +843,23 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
 		nitems++;
 		curr = next;
 		list_add_tail(&curr->tree_list, &batch_list);
+		total_reserved_size += curr->bytes_reserved;
 	}

 	ret = btrfs_del_items(trans, root, path, path->slots[0], nitems);
 	if (ret)
 		return ret;

+	/*
+	 * Check btrfs_delayed_item_reserve_metadata() to see why we don't need
+	 * to release/reserve qgroup space.
+	 */
+	trace_btrfs_space_reservation(fs_info, "delayed_item", 0,
+				      total_reserved_size, 0);
+	btrfs_block_rsv_release(fs_info, &fs_info->delayed_block_rsv,
+				total_reserved_size, NULL);
+
 	list_for_each_entry_safe(curr, next, &batch_list, tree_list) {
-		btrfs_delayed_item_release_metadata(root, curr);
 		list_del(&curr->tree_list);
 		btrfs_release_delayed_item(curr);
 	}
--
2.25.1

Re: [PATCH v2] btrfs: Batch up release of reserved metadata for delayed items used for deletion

[Filipe Manana]

On Fri, Jun 17, 2022 at 02:36:30PM +0300, Nikolay Borisov wrote:
> With Filipe's recent rework of the delayed inode code one aspect which
> isn't batched is the release of the reserved metadata of delayed inode's
> delete items. With this patch on top of Filipe's rework and running the
> same test as provided in the description of a patch titled
> "btrfs: improve batch deletion of delayed dir index items" I observe
> the following change of the number of calls to btrfs_block_rsv_release:
> 
> Before this change:
> @block_rsv_release: 1004
> @btrfs_delete_delayed_items_total_time: 14602
> @delete_batches: 505
> 
> After:
> @block_rsv_release: 510
> @btrfs_delete_delayed_items_total_time: 13643
> @delete_batches: 507
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 
> V2:
>  * Improved subject wording to make it more clear (Filipe)
> 
>  * Print the inode number in the tracepoint (Filipe)
> 
>  * More explicit referal to the test case in Filipe's initial patch to make it
>  more clear how those numbers are derived.
> 
>  fs/btrfs/delayed-inode.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index e1e856436ad5..6c06ddba5a7a 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -800,11 +800,13 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
>  				    struct btrfs_path *path,
>  				    struct btrfs_delayed_item *item)
>  {
> +	struct btrfs_fs_info *fs_info = root->fs_info;
>  	struct btrfs_delayed_item *curr, *next;
>  	struct extent_buffer *leaf = path->nodes[0];
>  	LIST_HEAD(batch_list);
>  	int nitems, slot, last_slot;
>  	int ret;
> +	u64 total_reserved_size = item->bytes_reserved;
> 
>  	ASSERT(leaf != NULL);
> 
> @@ -841,14 +843,23 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
>  		nitems++;
>  		curr = next;
>  		list_add_tail(&curr->tree_list, &batch_list);
> +		total_reserved_size += curr->bytes_reserved;
>  	}
> 
>  	ret = btrfs_del_items(trans, root, path, path->slots[0], nitems);
>  	if (ret)
>  		return ret;
> 
> +	/*
> +	 * Check btrfs_delayed_item_reserve_metadata() to see why we don't need
> +	 * to release/reserve qgroup space.
> +	 */
> +	trace_btrfs_space_reservation(fs_info, "delayed_item", 0,
> +				      total_reserved_size, 0);

Still using 0 instead of item->key.objectid, just like v1.

Also, I forgot to point out in v1, these calls should be done under a:

   if (!test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags))

(or total_reserved_size > 0)

Thanks.

> +	btrfs_block_rsv_release(fs_info, &fs_info->delayed_block_rsv,
> +				total_reserved_size, NULL);
> +
>  	list_for_each_entry_safe(curr, next, &batch_list, tree_list) {
> -		btrfs_delayed_item_release_metadata(root, curr);
>  		list_del(&curr->tree_list);
>  		btrfs_release_delayed_item(curr);
>  	}
> --
> 2.25.1
>