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

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

[Qu Wenruo]

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.
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 */
+		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

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

[Filipe Manana]

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
> 

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

[David Sterba]

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

Added to misc-next, thanks.