All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] f2fs: avoid sleeping under spin_lock
@ 2018-09-10  8:18 Zhikang Zhang
  2018-09-10 14:34 ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Zhikang Zhang @ 2018-09-10  8:18 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

In the call trace below, we might sleep in function dput().

So in order to avoid sleeping under spin_lock, we remove f2fs_mark_inode_dirty_sync
from __try_update_largest_extent && __drop_largest_extent.

BUG: sleeping function called from invalid context at fs/dcache.c:796
Call trace:
	dump_backtrace+0x0/0x3f4
	show_stack+0x24/0x30
	dump_stack+0xe0/0x138
	___might_sleep+0x2a8/0x2c8
	__might_sleep+0x78/0x10c
	dput+0x7c/0x750
	block_dump___mark_inode_dirty+0x120/0x17c
	__mark_inode_dirty+0x344/0x11f0
	f2fs_mark_inode_dirty_sync+0x40/0x50
	__insert_extent_tree+0x2e0/0x2f4
	f2fs_update_extent_tree_range+0xcf4/0xde8
	f2fs_update_extent_cache+0x114/0x12c
	f2fs_update_data_blkaddr+0x40/0x50
	write_data_page+0x150/0x314
	do_write_data_page+0x648/0x2318
	__write_data_page+0xdb4/0x1640
	f2fs_write_cache_pages+0x768/0xafc
	__f2fs_write_data_pages+0x590/0x1218
	f2fs_write_data_pages+0x64/0x74
	do_writepages+0x74/0xe4
	__writeback_single_inode+0xdc/0x15f0
	writeback_sb_inodes+0x574/0xc98
	__writeback_inodes_wb+0x190/0x204
	wb_writeback+0x730/0xf14
	wb_check_old_data_flush+0x1bc/0x1c8
	wb_workfn+0x554/0xf74
	process_one_work+0x440/0x118c
	worker_thread+0xac/0x974
	kthread+0x1a0/0x1c8
	ret_from_fork+0x10/0x1c
---
 fs/f2fs/extent_cache.c | 51 +++++++++++++++++++++++++++++++-------------------
 fs/f2fs/f2fs.h         |  7 ++++---
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 231b77e..a70cd25 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -308,14 +308,13 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
 	return count - atomic_read(&et->node_cnt);
 }
 
-static void __drop_largest_extent(struct inode *inode,
+static void __drop_largest_extent(struct extent_tree *et,
 					pgoff_t fofs, unsigned int len)
 {
-	struct extent_info *largest = &F2FS_I(inode)->extent_tree->largest;
-
-	if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs) {
-		largest->len = 0;
-		f2fs_mark_inode_dirty_sync(inode, true);
+	if (fofs < et->largest.fofs + et->largest.len &&
+			fofs + len > et->largest.fofs) {
+		et->largest.len = 0;
+		et->largest_updated = true;
 	}
 }
 
@@ -416,12 +415,11 @@ static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
 	return ret;
 }
 
-static struct extent_node *__try_merge_extent_node(struct inode *inode,
+static struct extent_node *__try_merge_extent_node(struct f2fs_sb_info *sbi,
 				struct extent_tree *et, struct extent_info *ei,
 				struct extent_node *prev_ex,
 				struct extent_node *next_ex)
 {
-	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct extent_node *en = NULL;
 
 	if (prev_ex && __is_back_mergeable(ei, &prev_ex->ei)) {
@@ -443,7 +441,7 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
 	if (!en)
 		return NULL;
 
-	__try_update_largest_extent(inode, et, en);
+	__try_update_largest_extent(et, en);
 
 	spin_lock(&sbi->extent_lock);
 	if (!list_empty(&en->list)) {
@@ -454,12 +452,11 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
 	return en;
 }
 
-static struct extent_node *__insert_extent_tree(struct inode *inode,
+static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 				struct extent_tree *et, struct extent_info *ei,
 				struct rb_node **insert_p,
 				struct rb_node *insert_parent)
 {
-	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct rb_node **p;
 	struct rb_node *parent = NULL;
 	struct extent_node *en = NULL;
@@ -476,7 +473,7 @@ static struct extent_node *__insert_extent_tree(struct inode *inode,
 	if (!en)
 		return NULL;
 
-	__try_update_largest_extent(inode, et, en);
+	__try_update_largest_extent(et, en);
 
 	/* update in global extent list */
 	spin_lock(&sbi->extent_lock);
@@ -497,6 +494,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
 	struct rb_node **insert_p = NULL, *insert_parent = NULL;
 	unsigned int end = fofs + len;
 	unsigned int pos = (unsigned int)fofs;
+	bool updated = false;
 
 	if (!et)
 		return;
@@ -517,7 +515,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
 	 * drop largest extent before lookup, in case it's already
 	 * been shrunk from extent tree
 	 */
-	__drop_largest_extent(inode, fofs, len);
+	__drop_largest_extent(et, fofs, len);
 
 	/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
 	en = (struct extent_node *)f2fs_lookup_rb_tree_ret(&et->root,
@@ -550,7 +548,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
 				set_extent_info(&ei, end,
 						end - dei.fofs + dei.blk,
 						org_end - end);
-				en1 = __insert_extent_tree(inode, et, &ei,
+				en1 = __insert_extent_tree(sbi, et, &ei,
 							NULL, NULL);
 				next_en = en1;
 			} else {
@@ -570,7 +568,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
 		}
 
 		if (parts)
-			__try_update_largest_extent(inode, et, en);
+			__try_update_largest_extent(et, en);
 		else
 			__release_extent_node(sbi, et, en);
 
@@ -590,15 +588,16 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
 	if (blkaddr) {
 
 		set_extent_info(&ei, fofs, blkaddr, len);
-		if (!__try_merge_extent_node(inode, et, &ei, prev_en, next_en))
-			__insert_extent_tree(inode, et, &ei,
+		if (!__try_merge_extent_node(sbi, et, &ei, prev_en, next_en))
+			__insert_extent_tree(sbi, et, &ei,
 						insert_p, insert_parent);
 
 		/* give up extent_cache, if split and small updates happen */
 		if (dei.len >= 1 &&
 				prev.len < F2FS_MIN_EXTENT_LEN &&
 				et->largest.len < F2FS_MIN_EXTENT_LEN) {
-			__drop_largest_extent(inode, 0, UINT_MAX);
+			et->largest.len = 0;
+			et->largest_updated = true;
 			set_inode_flag(inode, FI_NO_EXTENT);
 		}
 	}
@@ -606,7 +605,15 @@ static void f2fs_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;
+	}
+
 	write_unlock(&et->lock);
+
+	if (updated)
+		f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
@@ -705,6 +712,7 @@ void f2fs_drop_extent_tree(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct extent_tree *et = F2FS_I(inode)->extent_tree;
+	bool updated = false;
 
 	if (!f2fs_may_extent_tree(inode))
 		return;
@@ -713,8 +721,13 @@ void f2fs_drop_extent_tree(struct inode *inode)
 
 	write_lock(&et->lock);
 	__free_extent_tree(sbi, et);
-	__drop_largest_extent(inode, 0, UINT_MAX);
+	if (et->largest.len) {
+		et->largest.len = 0;
+		updated = true;
+	}
 	write_unlock(&et->lock);
+	if (updated)
+		f2fs_mark_inode_dirty_sync(inode, true);
 }
 
 void f2fs_destroy_extent_tree(struct inode *inode)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index abf9256..e146e6c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -572,6 +572,7 @@ struct extent_tree {
 	struct list_head list;		/* to be used by sbi->zombie_list */
 	rwlock_t lock;			/* protect extent info rb-tree */
 	atomic_t node_cnt;		/* # of extent node in rb-tree*/
+	bool largest_updated;		/* largest extent updated */
 };
 
 /*
@@ -754,12 +755,12 @@ static inline bool __is_front_mergeable(struct extent_info *cur,
 }
 
 extern void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync);
-static inline void __try_update_largest_extent(struct inode *inode,
-			struct extent_tree *et, struct extent_node *en)
+static inline void __try_update_largest_extent(struct extent_tree *et,
+						struct extent_node *en)
 {
 	if (en->ei.len > et->largest.len) {
 		et->largest = en->ei;
-		f2fs_mark_inode_dirty_sync(inode, true);
+		et->largest_updated = true;
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] f2fs: avoid sleeping under spin_lock
  2018-09-10  8:18 [PATCH v3] f2fs: avoid sleeping under spin_lock Zhikang Zhang
@ 2018-09-10 14:34 ` Chao Yu
  2018-09-11 20:24   ` Jaegeuk Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2018-09-10 14:34 UTC (permalink / raw)
  To: Zhikang Zhang, jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

On 2018/9/10 16:18, Zhikang Zhang wrote:
> In the call trace below, we might sleep in function dput().
> 
> So in order to avoid sleeping under spin_lock, we remove f2fs_mark_inode_dirty_sync
> from __try_update_largest_extent && __drop_largest_extent.
> 
> BUG: sleeping function called from invalid context at fs/dcache.c:796
> Call trace:
> 	dump_backtrace+0x0/0x3f4
> 	show_stack+0x24/0x30
> 	dump_stack+0xe0/0x138
> 	___might_sleep+0x2a8/0x2c8
> 	__might_sleep+0x78/0x10c
> 	dput+0x7c/0x750
> 	block_dump___mark_inode_dirty+0x120/0x17c
> 	__mark_inode_dirty+0x344/0x11f0
> 	f2fs_mark_inode_dirty_sync+0x40/0x50
> 	__insert_extent_tree+0x2e0/0x2f4
> 	f2fs_update_extent_tree_range+0xcf4/0xde8
> 	f2fs_update_extent_cache+0x114/0x12c
> 	f2fs_update_data_blkaddr+0x40/0x50
> 	write_data_page+0x150/0x314
> 	do_write_data_page+0x648/0x2318
> 	__write_data_page+0xdb4/0x1640
> 	f2fs_write_cache_pages+0x768/0xafc
> 	__f2fs_write_data_pages+0x590/0x1218
> 	f2fs_write_data_pages+0x64/0x74
> 	do_writepages+0x74/0xe4
> 	__writeback_single_inode+0xdc/0x15f0
> 	writeback_sb_inodes+0x574/0xc98
> 	__writeback_inodes_wb+0x190/0x204
> 	wb_writeback+0x730/0xf14
> 	wb_check_old_data_flush+0x1bc/0x1c8
> 	wb_workfn+0x554/0xf74
> 	process_one_work+0x440/0x118c
> 	worker_thread+0xac/0x974
> 	kthread+0x1a0/0x1c8
> 	ret_from_fork+0x10/0x1c

Missed a Signed-off-by here?

Signed-off-by: Zhikang Zhang <zhangzhikang1@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>

I think Jaegeuk can help to add them when merging this patch. :)

Thanks,

> ---
>  fs/f2fs/extent_cache.c | 51 +++++++++++++++++++++++++++++++-------------------
>  fs/f2fs/f2fs.h         |  7 ++++---
>  2 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 231b77e..a70cd25 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -308,14 +308,13 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
>  	return count - atomic_read(&et->node_cnt);
>  }
>  
> -static void __drop_largest_extent(struct inode *inode,
> +static void __drop_largest_extent(struct extent_tree *et,
>  					pgoff_t fofs, unsigned int len)
>  {
> -	struct extent_info *largest = &F2FS_I(inode)->extent_tree->largest;
> -
> -	if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs) {
> -		largest->len = 0;
> -		f2fs_mark_inode_dirty_sync(inode, true);
> +	if (fofs < et->largest.fofs + et->largest.len &&
> +			fofs + len > et->largest.fofs) {
> +		et->largest.len = 0;
> +		et->largest_updated = true;
>  	}
>  }
>  
> @@ -416,12 +415,11 @@ static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
>  	return ret;
>  }
>  
> -static struct extent_node *__try_merge_extent_node(struct inode *inode,
> +static struct extent_node *__try_merge_extent_node(struct f2fs_sb_info *sbi,
>  				struct extent_tree *et, struct extent_info *ei,
>  				struct extent_node *prev_ex,
>  				struct extent_node *next_ex)
>  {
> -	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	struct extent_node *en = NULL;
>  
>  	if (prev_ex && __is_back_mergeable(ei, &prev_ex->ei)) {
> @@ -443,7 +441,7 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
>  	if (!en)
>  		return NULL;
>  
> -	__try_update_largest_extent(inode, et, en);
> +	__try_update_largest_extent(et, en);
>  
>  	spin_lock(&sbi->extent_lock);
>  	if (!list_empty(&en->list)) {
> @@ -454,12 +452,11 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
>  	return en;
>  }
>  
> -static struct extent_node *__insert_extent_tree(struct inode *inode,
> +static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>  				struct extent_tree *et, struct extent_info *ei,
>  				struct rb_node **insert_p,
>  				struct rb_node *insert_parent)
>  {
> -	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	struct rb_node **p;
>  	struct rb_node *parent = NULL;
>  	struct extent_node *en = NULL;
> @@ -476,7 +473,7 @@ static struct extent_node *__insert_extent_tree(struct inode *inode,
>  	if (!en)
>  		return NULL;
>  
> -	__try_update_largest_extent(inode, et, en);
> +	__try_update_largest_extent(et, en);
>  
>  	/* update in global extent list */
>  	spin_lock(&sbi->extent_lock);
> @@ -497,6 +494,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
>  	struct rb_node **insert_p = NULL, *insert_parent = NULL;
>  	unsigned int end = fofs + len;
>  	unsigned int pos = (unsigned int)fofs;
> +	bool updated = false;
>  
>  	if (!et)
>  		return;
> @@ -517,7 +515,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
>  	 * drop largest extent before lookup, in case it's already
>  	 * been shrunk from extent tree
>  	 */
> -	__drop_largest_extent(inode, fofs, len);
> +	__drop_largest_extent(et, fofs, len);
>  
>  	/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
>  	en = (struct extent_node *)f2fs_lookup_rb_tree_ret(&et->root,
> @@ -550,7 +548,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
>  				set_extent_info(&ei, end,
>  						end - dei.fofs + dei.blk,
>  						org_end - end);
> -				en1 = __insert_extent_tree(inode, et, &ei,
> +				en1 = __insert_extent_tree(sbi, et, &ei,
>  							NULL, NULL);
>  				next_en = en1;
>  			} else {
> @@ -570,7 +568,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
>  		}
>  
>  		if (parts)
> -			__try_update_largest_extent(inode, et, en);
> +			__try_update_largest_extent(et, en);
>  		else
>  			__release_extent_node(sbi, et, en);
>  
> @@ -590,15 +588,16 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
>  	if (blkaddr) {
>  
>  		set_extent_info(&ei, fofs, blkaddr, len);
> -		if (!__try_merge_extent_node(inode, et, &ei, prev_en, next_en))
> -			__insert_extent_tree(inode, et, &ei,
> +		if (!__try_merge_extent_node(sbi, et, &ei, prev_en, next_en))
> +			__insert_extent_tree(sbi, et, &ei,
>  						insert_p, insert_parent);
>  
>  		/* give up extent_cache, if split and small updates happen */
>  		if (dei.len >= 1 &&
>  				prev.len < F2FS_MIN_EXTENT_LEN &&
>  				et->largest.len < F2FS_MIN_EXTENT_LEN) {
> -			__drop_largest_extent(inode, 0, UINT_MAX);
> +			et->largest.len = 0;
> +			et->largest_updated = true;
>  			set_inode_flag(inode, FI_NO_EXTENT);
>  		}
>  	}
> @@ -606,7 +605,15 @@ static void f2fs_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;
> +	}
> +
>  	write_unlock(&et->lock);
> +
> +	if (updated)
> +		f2fs_mark_inode_dirty_sync(inode, true);
>  }
>  
>  unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
> @@ -705,6 +712,7 @@ void f2fs_drop_extent_tree(struct inode *inode)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	struct extent_tree *et = F2FS_I(inode)->extent_tree;
> +	bool updated = false;
>  
>  	if (!f2fs_may_extent_tree(inode))
>  		return;
> @@ -713,8 +721,13 @@ void f2fs_drop_extent_tree(struct inode *inode)
>  
>  	write_lock(&et->lock);
>  	__free_extent_tree(sbi, et);
> -	__drop_largest_extent(inode, 0, UINT_MAX);
> +	if (et->largest.len) {
> +		et->largest.len = 0;
> +		updated = true;
> +	}
>  	write_unlock(&et->lock);
> +	if (updated)
> +		f2fs_mark_inode_dirty_sync(inode, true);
>  }
>  
>  void f2fs_destroy_extent_tree(struct inode *inode)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index abf9256..e146e6c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -572,6 +572,7 @@ struct extent_tree {
>  	struct list_head list;		/* to be used by sbi->zombie_list */
>  	rwlock_t lock;			/* protect extent info rb-tree */
>  	atomic_t node_cnt;		/* # of extent node in rb-tree*/
> +	bool largest_updated;		/* largest extent updated */
>  };
>  
>  /*
> @@ -754,12 +755,12 @@ static inline bool __is_front_mergeable(struct extent_info *cur,
>  }
>  
>  extern void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync);
> -static inline void __try_update_largest_extent(struct inode *inode,
> -			struct extent_tree *et, struct extent_node *en)
> +static inline void __try_update_largest_extent(struct extent_tree *et,
> +						struct extent_node *en)
>  {
>  	if (en->ei.len > et->largest.len) {
>  		et->largest = en->ei;
> -		f2fs_mark_inode_dirty_sync(inode, true);
> +		et->largest_updated = true;
>  	}
>  }
>  
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] f2fs: avoid sleeping under spin_lock
  2018-09-10 14:34 ` Chao Yu
@ 2018-09-11 20:24   ` Jaegeuk Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Jaegeuk Kim @ 2018-09-11 20:24 UTC (permalink / raw)
  To: Chao Yu; +Cc: miaoxie, linux-f2fs-devel

On 09/10, Chao Yu wrote:
> On 2018/9/10 16:18, Zhikang Zhang wrote:
> > In the call trace below, we might sleep in function dput().
> > 
> > So in order to avoid sleeping under spin_lock, we remove f2fs_mark_inode_dirty_sync
> > from __try_update_largest_extent && __drop_largest_extent.
> > 
> > BUG: sleeping function called from invalid context at fs/dcache.c:796
> > Call trace:
> > 	dump_backtrace+0x0/0x3f4
> > 	show_stack+0x24/0x30
> > 	dump_stack+0xe0/0x138
> > 	___might_sleep+0x2a8/0x2c8
> > 	__might_sleep+0x78/0x10c
> > 	dput+0x7c/0x750
> > 	block_dump___mark_inode_dirty+0x120/0x17c
> > 	__mark_inode_dirty+0x344/0x11f0
> > 	f2fs_mark_inode_dirty_sync+0x40/0x50
> > 	__insert_extent_tree+0x2e0/0x2f4
> > 	f2fs_update_extent_tree_range+0xcf4/0xde8
> > 	f2fs_update_extent_cache+0x114/0x12c
> > 	f2fs_update_data_blkaddr+0x40/0x50
> > 	write_data_page+0x150/0x314
> > 	do_write_data_page+0x648/0x2318
> > 	__write_data_page+0xdb4/0x1640
> > 	f2fs_write_cache_pages+0x768/0xafc
> > 	__f2fs_write_data_pages+0x590/0x1218
> > 	f2fs_write_data_pages+0x64/0x74
> > 	do_writepages+0x74/0xe4
> > 	__writeback_single_inode+0xdc/0x15f0
> > 	writeback_sb_inodes+0x574/0xc98
> > 	__writeback_inodes_wb+0x190/0x204
> > 	wb_writeback+0x730/0xf14
> > 	wb_check_old_data_flush+0x1bc/0x1c8
> > 	wb_workfn+0x554/0xf74
> > 	process_one_work+0x440/0x118c
> > 	worker_thread+0xac/0x974
> > 	kthread+0x1a0/0x1c8
> > 	ret_from_fork+0x10/0x1c
> 
> Missed a Signed-off-by here?
> 
> Signed-off-by: Zhikang Zhang <zhangzhikang1@huawei.com>
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> I think Jaegeuk can help to add them when merging this patch. :)

Done. :P

> 
> Thanks,
> 
> > ---
> >  fs/f2fs/extent_cache.c | 51 +++++++++++++++++++++++++++++++-------------------
> >  fs/f2fs/f2fs.h         |  7 ++++---
> >  2 files changed, 36 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> > index 231b77e..a70cd25 100644
> > --- a/fs/f2fs/extent_cache.c
> > +++ b/fs/f2fs/extent_cache.c
> > @@ -308,14 +308,13 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
> >  	return count - atomic_read(&et->node_cnt);
> >  }
> >  
> > -static void __drop_largest_extent(struct inode *inode,
> > +static void __drop_largest_extent(struct extent_tree *et,
> >  					pgoff_t fofs, unsigned int len)
> >  {
> > -	struct extent_info *largest = &F2FS_I(inode)->extent_tree->largest;
> > -
> > -	if (fofs < largest->fofs + largest->len && fofs + len > largest->fofs) {
> > -		largest->len = 0;
> > -		f2fs_mark_inode_dirty_sync(inode, true);
> > +	if (fofs < et->largest.fofs + et->largest.len &&
> > +			fofs + len > et->largest.fofs) {
> > +		et->largest.len = 0;
> > +		et->largest_updated = true;
> >  	}
> >  }
> >  
> > @@ -416,12 +415,11 @@ static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
> >  	return ret;
> >  }
> >  
> > -static struct extent_node *__try_merge_extent_node(struct inode *inode,
> > +static struct extent_node *__try_merge_extent_node(struct f2fs_sb_info *sbi,
> >  				struct extent_tree *et, struct extent_info *ei,
> >  				struct extent_node *prev_ex,
> >  				struct extent_node *next_ex)
> >  {
> > -	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >  	struct extent_node *en = NULL;
> >  
> >  	if (prev_ex && __is_back_mergeable(ei, &prev_ex->ei)) {
> > @@ -443,7 +441,7 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
> >  	if (!en)
> >  		return NULL;
> >  
> > -	__try_update_largest_extent(inode, et, en);
> > +	__try_update_largest_extent(et, en);
> >  
> >  	spin_lock(&sbi->extent_lock);
> >  	if (!list_empty(&en->list)) {
> > @@ -454,12 +452,11 @@ static struct extent_node *__try_merge_extent_node(struct inode *inode,
> >  	return en;
> >  }
> >  
> > -static struct extent_node *__insert_extent_tree(struct inode *inode,
> > +static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
> >  				struct extent_tree *et, struct extent_info *ei,
> >  				struct rb_node **insert_p,
> >  				struct rb_node *insert_parent)
> >  {
> > -	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >  	struct rb_node **p;
> >  	struct rb_node *parent = NULL;
> >  	struct extent_node *en = NULL;
> > @@ -476,7 +473,7 @@ static struct extent_node *__insert_extent_tree(struct inode *inode,
> >  	if (!en)
> >  		return NULL;
> >  
> > -	__try_update_largest_extent(inode, et, en);
> > +	__try_update_largest_extent(et, en);
> >  
> >  	/* update in global extent list */
> >  	spin_lock(&sbi->extent_lock);
> > @@ -497,6 +494,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
> >  	struct rb_node **insert_p = NULL, *insert_parent = NULL;
> >  	unsigned int end = fofs + len;
> >  	unsigned int pos = (unsigned int)fofs;
> > +	bool updated = false;
> >  
> >  	if (!et)
> >  		return;
> > @@ -517,7 +515,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
> >  	 * drop largest extent before lookup, in case it's already
> >  	 * been shrunk from extent tree
> >  	 */
> > -	__drop_largest_extent(inode, fofs, len);
> > +	__drop_largest_extent(et, fofs, len);
> >  
> >  	/* 1. lookup first extent node in range [fofs, fofs + len - 1] */
> >  	en = (struct extent_node *)f2fs_lookup_rb_tree_ret(&et->root,
> > @@ -550,7 +548,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
> >  				set_extent_info(&ei, end,
> >  						end - dei.fofs + dei.blk,
> >  						org_end - end);
> > -				en1 = __insert_extent_tree(inode, et, &ei,
> > +				en1 = __insert_extent_tree(sbi, et, &ei,
> >  							NULL, NULL);
> >  				next_en = en1;
> >  			} else {
> > @@ -570,7 +568,7 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
> >  		}
> >  
> >  		if (parts)
> > -			__try_update_largest_extent(inode, et, en);
> > +			__try_update_largest_extent(et, en);
> >  		else
> >  			__release_extent_node(sbi, et, en);
> >  
> > @@ -590,15 +588,16 @@ static void f2fs_update_extent_tree_range(struct inode *inode,
> >  	if (blkaddr) {
> >  
> >  		set_extent_info(&ei, fofs, blkaddr, len);
> > -		if (!__try_merge_extent_node(inode, et, &ei, prev_en, next_en))
> > -			__insert_extent_tree(inode, et, &ei,
> > +		if (!__try_merge_extent_node(sbi, et, &ei, prev_en, next_en))
> > +			__insert_extent_tree(sbi, et, &ei,
> >  						insert_p, insert_parent);
> >  
> >  		/* give up extent_cache, if split and small updates happen */
> >  		if (dei.len >= 1 &&
> >  				prev.len < F2FS_MIN_EXTENT_LEN &&
> >  				et->largest.len < F2FS_MIN_EXTENT_LEN) {
> > -			__drop_largest_extent(inode, 0, UINT_MAX);
> > +			et->largest.len = 0;
> > +			et->largest_updated = true;
> >  			set_inode_flag(inode, FI_NO_EXTENT);
> >  		}
> >  	}
> > @@ -606,7 +605,15 @@ static void f2fs_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;
> > +	}
> > +
> >  	write_unlock(&et->lock);
> > +
> > +	if (updated)
> > +		f2fs_mark_inode_dirty_sync(inode, true);
> >  }
> >  
> >  unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
> > @@ -705,6 +712,7 @@ void f2fs_drop_extent_tree(struct inode *inode)
> >  {
> >  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >  	struct extent_tree *et = F2FS_I(inode)->extent_tree;
> > +	bool updated = false;
> >  
> >  	if (!f2fs_may_extent_tree(inode))
> >  		return;
> > @@ -713,8 +721,13 @@ void f2fs_drop_extent_tree(struct inode *inode)
> >  
> >  	write_lock(&et->lock);
> >  	__free_extent_tree(sbi, et);
> > -	__drop_largest_extent(inode, 0, UINT_MAX);
> > +	if (et->largest.len) {
> > +		et->largest.len = 0;
> > +		updated = true;
> > +	}
> >  	write_unlock(&et->lock);
> > +	if (updated)
> > +		f2fs_mark_inode_dirty_sync(inode, true);
> >  }
> >  
> >  void f2fs_destroy_extent_tree(struct inode *inode)
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index abf9256..e146e6c 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -572,6 +572,7 @@ struct extent_tree {
> >  	struct list_head list;		/* to be used by sbi->zombie_list */
> >  	rwlock_t lock;			/* protect extent info rb-tree */
> >  	atomic_t node_cnt;		/* # of extent node in rb-tree*/
> > +	bool largest_updated;		/* largest extent updated */
> >  };
> >  
> >  /*
> > @@ -754,12 +755,12 @@ static inline bool __is_front_mergeable(struct extent_info *cur,
> >  }
> >  
> >  extern void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync);
> > -static inline void __try_update_largest_extent(struct inode *inode,
> > -			struct extent_tree *et, struct extent_node *en)
> > +static inline void __try_update_largest_extent(struct extent_tree *et,
> > +						struct extent_node *en)
> >  {
> >  	if (en->ei.len > et->largest.len) {
> >  		et->largest = en->ei;
> > -		f2fs_mark_inode_dirty_sync(inode, true);
> > +		et->largest_updated = true;
> >  	}
> >  }
> >  
> > 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-09-11 20:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10  8:18 [PATCH v3] f2fs: avoid sleeping under spin_lock Zhikang Zhang
2018-09-10 14:34 ` Chao Yu
2018-09-11 20:24   ` Jaegeuk Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.