[PATCH] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[PATCH] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[Liu Bo]

Before this, we use 'filled' mode here, ie. if all range has been filled
with EXTENT_DEFRAG bits, get to clear it, but if the defrag range joins
the adjacent delalloc range, then we'll leave EXTENT_DEFRAG bits until
evicting inode.

This clears the bit if any was found within the ordered extent.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 78ab511..1293810 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2875,7 +2875,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 
 	ret = test_range_bit(io_tree, ordered_extent->file_offset,
 			ordered_extent->file_offset + ordered_extent->len - 1,
-			EXTENT_DEFRAG, 1, cached_state);
+			EXTENT_DEFRAG, 0, cached_state);
 	if (ret) {
 		u64 last_snapshot = btrfs_root_last_snapshot(&root->root_item);
 		if (0 && last_snapshot >= BTRFS_I(inode)->generation)
-- 
2.5.5

Re: [PATCH] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[David Sterba]

On Tue, May 09, 2017 at 05:02:15PM -0600, Liu Bo wrote:
> Before this, we use 'filled' mode here, ie. if all range has been filled
> with EXTENT_DEFRAG bits, get to clear it, but if the defrag range joins
> the adjacent delalloc range, then we'll leave EXTENT_DEFRAG bits until
> evicting inode.
>
> This clears the bit if any was found within the ordered extent.

What effects, good or bad, can this have?

Is it worth backporting to stable trees?

Re: [PATCH] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[Liu Bo]

On Fri, May 19, 2017 at 09:06:42PM +0200, David Sterba wrote:
> On Tue, May 09, 2017 at 05:02:15PM -0600, Liu Bo wrote:
> > Before this, we use 'filled' mode here, ie. if all range has been filled
> > with EXTENT_DEFRAG bits, get to clear it, but if the defrag range joins
> > the adjacent delalloc range, then we'll leave EXTENT_DEFRAG bits until
> > evicting inode.
> >
> > This clears the bit if any was found within the ordered extent.
> 
> What effects, good or bad, can this have?
> 
> Is it worth backporting to stable trees?

The good effect of this patch is to free extent_state quickly if we
don't need it, without this, it can't be freed since the extent_state
has at least EXTENT_DEFRAG bit in ->state.

Just notice that I made a mistake in the changelog, the bit will be
cleared until releasing pages, which may be called by
invalidate_mapping_ranges(), not evicting inode.

No, I don't think it's a candidate for stable tree.

thanks,
-liubo

Re: [PATCH] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[David Sterba]

On Fri, May 19, 2017 at 01:01:42PM -0700, Liu Bo wrote:
> On Fri, May 19, 2017 at 09:06:42PM +0200, David Sterba wrote:
> > On Tue, May 09, 2017 at 05:02:15PM -0600, Liu Bo wrote:
> > > Before this, we use 'filled' mode here, ie. if all range has been filled
> > > with EXTENT_DEFRAG bits, get to clear it, but if the defrag range joins
> > > the adjacent delalloc range, then we'll leave EXTENT_DEFRAG bits until
> > > evicting inode.
> > >
> > > This clears the bit if any was found within the ordered extent.
> > 
> > What effects, good or bad, can this have?
> > 
> > Is it worth backporting to stable trees?
> 
> The good effect of this patch is to free extent_state quickly if we
> don't need it, without this, it can't be freed since the extent_state
> has at least EXTENT_DEFRAG bit in ->state.
> 
> Just notice that I made a mistake in the changelog, the bit will be
> cleared until releasing pages, which may be called by
> invalidate_mapping_ranges(), not evicting inode.
> 
> No, I don't think it's a candidate for stable tree.

Thanks for the answers. Please update the patch changelog and resend.

[PATCH v2] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[Liu Bo]

Before this, we use 'filled' mode here, ie. if all range has been
filled with EXTENT_DEFRAG bits, get to clear it, but if the defrag
range joins the adjacent delalloc range, then we'll have EXTENT_DEFRAG
bits in extent_state until releasing this inode's pages, and that
prevents extent_date from being freed.

This clears the bit if any was found within the ordered extent.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: Improve changelog.

 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 78ab511..1293810 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2875,7 +2875,7 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 
 	ret = test_range_bit(io_tree, ordered_extent->file_offset,
 			ordered_extent->file_offset + ordered_extent->len - 1,
-			EXTENT_DEFRAG, 1, cached_state);
+			EXTENT_DEFRAG, 0, cached_state);
 	if (ret) {
 		u64 last_snapshot = btrfs_root_last_snapshot(&root->root_item);
 		if (0 && last_snapshot >= BTRFS_I(inode)->generation)
-- 
2.5.5

Re: [PATCH v2] Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

[David Sterba]

On Fri, May 26, 2017 at 05:44:23PM -0600, Liu Bo wrote:
> Before this, we use 'filled' mode here, ie. if all range has been
> filled with EXTENT_DEFRAG bits, get to clear it, but if the defrag
> range joins the adjacent delalloc range, then we'll have EXTENT_DEFRAG
> bits in extent_state until releasing this inode's pages, and that
> prevents extent_date from being freed.
> 
> This clears the bit if any was found within the ordered extent.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>