Re: [PATCH 2/2] btrfs: defrag: remove an ambiguous condition for rejection

[Filipe Manana <fdmanana@kernel.org>]

On Thu, Jan 27, 2022 at 01:24:43PM +0800, Qu Wenruo wrote:
> From the very beginning of btrfs defrag, there is a check to reject
> extents which meet both conditions:
> 
> - Physically adjacent
> 
>   We may want to defrag physically adjacent extents to reduce the number
>   of extents or the size of subvolume tree.
> 
> - Larger than 128K
> 
>   This may be there for compressed extents, but unfortunately 128K is
>   exactly the max capacity for compressed extents.
>   And the check is > 128K, thus it never rejects compressed extents.
> 
>   Furthermore, the compressed extent capacity bug is fixed by previous
>   patch, there is no reason for that check anymore.
> 
> The original check has a very small ranges to reject (the target extent
> size is > 128K, and default extent threshold is 256K), and for
> compressed extent it doesn't work at all.
> 
> So it's better just to remove the rejection, and allow us to defrag
> physically adjacent extents.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good now, thanks.

> ---
>  fs/btrfs/ioctl.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a03c31e1ff18..af95e3b7aa72 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1078,10 +1078,6 @@ static bool defrag_check_next_extent(struct inode *inode, struct extent_map *em,
>  	 */
>  	if (next->len >= get_extent_max_capacity(em))
>  		goto out;
> -	/* Physically adjacent and large enough */
> -	if ((em->block_start + em->block_len == next->block_start) &&
> -	    (em->block_len > SZ_128K && next->block_len > SZ_128K))
> -		goto out;
>  	ret = true;
>  out:
>  	free_extent_map(next);
> -- 
> 2.34.1
>