diff mbox series

[f2fs-dev] f2fs: introduce two helper functions for the largest cached extent

Message ID 20230718083310.367067-1-chaoliu719@gmail.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev] f2fs: introduce two helper functions for the largest cached extent | expand

Commit Message

Chao Liu July 18, 2023, 8:33 a.m. UTC 
From: Chao Liu <liuchao@coolpad.com>

This patch is a cleanup:
1. Merge __drop_largest_extent() since it has only one caller.
2. Introduce __notify_largest_extent_updated() and
   __drop_largest_extent() to help manage largest and largest_update
   in extent_tree.

Signed-off-by: Chao Liu <liuchao@coolpad.com>
---
 fs/f2fs/extent_cache.c | 60 +++++++++++++++++++-----------------------
 fs/f2fs/f2fs.h         |  4 +--
 2 files changed, 29 insertions(+), 35 deletions(-)

Comments

Chao Yu July 22, 2023, 1:25 p.m. UTC | #1 
On 2023/7/18 16:33, Chao Liu wrote:
> From: Chao Liu <liuchao@coolpad.com>
> 
> This patch is a cleanup:
> 1. Merge __drop_largest_extent() since it has only one caller.
> 2. Introduce __notify_largest_extent_updated() and
>     __drop_largest_extent() to help manage largest and largest_update
>     in extent_tree.
> 
> Signed-off-by: Chao Liu <liuchao@coolpad.com>
> ---
>   fs/f2fs/extent_cache.c | 60 +++++++++++++++++++-----------------------
>   fs/f2fs/f2fs.h         |  4 +--
>   2 files changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 0e2d49140c07f..45dfddd8c3ad0 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -19,6 +19,12 @@
>   #include "node.h"
>   #include <trace/events/f2fs.h>
>   
> +static void __drop_largest_extent(struct extent_tree *et)
> +{
> +	et->largest.len = 0;
> +	et->largest_updated = true;
> +}
> +
>   bool sanity_check_extent_cache(struct inode *inode)
>   {
>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> @@ -35,8 +41,7 @@ bool sanity_check_extent_cache(struct inode *inode)
>   
>   	/* Let's drop, if checkpoint got corrupted. */
>   	if (is_set_ckpt_flags(sbi, CP_ERROR_FLAG)) {
> -		ei->len = 0;
> -		et->largest_updated = true;
> +		__drop_largest_extent(et);
>   		return true;
>   	}
>   
> @@ -310,6 +315,8 @@ static void __detach_extent_node(struct f2fs_sb_info *sbi,
>   
>   	if (et->cached_en == en)
>   		et->cached_en = NULL;
> +
> +	/* keep the largest as we can still use it */
>   	kmem_cache_free(extent_node_slab, en);
>   }
>   
> @@ -385,15 +392,6 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
>   	return count - atomic_read(&et->node_cnt);
>   }
>   
> -static void __drop_largest_extent(struct extent_tree *et,
> -					pgoff_t fofs, unsigned int len)
> -{
> -	if (fofs < et->largest.fofs + et->largest.len &&
> -			fofs + len > et->largest.fofs) {
> -		et->largest.len = 0;
> -		et->largest_updated = true;
> -	}
> -}
>   
>   void f2fs_init_read_extent_tree(struct inode *inode, struct page *ipage)
>   {
> @@ -601,6 +599,15 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>   	return en;
>   }
>   
> +static void __notify_largest_extent_updated(struct extent_tree *et,
> +					struct inode *inode)
> +{
> +	if (et->type == EX_READ && et->largest_updated) {
> +		f2fs_mark_inode_dirty_sync(inode, true);
> +		et->largest_updated = false;
> +	}
> +}
> +
>   static void __update_extent_tree_range(struct inode *inode,
>   			struct extent_info *tei, enum extent_type type)
>   {
> @@ -612,7 +619,6 @@ static void __update_extent_tree_range(struct inode *inode,
>   	struct rb_node **insert_p = NULL, *insert_parent = NULL;
>   	unsigned int fofs = tei->fofs, len = tei->len;
>   	unsigned int end = fofs + len;
> -	bool updated = false;
>   	bool leftmost = false;
>   
>   	if (!et)
> @@ -636,11 +642,10 @@ static void __update_extent_tree_range(struct inode *inode,
>   		prev = et->largest;
>   		dei.len = 0;
>   
> -		/*
> -		 * drop largest extent before lookup, in case it's already
> -		 * been shrunk from extent tree
> -		 */
> -		__drop_largest_extent(et, fofs, len);
> +		/* updates may cause largest extent cache to become invalid */
> +		if (fofs < et->largest.fofs + et->largest.len &&
> +		    fofs + len > et->largest.fofs)
> +			__drop_largest_extent(et);
>   	}
>   
>   	/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
> @@ -733,8 +738,7 @@ static void __update_extent_tree_range(struct inode *inode,
>   		if (dei.len >= 1 &&
>   				prev.len < F2FS_MIN_EXTENT_LEN &&
>   				et->largest.len < F2FS_MIN_EXTENT_LEN) {
> -			et->largest.len = 0;
> -			et->largest_updated = true;
> +			__drop_largest_extent(et);
>   			set_inode_flag(inode, FI_NO_EXTENT);
>   		}
>   	}
> @@ -742,10 +746,6 @@ static void __update_extent_tree_range(struct inode *inode,
>   	if (is_inode_flag_set(inode, FI_NO_EXTENT))
>   		__free_extent_tree(sbi, et);
>   
> -	if (et->largest_updated) {
> -		et->largest_updated = false;
> -		updated = true;
> -	}
>   	goto out_read_extent_cache;
>   update_age_extent_cache:
>   	if (!tei->last_blocks)
> @@ -758,9 +758,7 @@ static void __update_extent_tree_range(struct inode *inode,
>   					insert_p, insert_parent, leftmost);
>   out_read_extent_cache:
>   	write_unlock(&et->lock);
> -
> -	if (updated)
> -		f2fs_mark_inode_dirty_sync(inode, true);
> +	__notify_largest_extent_updated(et, inode);

et->largest_updated should be updated w/ &et->lock, this is why we checks
local variable @updated here to decide whether setting inode dirty or not.

Thanks,

>   }
>   
>   #ifdef CONFIG_F2FS_FS_COMPRESSION
> @@ -1092,7 +1090,6 @@ static void __drop_extent_tree(struct inode *inode, enum extent_type type)
>   {
>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>   	struct extent_tree *et = F2FS_I(inode)->extent_tree[type];
> -	bool updated = false;
>   
>   	if (!__may_extent_tree(inode, type))
>   		return;
> @@ -1101,14 +1098,11 @@ static void __drop_extent_tree(struct inode *inode, enum extent_type type)
>   	__free_extent_tree(sbi, et);
>   	if (type == EX_READ) {
>   		set_inode_flag(inode, FI_NO_EXTENT);
> -		if (et->largest.len) {
> -			et->largest.len = 0;
> -			updated = true;
> -		}
> +		if (et->largest.len)
> +			__drop_largest_extent(et);
>   	}
>   	write_unlock(&et->lock);
> -	if (updated)
> -		f2fs_mark_inode_dirty_sync(inode, true);
> +	__notify_largest_extent_updated(et, inode);
>   }
>   
>   void f2fs_drop_extent_tree(struct inode *inode)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d372bedb0fe4e..da02e120e5ea6 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -665,7 +665,7 @@ struct extent_tree {
>   
>   struct extent_tree_info {
>   	struct radix_tree_root extent_tree_root;/* cache extent cache entries */
> -	struct mutex extent_tree_lock;	/* locking extent radix tree */
> +	struct mutex extent_tree_lock;		/* locking extent radix tree */
>   	struct list_head extent_list;		/* lru list for shrinker */
>   	spinlock_t extent_lock;			/* locking extent lru list */
>   	atomic_t total_ext_tree;		/* extent tree count */
> @@ -766,7 +766,7 @@ enum {
>   	FI_ACL_MODE,		/* indicate acl mode */
>   	FI_NO_ALLOC,		/* should not allocate any blocks */
>   	FI_FREE_NID,		/* free allocated nide */
> -	FI_NO_EXTENT,		/* not to use the extent cache */
> +	FI_NO_EXTENT,		/* not to use the read extent cache */
>   	FI_INLINE_XATTR,	/* used for inline xattr */
>   	FI_INLINE_DATA,		/* used for inline data*/
>   	FI_INLINE_DENTRY,	/* used for inline dentry */
diff mbox series

Patch

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 0e2d49140c07f..45dfddd8c3ad0 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -19,6 +19,12 @@ 
 #include "node.h"
 #include <trace/events/f2fs.h>
 
+static void __drop_largest_extent(struct extent_tree *et)
+{
+	et->largest.len = 0;
+	et->largest_updated = true;
+}
+
 bool sanity_check_extent_cache(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -35,8 +41,7 @@  bool sanity_check_extent_cache(struct inode *inode)
 
 	/* Let's drop, if checkpoint got corrupted. */
 	if (is_set_ckpt_flags(sbi, CP_ERROR_FLAG)) {
-		ei->len = 0;
-		et->largest_updated = true;
+		__drop_largest_extent(et);
 		return true;
 	}
 
@@ -310,6 +315,8 @@  static void __detach_extent_node(struct f2fs_sb_info *sbi,
 
 	if (et->cached_en == en)
 		et->cached_en = NULL;
+
+	/* keep the largest as we can still use it */
 	kmem_cache_free(extent_node_slab, en);
 }
 
@@ -385,15 +392,6 @@  static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
 	return count - atomic_read(&et->node_cnt);
 }
 
-static void __drop_largest_extent(struct extent_tree *et,
-					pgoff_t fofs, unsigned int len)
-{
-	if (fofs < et->largest.fofs + et->largest.len &&
-			fofs + len > et->largest.fofs) {
-		et->largest.len = 0;
-		et->largest_updated = true;
-	}
-}
 
 void f2fs_init_read_extent_tree(struct inode *inode, struct page *ipage)
 {
@@ -601,6 +599,15 @@  static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 	return en;
 }
 
+static void __notify_largest_extent_updated(struct extent_tree *et,
+					struct inode *inode)
+{
+	if (et->type == EX_READ && et->largest_updated) {
+		f2fs_mark_inode_dirty_sync(inode, true);
+		et->largest_updated = false;
+	}
+}
+
 static void __update_extent_tree_range(struct inode *inode,
 			struct extent_info *tei, enum extent_type type)
 {
@@ -612,7 +619,6 @@  static void __update_extent_tree_range(struct inode *inode,
 	struct rb_node **insert_p = NULL, *insert_parent = NULL;
 	unsigned int fofs = tei->fofs, len = tei->len;
 	unsigned int end = fofs + len;
-	bool updated = false;
 	bool leftmost = false;
 
 	if (!et)
@@ -636,11 +642,10 @@  static void __update_extent_tree_range(struct inode *inode,
 		prev = et->largest;
 		dei.len = 0;
 
-		/*
-		 * drop largest extent before lookup, in case it's already
-		 * been shrunk from extent tree
-		 */
-		__drop_largest_extent(et, fofs, len);
+		/* updates may cause largest extent cache to become invalid */
+		if (fofs < et->largest.fofs + et->largest.len &&
+		    fofs + len > et->largest.fofs)
+			__drop_largest_extent(et);
 	}
 
 	/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
@@ -733,8 +738,7 @@  static void __update_extent_tree_range(struct inode *inode,
 		if (dei.len >= 1 &&
 				prev.len < F2FS_MIN_EXTENT_LEN &&
 				et->largest.len < F2FS_MIN_EXTENT_LEN) {
-			et->largest.len = 0;
-			et->largest_updated = true;
+			__drop_largest_extent(et);
 			set_inode_flag(inode, FI_NO_EXTENT);
 		}
 	}
@@ -742,10 +746,6 @@  static void __update_extent_tree_range(struct inode *inode,
 	if (is_inode_flag_set(inode, FI_NO_EXTENT))
 		__free_extent_tree(sbi, et);
 
-	if (et->largest_updated) {
-		et->largest_updated = false;
-		updated = true;
-	}
 	goto out_read_extent_cache;
 update_age_extent_cache:
 	if (!tei->last_blocks)
@@ -758,9 +758,7 @@  static void __update_extent_tree_range(struct inode *inode,
 					insert_p, insert_parent, leftmost);
 out_read_extent_cache:
 	write_unlock(&et->lock);
-
-	if (updated)
-		f2fs_mark_inode_dirty_sync(inode, true);
+	__notify_largest_extent_updated(et, inode);
 }
 
 #ifdef CONFIG_F2FS_FS_COMPRESSION
@@ -1092,7 +1090,6 @@  static void __drop_extent_tree(struct inode *inode, enum extent_type type)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct extent_tree *et = F2FS_I(inode)->extent_tree[type];
-	bool updated = false;
 
 	if (!__may_extent_tree(inode, type))
 		return;
@@ -1101,14 +1098,11 @@  static void __drop_extent_tree(struct inode *inode, enum extent_type type)
 	__free_extent_tree(sbi, et);
 	if (type == EX_READ) {
 		set_inode_flag(inode, FI_NO_EXTENT);
-		if (et->largest.len) {
-			et->largest.len = 0;
-			updated = true;
-		}
+		if (et->largest.len)
+			__drop_largest_extent(et);
 	}
 	write_unlock(&et->lock);
-	if (updated)
-		f2fs_mark_inode_dirty_sync(inode, true);
+	__notify_largest_extent_updated(et, inode);
 }
 
 void f2fs_drop_extent_tree(struct inode *inode)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d372bedb0fe4e..da02e120e5ea6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -665,7 +665,7 @@  struct extent_tree {
 
 struct extent_tree_info {
 	struct radix_tree_root extent_tree_root;/* cache extent cache entries */
-	struct mutex extent_tree_lock;	/* locking extent radix tree */
+	struct mutex extent_tree_lock;		/* locking extent radix tree */
 	struct list_head extent_list;		/* lru list for shrinker */
 	spinlock_t extent_lock;			/* locking extent lru list */
 	atomic_t total_ext_tree;		/* extent tree count */
@@ -766,7 +766,7 @@  enum {
 	FI_ACL_MODE,		/* indicate acl mode */
 	FI_NO_ALLOC,		/* should not allocate any blocks */
 	FI_FREE_NID,		/* free allocated nide */
-	FI_NO_EXTENT,		/* not to use the extent cache */
+	FI_NO_EXTENT,		/* not to use the read extent cache */
 	FI_INLINE_XATTR,	/* used for inline xattr */
 	FI_INLINE_DATA,		/* used for inline data*/
 	FI_INLINE_DENTRY,	/* used for inline dentry */