Re: [PATCH] f2fs: optimize code of f2fs_update_extent_tree_range

[Chao Yu <chao2.yu@samsung.com>]

> -----Original Message-----
> From: Fan Li [mailto:fanofcode.li@samsung.com]
> Sent: Wednesday, September 09, 2015 10:27 AM
> To: jaegeuk@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: [f2fs-dev] [PATCH] f2fs: optimize code of f2fs_update_extent_tree_range
> 
> 
> Fix two potential problems:
> 1. when largest extent needs to be invalidated, it will be reset in
>    __drop_largest_extent, which makes __is_extent_same after always
>    return false, and largest extent unchanged. Now we update it properly.
> 
> 2. When extent is split and the latter part remains in tree, next_en
>    should be the latter part instead of next extent of original extent.
>    It will cause merge failure if there is in-place update, although
>    there is not, I think this fix will still makes codes less ambiguous.
> 
> This patch also simpfies codes of invalidating extents, and optimizes the
> procedues that split extent into two.
> 
> 
> Signed-off-by: Fan li <fanofcode.li@samsung.com>
> ---
>  fs/f2fs/extent_cache.c |  150
> +++++++++++++++++++-----------------------------
>  1 file changed, 58 insertions(+), 92 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 997ac86..b95cfb3 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -399,7 +399,7 @@ unsigned int f2fs_update_extent_tree_range(struct inode
> *inode,
>  {
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	struct extent_tree *et = F2FS_I(inode)->extent_tree;
> -	struct extent_node *en = NULL, *en1 = NULL, *en2 = NULL, *en3 =
> NULL;

This patch is out of format, I guess your email client was set with
"auto line wrap" option. Can you fix and resend the patch?

> +	struct extent_node *en = NULL, *en1 = NULL;
>  	struct extent_node *prev_en = NULL, *next_en = NULL;
>  	struct extent_info ei, dei, prev;
>  	struct rb_node **insert_p = NULL, *insert_parent = NULL;
> @@ -419,9 +419,6 @@ unsigned int f2fs_update_extent_tree_range(struct inode
> *inode,
>  	prev = et->largest;
>  	dei.len = 0;
> 
> -	/* we do not guarantee that the largest extent is cached all the
> time */
> -	__drop_largest_extent(inode, fofs);

I think we should keep this, since in shrink flow, we will not update largest
info in inode's extent tree, that means largest extent will be remained if
related extent node has been shrunk. So here if we do not drop largest extent
when fofs located in largest extent, our mapping info in dnode page and largest
extent in cache will be consistent. How do you think?

Thanks,



------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140