Message ID | 1494370935-19965-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
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 --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)
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(-)