diff mbox

Btrfs: clear EXTENT_DEFRAG bits in finish_ordered_io

Message ID 1494370935-19965-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo May 9, 2017, 11:02 p.m. UTC
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(-)

Comments

David Sterba May 19, 2017, 7:06 p.m. UTC | #1
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Bo May 19, 2017, 8:01 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba May 25, 2017, 5:11 p.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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)