Re: [PATCH 07/12] btrfs: improve batch deletion of delayed dir index items

[Nikolay Borisov <nborisov@suse.com>]

On 31.05.22 г. 18:06 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 

<snip>

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

One small idea, see further down below though.

> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   fs/btrfs/delayed-inode.c | 60 +++++++++++++++++-----------------------
>   1 file changed, 25 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index 0125586fd565..74c806d3ab2a 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -791,68 +791,58 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
>   {

<snip>

> +	while (slot < last_slot) {
> +		struct btrfs_key key;
>   
> -		curr = next;
>   		next = __btrfs_next_delayed_item(curr);
>   		if (!next)
>   			break;
>   
> -		if (!btrfs_is_continuous_delayed_item(curr, next))
> +		slot++;
> +		btrfs_item_key_to_cpu(leaf, &key, slot);
> +		if (btrfs_comp_cpu_keys(&next->key, &key) != 0)
>   			break;
> -
> -		i++;
> -		if (i > last_item)
> -			break;
> -		btrfs_item_key_to_cpu(leaf, &key, i);
> +		nitems++;
> +		curr = next;
> +		list_add_tail(&curr->tree_list, &batch_list);
>   	}
>   
> -	/*
> -	 * Our caller always gives us a path pointing to an existing item, so
> -	 * this can not happen.
> -	 */
> -	ASSERT(nitems >= 1);
> -	if (nitems < 1)
> -		return -ENOENT;
> -
>   	ret = btrfs_del_items(trans, root, path, path->slots[0], nitems);
>   	if (ret)
> -		goto out;
> +		return ret;
>   
> -	list_for_each_entry_safe(curr, next, &head, tree_list) {
> +	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);
>   	}

nit: This function could be further optimized
with the following snippet.

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index fedbc45b71a2..950f5a2a9950 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -805,6 +805,7 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
         LIST_HEAD(batch_list);
         int nitems, slot, last_slot;
         int ret;
+       u64 bytes_reserved = item->bytes_reserved;
  
         ASSERT(leaf != NULL);
  
@@ -840,6 +841,7 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
                         break;
                 nitems++;
                 curr = next;
+               bytes_reserved += curr->bytes_reserved;
                 list_add_tail(&curr->tree_list, &batch_list);
         }
  
@@ -847,8 +849,11 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans,
         if (ret)
                 return ret;
  
+       btrfs_block_rsv_release(root->fs_info, &root->fs_info->delayed_block_rsv,
+                               bytes_reserved, NULL);
+
         list_for_each_entry_safe(curr, next, &batch_list, tree_list) {
-               btrfs_delayed_item_release_metadata(root, curr);
+               //btrfs_delayed_item_release_metadata(root, curr);
                 list_del(&curr->tree_list);
                 btrfs_release_delayed_item(curr);
         }


Running your series VS this diff on top produces:

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


>   
> -out:
> -	return ret;
> +	return 0;
>   }
>   
>   static int btrfs_delete_delayed_items(struct btrfs_trans_handle *trans,