Re: [PATCH] Btrfs: allow clear_extent_dirty() to receive a cached extent state record

[Nikolay Borisov <nborisov@suse.com>]

On 16.11.18 г. 15:04 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> We can have a lot freed extents during the life span of transaction, so
> the red black tree that keeps track of the ranges of each freed extent
> (fs_info->freed_extents[]) can get quite big. When finishing a transaction
> commit we find each range, process it (discard the extents, unpin them)
> and then remove it from the red black tree. We can use an extent state
> record as a cache when searching for a range, so that when we clean the
> range we can use the cached extent state we passed to the search function
> instead of iterating the red black tree again. Doing things as fast as
> possible when finishing a transaction (in state TRANS_STATE_UNBLOCKED)
> is convenient as it reduces the time we block another task that wants to
> commit the next transaction.
> 
> So change clear_extent_dirty() to allow an optional extent state record to
> be passed as an argument, which will be passed down to __clear_extent_bit.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

> ---
>  fs/btrfs/disk-io.c     | 7 +++++--
>  fs/btrfs/extent-tree.c | 7 +++++--
>  fs/btrfs/extent_io.h   | 4 ++--
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 05dc3c17cb62..ecf3a45490e3 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4359,12 +4359,15 @@ static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
>  	unpin = pinned_extents;
>  again:
>  	while (1) {
> +		struct extent_state *cached_state = NULL;
> +
>  		ret = find_first_extent_bit(unpin, 0, &start, &end,
> -					    EXTENT_DIRTY, NULL);
> +					    EXTENT_DIRTY, &cached_state);
>  		if (ret)
>  			break;
>  
> -		clear_extent_dirty(unpin, start, end);
> +		clear_extent_dirty(unpin, start, end, &cached_state);
> +		free_extent_state(cached_state);
>  		btrfs_error_unpin_extent_range(fs_info, start, end);
>  		cond_resched();
>  	}
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 577878324799..33142d9c36d5 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -6615,9 +6615,11 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
>  		unpin = &fs_info->freed_extents[0];
>  
>  	while (!trans->aborted) {
> +		struct extent_state *cached_state = NULL;
> +
>  		mutex_lock(&fs_info->unused_bg_unpin_mutex);
>  		ret = find_first_extent_bit(unpin, 0, &start, &end,
> -					    EXTENT_DIRTY, NULL);
> +					    EXTENT_DIRTY, &cached_state);
>  		if (ret) {
>  			mutex_unlock(&fs_info->unused_bg_unpin_mutex);
>  			break;
> @@ -6627,9 +6629,10 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
>  			ret = btrfs_discard_extent(fs_info, start,
>  						   end + 1 - start, NULL);
>  
> -		clear_extent_dirty(unpin, start, end);
> +		clear_extent_dirty(unpin, start, end, &cached_state);
>  		unpin_extent_range(fs_info, start, end, true);
>  		mutex_unlock(&fs_info->unused_bg_unpin_mutex);
> +		free_extent_state(cached_state);
>  		cond_resched();
>  	}
>  
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index b4d03e677e1d..36f7a9f87e46 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -353,11 +353,11 @@ static inline int set_extent_dirty(struct extent_io_tree *tree, u64 start,
>  }
>  
>  static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
> -		u64 end)
> +				     u64 end, struct extent_state **cached)
>  {
>  	return clear_extent_bit(tree, start, end,
>  				EXTENT_DIRTY | EXTENT_DELALLOC |
> -				EXTENT_DO_ACCOUNTING, 0, 0, NULL);
> +				EXTENT_DO_ACCOUNTING, 0, 0, cached);
>  }
>  
>  int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>