Re: [PATCH v2] btrfs: defrag: don't try to defrag extents which are under writeback

[Filipe Manana <fdmanana@kernel.org>]

On Tue, Feb 08, 2022 at 02:54:05PM +0800, Qu Wenruo wrote:
> Once we start writeback (have called btrfs_run_delalloc_range()), we
> allocate an extent, create an extent map point to that extent, with a
> generation of (u64)-1, created the ordered extent and then clear the
> DELALLOC bit from the range in the inode's io tree.
> 
> Such extent map can pass the first call of defrag_collect_targets(), as
> its generation is (u64)-1, meets any possible minimal geneartion check.

Same as in the other patch, geneartion -> generation.

> And the range will not have DELALLOC bit, also passing the DELALLOC bit
> check.
> 
> It will only be re-checked in the second call of
> defrag_collect_targets(), which will wait for writeback.
> 
> But at that stage we have already spent our time waiting for some IO we
> may or may not want to defrag.
> 
> Let's reject such extents early so we won't waste our time.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> - Update the subject, commit message and comment.
>   To replace the confusing phrase "be going to be written back" with
>   "under writeback".
> 
> - Update the commit message to indicate it's not always going to be marked
>   for defrag 
>   The second defrag_collect_targets() call will determine its destiny.
> 
> - Update the commit message to show why we want to skip it early
>   To save some time waiting for IO.
> ---
>  fs/btrfs/ioctl.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 3a5ada561298..f08005b41deb 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1383,6 +1383,10 @@ static int defrag_collect_targets(struct btrfs_inode *inode,
>  		if (em->generation < ctrl->newer_than)
>  			goto next;
>  
> +		/* This em is goging to be written back, no need to defrag */

Still as in v1, should be "This em is under writeback...".

With that fixed:

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


> +		if (em->generation == (u64)-1)
> +			goto next;
> +
>  		/*
>  		 * Our start offset might be in the middle of an existing extent
>  		 * map, so take that into account.
> -- 
> 2.35.0
>