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

[Filipe Manana <fdmanana@kernel.org>]

On Thu, Jun 02, 2022 at 11:24:53AM +0300, Nikolay Borisov wrote:
> 
> 
> 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.

That's a rather different thing from what the patch intends, which
is just to improve the deletion of items in the btree.

Send that as a separate patch with a proper changelog.

Thanks.

> 
> > 
> > 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,