[PATCH 01/12] f2fs: avoid freed stat information

[PATCH 01/12] f2fs: avoid freed stat information

[Jaegeuk Kim]

The write_checkpoint can update stat information, so we should destroy the stat
structure after it.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a06b0b4..da27710 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -498,7 +498,6 @@ static void f2fs_put_super(struct super_block *sb)
 	}
 	kobject_del(&sbi->s_kobj);
 
-	f2fs_destroy_stats(sbi);
 	stop_gc_thread(sbi);
 
 	/*
@@ -514,6 +513,9 @@ static void f2fs_put_super(struct super_block *sb)
 		write_checkpoint(sbi, &cpc);
 	}
 
+	/* write_checkpoint can update stat informaion */
+	f2fs_destroy_stats(sbi);
+
 	/*
 	 * normally superblock is clean, so we need to release this.
 	 * In addition, EIO will skip do checkpoint, we need this as well.
-- 
2.1.1

[PATCH 02/12] f2fs: avoid to use failed inode immediately

[Jaegeuk Kim]

Before iput is called, the inode number used by a bad inode can be reassigned
to other new inode, resulting in any abnormal behaviors on the new inode.
This should not happen for the new inode.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h  |  1 +
 fs/f2fs/inode.c | 19 ++++++++++++-------
 fs/f2fs/namei.c |  4 ++--
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5162204..3aaa4b9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1343,6 +1343,7 @@ enum {
 	FI_INC_LINK,		/* need to increment i_nlink */
 	FI_ACL_MODE,		/* indicate acl mode */
 	FI_NO_ALLOC,		/* should not allocate any blocks */
+	FI_FREE_NID,		/* free allocated nide */
 	FI_UPDATE_DIR,		/* should update inode block for consistency */
 	FI_DELAY_IPUT,		/* used for the recovery */
 	FI_NO_EXTENT,		/* not to use the extent cache */
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 2550868..757fed2 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -314,7 +314,8 @@ int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
 void f2fs_evict_inode(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	nid_t xnid = F2FS_I(inode)->i_xattr_nid;
+	struct f2fs_inode_info *fi = F2FS_I(inode);
+	nid_t xnid = fi->i_xattr_nid;
 
 	/* some remained atomic pages should discarded */
 	if (f2fs_is_atomic_file(inode))
@@ -334,7 +335,7 @@ void f2fs_evict_inode(struct inode *inode)
 		goto no_delete;
 
 	sb_start_intwrite(inode->i_sb);
-	set_inode_flag(F2FS_I(inode), FI_NO_ALLOC);
+	set_inode_flag(fi, FI_NO_ALLOC);
 	i_size_write(inode, 0);
 
 	if (F2FS_HAS_BLOCKS(inode))
@@ -357,14 +358,18 @@ no_delete:
 	invalidate_mapping_pages(NODE_MAPPING(sbi), inode->i_ino, inode->i_ino);
 	if (xnid)
 		invalidate_mapping_pages(NODE_MAPPING(sbi), xnid, xnid);
-	if (is_inode_flag_set(F2FS_I(inode), FI_APPEND_WRITE))
+	if (is_inode_flag_set(fi, FI_APPEND_WRITE))
 		add_dirty_inode(sbi, inode->i_ino, APPEND_INO);
-	if (is_inode_flag_set(F2FS_I(inode), FI_UPDATE_WRITE))
+	if (is_inode_flag_set(fi, FI_UPDATE_WRITE))
 		add_dirty_inode(sbi, inode->i_ino, UPDATE_INO);
+	if (is_inode_flag_set(fi, FI_FREE_NID)) {
+		alloc_nid_failed(sbi, inode->i_ino);
+		clear_inode_flag(fi, FI_FREE_NID);
+	}
 out_clear:
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
-	if (F2FS_I(inode)->i_crypt_info)
-		f2fs_free_encryption_info(inode, F2FS_I(inode)->i_crypt_info);
+	if (fi->i_crypt_info)
+		f2fs_free_encryption_info(inode, fi->i_crypt_info);
 #endif
 	clear_inode(inode);
 }
@@ -384,9 +389,9 @@ void handle_failed_inode(struct inode *inode)
 
 	remove_inode_page(inode);
 
+	set_inode_flag(F2FS_I(inode), FI_FREE_NID);
 	clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
 	clear_inode_flag(F2FS_I(inode), FI_INLINE_DENTRY);
-	alloc_nid_failed(sbi, inode->i_ino);
 	f2fs_unlock_op(sbi);
 
 	/* iput will drop the inode object */
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index fdbae21..08656fc 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -78,9 +78,9 @@ out:
 fail:
 	trace_f2fs_new_inode(inode, err);
 	make_bad_inode(inode);
-	iput(inode);
 	if (nid_free)
-		alloc_nid_failed(sbi, ino);
+		set_inode_flag(F2FS_I(inode), FI_FREE_NID);
+	iput(inode);
 	return ERR_PTR(err);
 }
 
-- 
2.1.1

RE: [f2fs-dev] [PATCH 02/12] f2fs: avoid to use failed inode immediately

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:39 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 02/12] f2fs: avoid to use failed inode immediately
> 
> Before iput is called, the inode number used by a bad inode can be reassigned
> to other new inode, resulting in any abnormal behaviors on the new inode.
> This should not happen for the new inode.

Nice catch!

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 03/12] f2fs: convert inline_data for various fallocate

[Jaegeuk Kim]

For newly added fallocate types, it should convert inline_data before handling
block swapping.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/file.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ada2a3d..1b67215 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -885,6 +885,14 @@ static int f2fs_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	if (offset & (F2FS_BLKSIZE - 1) || len & (F2FS_BLKSIZE - 1))
 		return -EINVAL;
 
+	f2fs_balance_fs(F2FS_I_SB(inode));
+
+	if (f2fs_has_inline_data(inode)) {
+		ret = f2fs_convert_inline_inode(inode);
+		if (ret)
+			return ret;
+	}
+
 	pg_start = offset >> PAGE_CACHE_SHIFT;
 	pg_end = (offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -1033,6 +1041,12 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 
 	f2fs_balance_fs(sbi);
 
+	if (f2fs_has_inline_data(inode)) {
+		ret = f2fs_convert_inline_inode(inode);
+		if (ret)
+			return ret;
+	}
+
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
 		return ret;
-- 
2.1.1

RE: [f2fs-dev] [PATCH 03/12] f2fs: convert inline_data for various fallocate

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:39 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 03/12] f2fs: convert inline_data for various fallocate
> 
> For newly added fallocate types, it should convert inline_data before handling
> block swapping.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 04/12] f2fs: remove wrong f2fs_bug_on when merging extents

[Jaegeuk Kim]

In f2fs_update_extent_tree, if there is existing extent, f2fs tries to split
it with two parts.
In each trial, __insert_extent_tree checks __is_front/back_mergeable, and then
if it hits to go, there is f2fs_bug_on(!den), which triggers a kernel panic.

Actually, we don't need to check this. Instead, we can do __try_back_merge only
when there exists a den pointer.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9bedfa8..7817167 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -519,19 +519,19 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 
 		if (ei->fofs < en->ei.fofs) {
 			if (__is_front_mergeable(ei, &en->ei)) {
-				f2fs_bug_on(sbi, !den);
 				en->ei.fofs = ei->fofs;
 				en->ei.blk = ei->blk;
 				en->ei.len += ei->len;
-				*den = __try_back_merge(sbi, et, en);
+				if (den)
+					*den = __try_back_merge(sbi, et, en);
 				return en;
 			}
 			p = &(*p)->rb_left;
 		} else if (ei->fofs >= en->ei.fofs + en->ei.len) {
 			if (__is_back_mergeable(ei, &en->ei)) {
-				f2fs_bug_on(sbi, !den);
 				en->ei.len += ei->len;
-				*den = __try_front_merge(sbi, et, en);
+				if (den)
+					*den = __try_front_merge(sbi, et, en);
 				return en;
 			}
 			p = &(*p)->rb_right;
-- 
2.1.1

RE: [f2fs-dev] [PATCH 04/12] f2fs: remove wrong f2fs_bug_on when merging extents

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:39 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 04/12] f2fs: remove wrong f2fs_bug_on when merging extents
> 
> In f2fs_update_extent_tree, if there is existing extent, f2fs tries to split
> it with two parts.
> In each trial, __insert_extent_tree checks __is_front/back_mergeable, and then
> if it hits to go, there is f2fs_bug_on(!den), which triggers a kernel panic.
> 
> Actually, we don't need to check this. Instead, we can do __try_back_merge only
> when there exists a den pointer.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9bedfa8..7817167 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -519,19 +519,19 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
> 
>  		if (ei->fofs < en->ei.fofs) {
>  			if (__is_front_mergeable(ei, &en->ei)) {
> -				f2fs_bug_on(sbi, !den);

I add a BUG_ON here because we assume that in extent cache there is no such 
two extents whose mapping address is continuous but without being merged,
since whenever we add a new extent(not splitted one) into cache, we tries to
merge it frontward/backward, then all extent with continuous mapping should be
merged. So when we split one extent to two parts, each part should not be able
to merge with others. Otherwise it should be a bug.

Is there some special case to trigger this BUG_ON?

Thanks,

>  				en->ei.fofs = ei->fofs;
>  				en->ei.blk = ei->blk;
>  				en->ei.len += ei->len;
> -				*den = __try_back_merge(sbi, et, en);
> +				if (den)
> +					*den = __try_back_merge(sbi, et, en);
>  				return en;
>  			}
>  			p = &(*p)->rb_left;
>  		} else if (ei->fofs >= en->ei.fofs + en->ei.len) {
>  			if (__is_back_mergeable(ei, &en->ei)) {
> -				f2fs_bug_on(sbi, !den);
>  				en->ei.len += ei->len;
> -				*den = __try_front_merge(sbi, et, en);
> +				if (den)
> +					*den = __try_front_merge(sbi, et, en);
>  				return en;
>  			}
>  			p = &(*p)->rb_right;
> --
> 2.1.1
> 
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 04/12] f2fs: remove wrong f2fs_bug_on when merging extents

[Jaegeuk Kim]

On Tue, Jun 30, 2015 at 11:00:13AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Tuesday, June 30, 2015 2:39 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 04/12] f2fs: remove wrong f2fs_bug_on when merging extents
> > 
> > In f2fs_update_extent_tree, if there is existing extent, f2fs tries to split
> > it with two parts.
> > In each trial, __insert_extent_tree checks __is_front/back_mergeable, and then
> > if it hits to go, there is f2fs_bug_on(!den), which triggers a kernel panic.
> > 
> > Actually, we don't need to check this. Instead, we can do __try_back_merge only
> > when there exists a den pointer.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/data.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 9bedfa8..7817167 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -519,19 +519,19 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
> > 
> >  		if (ei->fofs < en->ei.fofs) {
> >  			if (__is_front_mergeable(ei, &en->ei)) {
> > -				f2fs_bug_on(sbi, !den);
> 
> I add a BUG_ON here because we assume that in extent cache there is no such 
> two extents whose mapping address is continuous but without being merged,
> since whenever we add a new extent(not splitted one) into cache, we tries to
> merge it frontward/backward, then all extent with continuous mapping should be
> merged. So when we split one extent to two parts, each part should not be able
> to merge with others. Otherwise it should be a bug.

Oh, I see.
Let me take a look at this one more time.
I'm confusing whether this patch was written before fixing block address
calculation and then change the order of the patches.

Thanks,

[PATCH 05/12] f2fs: fix wrong block address calculation for a split extent

[Jaegeuk Kim]

This patch fixes wrong calculation on block address field when an extent is
split.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7817167..dd62228 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -674,7 +674,7 @@ static void f2fs_update_extent_tree(struct inode *inode, pgoff_t fofs,
 		endofs = dei.fofs + dei.len - 1;
 		if (endofs - fofs >= F2FS_MIN_EXTENT_LEN) {
 			set_extent_info(&ei, fofs + 1,
-				fofs - dei.fofs + dei.blk, endofs - fofs);
+				fofs - dei.fofs + dei.blk + 1, endofs - fofs);
 			en2 = __insert_extent_tree(sbi, et, &ei, NULL);
 		}
 	}
-- 
2.1.1

RE: [f2fs-dev] [PATCH 05/12] f2fs: fix wrong block address calculation for a split extent

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:39 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 05/12] f2fs: fix wrong block address calculation for a split extent
> 
> This patch fixes wrong calculation on block address field when an extent is
> split.

Oh, that's a terrible mistake, sorry about it.
Thank you for fixing this.

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 06/12] f2fs: update on-disk extents even under extent_cache

[Jaegeuk Kim]

Previously, f2fs_update_extent_cache() updates in-memory extent_cache all the
time, and then finally preserves its up-to-date extent into on-disk one during
f2fs_evict_inode.

But, in the following scenario:

1. mount
2. open & write an extent X
3. f2fs_evict_inode; on-disk extent is X
4. open & update the extent X with Y
5. sync; trigger checkpoint
6. power-cut

after power-on, f2fs should serve extent Y, but we have an on-disk extent X.

This causes a failure on xfstests/311.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index dd62228..1e6609b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -899,9 +899,9 @@ void f2fs_update_extent_cache(struct dnode_of_data *dn)
 	fofs = start_bidx_of_node(ofs_of_node(dn->node_page), fi) +
 							dn->ofs_in_node;
 
+	/* we should call update_extent_info() to update on-disk extent */
 	if (test_opt(F2FS_I_SB(dn->inode), EXTENT_CACHE))
-		return f2fs_update_extent_tree(dn->inode, fofs,
-							dn->data_blkaddr);
+		f2fs_update_extent_tree(dn->inode, fofs, dn->data_blkaddr);
 
 	if (update_extent_info(dn->inode, fofs, dn->data_blkaddr))
 		sync_inode_page(dn);
-- 
2.1.1

RE: [f2fs-dev] [PATCH 06/12] f2fs: update on-disk extents even under extent_cache

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 06/12] f2fs: update on-disk extents even under extent_cache
> 
> Previously, f2fs_update_extent_cache() updates in-memory extent_cache all the
> time, and then finally preserves its up-to-date extent into on-disk one during
> f2fs_evict_inode.
> 
> But, in the following scenario:
> 
> 1. mount
> 2. open & write an extent X
> 3. f2fs_evict_inode; on-disk extent is X
> 4. open & update the extent X with Y
> 5. sync; trigger checkpoint
> 6. power-cut
> 
> after power-on, f2fs should serve extent Y, but we have an on-disk extent X.
> 
> This causes a failure on xfstests/311.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 07/12] f2fs: set cached_en after checking finally

[Jaegeuk Kim]

This patch relocates cached_en not only to be covered by spin_lock, but also
to set once after checking out completely.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1e6609b..676e7b9 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -453,14 +453,12 @@ static struct extent_node *__lookup_extent_tree(struct extent_tree *et,
 	while (node) {
 		en = rb_entry(node, struct extent_node, rb_node);
 
-		if (fofs < en->ei.fofs) {
+		if (fofs < en->ei.fofs)
 			node = node->rb_left;
-		} else if (fofs >= en->ei.fofs + en->ei.len) {
+		else if (fofs >= en->ei.fofs + en->ei.len)
 			node = node->rb_right;
-		} else {
-			et->cached_en = en;
+		else
 			return en;
-		}
 	}
 	return NULL;
 }
@@ -625,6 +623,7 @@ static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
 		spin_lock(&sbi->extent_lock);
 		if (!list_empty(&en->list))
 			list_move_tail(&en->list, &sbi->extent_list);
+		et->cached_en = en;
 		spin_unlock(&sbi->extent_lock);
 		stat_inc_read_hit(sbi->sb);
 	}
-- 
2.1.1

RE: [f2fs-dev] [PATCH 07/12] f2fs: set cached_en after checking finally

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 07/12] f2fs: set cached_en after checking finally
> 
> This patch relocates cached_en not only to be covered by spin_lock, but also
> to set once after checking out completely.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 08/12] f2fs: introduce a shrinker for mounted fs

[Jaegeuk Kim]

This patch introduces a shrinker targeting to reduce memory footprint consumed
by a number of in-memory f2fs data structures.

In addition, it newly adds:
 - sbi->umount_mutex to avoid data races on shrinker and put_super
 - sbi->shruinker_run_no to not revisit objects

Noteh that the basic implementation was copied from fs/btrfs/shrinker.c

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/Makefile   |   1 +
 fs/f2fs/f2fs.h     |  13 +++++++
 fs/f2fs/shrinker.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/f2fs/super.c    |  24 +++++++++++++
 4 files changed, 142 insertions(+)
 create mode 100644 fs/f2fs/shrinker.c

diff --git a/fs/f2fs/Makefile b/fs/f2fs/Makefile
index 396be1a..005251b 100644
--- a/fs/f2fs/Makefile
+++ b/fs/f2fs/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_F2FS_FS) += f2fs.o
 
 f2fs-y		:= dir.o file.o inode.o namei.o hash.o super.o inline.o
 f2fs-y		+= checkpoint.o gc.o data.o node.o segment.o recovery.o
+f2fs-y		+= shrinker.o
 f2fs-$(CONFIG_F2FS_STAT_FS) += debug.o
 f2fs-$(CONFIG_F2FS_FS_XATTR) += xattr.o
 f2fs-$(CONFIG_F2FS_FS_POSIX_ACL) += acl.o
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3aaa4b9..e82af8c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -791,6 +791,11 @@ struct f2fs_sb_info {
 	/* For sysfs suppport */
 	struct kobject s_kobj;
 	struct completion s_kobj_unregister;
+
+	/* For shrinker support */
+	struct list_head s_list;
+	struct mutex umount_mutex;
+	unsigned int shrinker_run_no;
 };
 
 /*
@@ -1952,6 +1957,14 @@ int f2fs_read_inline_dir(struct file *, struct dir_context *,
 						struct f2fs_str *);
 
 /*
+ * shrinker.c
+ */
+unsigned long f2fs_shrink_count(struct shrinker *, struct shrink_control *);
+unsigned long f2fs_shrink_scan(struct shrinker *, struct shrink_control *);
+void f2fs_join_shrinker(struct f2fs_sb_info *);
+void f2fs_leave_shrinker(struct f2fs_sb_info *);
+
+/*
  * crypto support
  */
 static inline int f2fs_encrypted_inode(struct inode *inode)
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
new file mode 100644
index 0000000..a680145
--- /dev/null
+++ b/fs/f2fs/shrinker.c
@@ -0,0 +1,104 @@
+/*
+ * f2fs shrinker support
+ *   the basic infra was copied from fs/btrfs/shrinker.c
+ *
+ * Copyright (c) 2015 Motorola Mobility
+ * Copyright (c) 2015 Jaegeuk Kim <jaegeuk@kernel.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/f2fs_fs.h>
+
+#include "f2fs.h"
+
+static LIST_HEAD(f2fs_list);
+static DEFINE_SPINLOCK(f2fs_list_lock);
+static unsigned int shrinker_run_no;
+
+unsigned long f2fs_shrink_count(struct shrinker *shrink,
+				struct shrink_control *sc)
+{
+	struct f2fs_sb_info *sbi;
+	struct list_head *p;
+	unsigned long count = 0;
+
+	spin_lock(&f2fs_list_lock);
+	p = f2fs_list.next;
+	while (p != &f2fs_list) {
+		sbi = list_entry(p, struct f2fs_sb_info, s_list);
+
+		/* stop f2fs_put_super */
+		if (!mutex_trylock(&sbi->umount_mutex)) {
+			p = p->next;
+			continue;
+		}
+		spin_unlock(&f2fs_list_lock);
+
+		/* TODO: count # of objects */
+
+		spin_lock(&f2fs_list_lock);
+		p = p->next;
+		mutex_unlock(&sbi->umount_mutex);
+	}
+	spin_unlock(&f2fs_list_lock);
+	return count;
+}
+
+unsigned long f2fs_shrink_scan(struct shrinker *shrink,
+				struct shrink_control *sc)
+{
+	unsigned long nr = sc->nr_to_scan;
+	struct f2fs_sb_info *sbi;
+	struct list_head *p;
+	unsigned int run_no;
+	unsigned long freed = 0;
+
+	spin_lock(&f2fs_list_lock);
+	do {
+		run_no = ++shrinker_run_no;
+	} while (run_no == 0);
+	p = f2fs_list.next;
+	while (p != &f2fs_list) {
+		sbi = list_entry(p, struct f2fs_sb_info, s_list);
+
+		if (sbi->shrinker_run_no == run_no)
+			break;
+
+		/* stop f2fs_put_super */
+		if (!mutex_trylock(&sbi->umount_mutex)) {
+			p = p->next;
+			continue;
+		}
+		spin_unlock(&f2fs_list_lock);
+
+		sbi->shrinker_run_no = run_no;
+
+		/* TODO: shrink caches */
+
+		spin_lock(&f2fs_list_lock);
+		p = p->next;
+		list_move_tail(&sbi->s_list, &f2fs_list);
+		mutex_unlock(&sbi->umount_mutex);
+		if (freed >= nr)
+			break;
+	}
+	spin_unlock(&f2fs_list_lock);
+	return freed;
+}
+
+void f2fs_join_shrinker(struct f2fs_sb_info *sbi)
+{
+	spin_lock(&f2fs_list_lock);
+	list_add_tail(&sbi->s_list, &f2fs_list);
+	spin_unlock(&f2fs_list_lock);
+}
+
+void f2fs_leave_shrinker(struct f2fs_sb_info *sbi)
+{
+	spin_lock(&f2fs_list_lock);
+	list_del(&sbi->s_list);
+	spin_unlock(&f2fs_list_lock);
+}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index da27710..2e8645e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -39,6 +39,13 @@ static struct proc_dir_entry *f2fs_proc_root;
 static struct kmem_cache *f2fs_inode_cachep;
 static struct kset *f2fs_kset;
 
+/* f2fs-wide shrinker description */
+static struct shrinker f2fs_shrinker_info = {
+	.scan_objects = f2fs_shrink_scan,
+	.count_objects = f2fs_shrink_count,
+	.seeks = DEFAULT_SEEKS,
+};
+
 enum {
 	Opt_gc_background,
 	Opt_disable_roll_forward,
@@ -500,6 +507,9 @@ static void f2fs_put_super(struct super_block *sb)
 
 	stop_gc_thread(sbi);
 
+	/* prevent remaining shrinker jobs */
+	mutex_lock(&sbi->umount_mutex);
+
 	/*
 	 * We don't need to do checkpoint when superblock is clean.
 	 * But, the previous checkpoint was not done by umount, it needs to do
@@ -523,6 +533,9 @@ static void f2fs_put_super(struct super_block *sb)
 	release_dirty_inode(sbi);
 	release_discard_addrs(sbi);
 
+	f2fs_leave_shrinker(sbi);
+	mutex_unlock(&sbi->umount_mutex);
+
 	iput(sbi->node_inode);
 	iput(sbi->meta_inode);
 
@@ -972,6 +985,9 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	sbi->dir_level = DEF_DIR_LEVEL;
 	clear_sbi_flag(sbi, SBI_NEED_FSCK);
+
+	INIT_LIST_HEAD(&sbi->s_list);
+	mutex_init(&sbi->umount_mutex);
 }
 
 /*
@@ -1214,6 +1230,8 @@ try_onemore:
 		goto free_nm;
 	}
 
+	f2fs_join_shrinker(sbi);
+
 	/* if there are nt orphan nodes free them */
 	recover_orphan_inodes(sbi);
 
@@ -1310,6 +1328,7 @@ free_root_inode:
 	dput(sb->s_root);
 	sb->s_root = NULL;
 free_node_inode:
+	f2fs_leave_shrinker(sbi);
 	iput(sbi->node_inode);
 free_nm:
 	destroy_node_manager(sbi);
@@ -1406,6 +1425,9 @@ static int __init init_f2fs_fs(void)
 	err = f2fs_init_crypto();
 	if (err)
 		goto free_kset;
+
+	register_shrinker(&f2fs_shrinker_info);
+
 	err = register_filesystem(&f2fs_fs_type);
 	if (err)
 		goto free_crypto;
@@ -1414,6 +1436,7 @@ static int __init init_f2fs_fs(void)
 	return 0;
 
 free_crypto:
+	unregister_shrinker(&f2fs_shrinker_info);
 	f2fs_exit_crypto();
 free_kset:
 	kset_unregister(f2fs_kset);
@@ -1435,6 +1458,7 @@ static void __exit exit_f2fs_fs(void)
 {
 	remove_proc_entry("fs/f2fs", NULL);
 	f2fs_destroy_root_stats();
+	unregister_shrinker(&f2fs_shrinker_info);
 	unregister_filesystem(&f2fs_fs_type);
 	f2fs_exit_crypto();
 	destroy_extent_cache();
-- 
2.1.1

RE: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs
> 
> This patch introduces a shrinker targeting to reduce memory footprint consumed
> by a number of in-memory f2fs data structures.
> 
> In addition, it newly adds:
>  - sbi->umount_mutex to avoid data races on shrinker and put_super
>  - sbi->shruinker_run_no to not revisit objects
> 
> Noteh that the basic implementation was copied from fs/btrfs/shrinker.c

Great! Good to see it's being implemented in f2fs.

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[snip]

> @@ -1406,6 +1425,9 @@ static int __init init_f2fs_fs(void)
>  	err = f2fs_init_crypto();
>  	if (err)
>  		goto free_kset;
> +
> +	register_shrinker(&f2fs_shrinker_info);

This function can fail due to no memory, please check the return value here.

Thanks,

Re: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs

[Jaegeuk Kim]

On Tue, Jun 30, 2015 at 11:43:29AM +0800, Chao Yu wrote:
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Tuesday, June 30, 2015 2:40 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs
> > 
> > This patch introduces a shrinker targeting to reduce memory footprint consumed
> > by a number of in-memory f2fs data structures.
> > 
> > In addition, it newly adds:
> >  - sbi->umount_mutex to avoid data races on shrinker and put_super
> >  - sbi->shruinker_run_no to not revisit objects
> > 
> > Noteh that the basic implementation was copied from fs/btrfs/shrinker.c
> 
> Great! Good to see it's being implemented in f2fs.
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> Reviewed-by: Chao Yu <chao2.yu@samsung.com>
> 
> [snip]
> 
> > @@ -1406,6 +1425,9 @@ static int __init init_f2fs_fs(void)
> >  	err = f2fs_init_crypto();
> >  	if (err)
> >  		goto free_kset;
> > +
> > +	register_shrinker(&f2fs_shrinker_info);
> 
> This function can fail due to no memory, please check the return value here.

Agreed, done.

Thanks,

> 
> Thanks,

RE: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs
> 
> This patch introduces a shrinker targeting to reduce memory footprint consumed
> by a number of in-memory f2fs data structures.
> 
> In addition, it newly adds:
>  - sbi->umount_mutex to avoid data races on shrinker and put_super
>  - sbi->shruinker_run_no to not revisit objects
> 
> Noteh that the basic implementation was copied from fs/btrfs/shrinker.c

This file seems not exist...

> @@ -1310,6 +1328,7 @@ free_root_inode:
>  	dput(sb->s_root);
>  	sb->s_root = NULL;
>  free_node_inode:
> +	f2fs_leave_shrinker(sbi);

We should detach shrinker under sbi->umount_mutex.
Otherwise we will access freed memory in following call path:

mount					shrinker
->fill_super
  Failed after f2fs_join_shrinker
  ->f2fs_leave_shrinker
					->f2fs_shrink_scan
					  spin_lock
					  get sbi pointer
					  spin_unlock
    spin_lock
    list_del sbi->s_list
    spin_unlock
    free sbi
					  use-after-free for sbi

Thanks,

Re: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs

[Jaegeuk Kim]

On Thu, Jul 02, 2015 at 08:32:39PM +0800, Chao Yu wrote:
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Tuesday, June 30, 2015 2:40 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 08/12] f2fs: introduce a shrinker for mounted fs
> > 
> > This patch introduces a shrinker targeting to reduce memory footprint consumed
> > by a number of in-memory f2fs data structures.
> > 
> > In addition, it newly adds:
> >  - sbi->umount_mutex to avoid data races on shrinker and put_super
> >  - sbi->shruinker_run_no to not revisit objects
> > 
> > Noteh that the basic implementation was copied from fs/btrfs/shrinker.c
> 
> This file seems not exist...
> 
> > @@ -1310,6 +1328,7 @@ free_root_inode:
> >  	dput(sb->s_root);
> >  	sb->s_root = NULL;
> >  free_node_inode:
> > +	f2fs_leave_shrinker(sbi);
> 
> We should detach shrinker under sbi->umount_mutex.
> Otherwise we will access freed memory in following call path:
> 
> mount					shrinker
> ->fill_super
>   Failed after f2fs_join_shrinker
>   ->f2fs_leave_shrinker
> 					->f2fs_shrink_scan
> 					  spin_lock
> 					  get sbi pointer
> 					  spin_unlock
>     spin_lock
>     list_del sbi->s_list
>     spin_unlock
>     free sbi
> 					  use-after-free for sbi

Right, confirmed this.

Thanks,

> 
> Thanks,
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 09/12] f2fs: shrink nat_cache entries

[Jaegeuk Kim]

This patch registers shrinking nat_cache entries.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/node.c     |  6 +++---
 fs/f2fs/segment.c  |  8 ++++++--
 fs/f2fs/shrinker.c | 11 +++++++++--
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index d9c5242..318ee86 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -328,11 +328,11 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink)
 {
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
+	int nr = nr_shrink;
 
-	if (available_free_memory(sbi, NAT_ENTRIES))
+	if (!down_write_trylock(&nm_i->nat_tree_lock))
 		return 0;
 
-	down_write(&nm_i->nat_tree_lock);
 	while (nr_shrink && !list_empty(&nm_i->nat_entries)) {
 		struct nat_entry *ne;
 		ne = list_first_entry(&nm_i->nat_entries,
@@ -341,7 +341,7 @@ int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink)
 		nr_shrink--;
 	}
 	up_write(&nm_i->nat_tree_lock);
-	return nr_shrink;
+	return nr - nr_shrink;
 }
 
 /*
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 1eb3437..dec3afb 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -305,8 +305,12 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
 	/* try to shrink extent cache when there is no enough memory */
 	f2fs_shrink_extent_tree(sbi, EXTENT_CACHE_SHRINK_NUMBER);
 
-	/* check the # of cached NAT entries and prefree segments */
-	if (try_to_free_nats(sbi, NAT_ENTRY_PER_BLOCK) ||
+	/* check the # of cached NAT entries */
+	if (!available_free_memory(sbi, NAT_ENTRIES))
+		try_to_free_nats(sbi, NAT_ENTRY_PER_BLOCK);
+
+	/* checkpoint is the only way to shrink partial cached entries */
+	if (!available_free_memory(sbi, NAT_ENTRIES) ||
 			excess_prefree_segs(sbi) ||
 			!available_free_memory(sbi, INO_ENTRIES))
 		f2fs_sync_fs(sbi->sb, true);
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
index a680145..3350ac3 100644
--- a/fs/f2fs/shrinker.c
+++ b/fs/f2fs/shrinker.c
@@ -18,6 +18,11 @@ static LIST_HEAD(f2fs_list);
 static DEFINE_SPINLOCK(f2fs_list_lock);
 static unsigned int shrinker_run_no;
 
+static unsigned long __count_nat_entries(struct f2fs_sb_info *sbi)
+{
+	return NM_I(sbi)->nat_cnt - NM_I(sbi)->dirty_nat_cnt;
+}
+
 unsigned long f2fs_shrink_count(struct shrinker *shrink,
 				struct shrink_control *sc)
 {
@@ -37,7 +42,8 @@ unsigned long f2fs_shrink_count(struct shrinker *shrink,
 		}
 		spin_unlock(&f2fs_list_lock);
 
-		/* TODO: count # of objects */
+		/* shrink clean nat cache entries */
+		count += __count_nat_entries(sbi);
 
 		spin_lock(&f2fs_list_lock);
 		p = p->next;
@@ -76,7 +82,8 @@ unsigned long f2fs_shrink_scan(struct shrinker *shrink,
 
 		sbi->shrinker_run_no = run_no;
 
-		/* TODO: shrink caches */
+		/* shrink clean nat cache entries */
+		freed += try_to_free_nats(sbi, nr);
 
 		spin_lock(&f2fs_list_lock);
 		p = p->next;
-- 
2.1.1

RE: [f2fs-dev] [PATCH 09/12] f2fs: shrink nat_cache entries

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 09/12] f2fs: shrink nat_cache entries
> 
> This patch registers shrinking nat_cache entries.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 10/12] f2fs: shrink extent_cache entries

[Jaegeuk Kim]

This patch registers shrinking extent_caches.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c     | 19 +++++++++++--------
 fs/f2fs/f2fs.h     |  2 +-
 fs/f2fs/segment.c  |  3 ++-
 fs/f2fs/shrinker.c | 14 +++++++++++++-
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 676e7b9..18bd0ac 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -767,7 +767,7 @@ out:
 		update_inode_page(inode);
 }
 
-void f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
+unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 {
 	struct extent_tree *treevec[EXT_TREE_VEC_SIZE];
 	struct extent_node *en, *tmp;
@@ -778,10 +778,7 @@ void f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 	unsigned int node_cnt = 0, tree_cnt = 0;
 
 	if (!test_opt(sbi, EXTENT_CACHE))
-		return;
-
-	if (available_free_memory(sbi, EXTENT_CACHE))
-		return;
+		return 0;
 
 	spin_lock(&sbi->extent_lock);
 	list_for_each_entry_safe(en, tmp, &sbi->extent_list, list) {
@@ -791,7 +788,9 @@ void f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 	}
 	spin_unlock(&sbi->extent_lock);
 
-	down_read(&sbi->extent_tree_lock);
+	if (!down_read_trylock(&sbi->extent_tree_lock))
+		goto out;
+
 	while ((found = radix_tree_gang_lookup(&sbi->extent_tree_root,
 				(void **)treevec, ino, EXT_TREE_VEC_SIZE))) {
 		unsigned i;
@@ -809,7 +808,9 @@ void f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 	}
 	up_read(&sbi->extent_tree_lock);
 
-	down_write(&sbi->extent_tree_lock);
+	if (!down_write_trylock(&sbi->extent_tree_lock))
+		goto out;
+
 	radix_tree_for_each_slot(slot, &sbi->extent_tree_root, &iter,
 							F2FS_ROOT_INO(sbi)) {
 		struct extent_tree *et = (struct extent_tree *)*slot;
@@ -822,8 +823,10 @@ void f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 		}
 	}
 	up_write(&sbi->extent_tree_lock);
-
+out:
 	trace_f2fs_shrink_extent_tree(sbi, node_cnt, tree_cnt);
+
+	return node_cnt + tree_cnt;
 }
 
 void f2fs_destroy_extent_tree(struct inode *inode)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e82af8c..eeef3eb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1754,7 +1754,7 @@ void f2fs_submit_page_mbio(struct f2fs_io_info *);
 void set_data_blkaddr(struct dnode_of_data *);
 int reserve_new_block(struct dnode_of_data *);
 int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
-void f2fs_shrink_extent_tree(struct f2fs_sb_info *, int);
+unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *, int);
 void f2fs_destroy_extent_tree(struct inode *);
 void f2fs_init_extent_cache(struct inode *, struct f2fs_extent *);
 void f2fs_update_extent_cache(struct dnode_of_data *);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index dec3afb..08b2ebc 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -303,7 +303,8 @@ void f2fs_balance_fs(struct f2fs_sb_info *sbi)
 void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
 {
 	/* try to shrink extent cache when there is no enough memory */
-	f2fs_shrink_extent_tree(sbi, EXTENT_CACHE_SHRINK_NUMBER);
+	if (!available_free_memory(sbi, EXTENT_CACHE))
+		f2fs_shrink_extent_tree(sbi, EXTENT_CACHE_SHRINK_NUMBER);
 
 	/* check the # of cached NAT entries */
 	if (!available_free_memory(sbi, NAT_ENTRIES))
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
index 3350ac3..a7d7a7c 100644
--- a/fs/f2fs/shrinker.c
+++ b/fs/f2fs/shrinker.c
@@ -23,6 +23,11 @@ static unsigned long __count_nat_entries(struct f2fs_sb_info *sbi)
 	return NM_I(sbi)->nat_cnt - NM_I(sbi)->dirty_nat_cnt;
 }
 
+static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi)
+{
+	return sbi->total_ext_tree + atomic_read(&sbi->total_ext_node);
+}
+
 unsigned long f2fs_shrink_count(struct shrinker *shrink,
 				struct shrink_control *sc)
 {
@@ -42,6 +47,9 @@ unsigned long f2fs_shrink_count(struct shrinker *shrink,
 		}
 		spin_unlock(&f2fs_list_lock);
 
+		/* count extent cache entries */
+		count += __count_extent_cache(sbi);
+
 		/* shrink clean nat cache entries */
 		count += __count_nat_entries(sbi);
 
@@ -82,8 +90,12 @@ unsigned long f2fs_shrink_scan(struct shrinker *shrink,
 
 		sbi->shrinker_run_no = run_no;
 
+		/* shrink extent cache entries */
+		freed += f2fs_shrink_extent_tree(sbi, nr >> 1);
+
 		/* shrink clean nat cache entries */
-		freed += try_to_free_nats(sbi, nr);
+		if (freed < nr)
+			freed += try_to_free_nats(sbi, nr - freed);
 
 		spin_lock(&f2fs_list_lock);
 		p = p->next;
-- 
2.1.1

RE: [f2fs-dev] [PATCH 10/12] f2fs: shrink extent_cache entries

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 10/12] f2fs: shrink extent_cache entries
> 
> This patch registers shrinking extent_caches.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 11/12] f2fs: add noextent_cache mount option

[Jaegeuk Kim]

This patch adds noextent_cache mount option.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2e8645e..a9db896 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -65,6 +65,7 @@ enum {
 	Opt_nobarrier,
 	Opt_fastboot,
 	Opt_extent_cache,
+	Opt_noextent_cache,
 	Opt_noinline_data,
 	Opt_err,
 };
@@ -88,6 +89,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_nobarrier, "nobarrier"},
 	{Opt_fastboot, "fastboot"},
 	{Opt_extent_cache, "extent_cache"},
+	{Opt_noextent_cache, "noextent_cache"},
 	{Opt_noinline_data, "noinline_data"},
 	{Opt_err, NULL},
 };
@@ -389,6 +391,9 @@ static int parse_options(struct super_block *sb, char *options)
 		case Opt_extent_cache:
 			set_opt(sbi, EXTENT_CACHE);
 			break;
+		case Opt_noextent_cache:
+			clear_opt(sbi, EXTENT_CACHE);
+			break;
 		case Opt_noinline_data:
 			clear_opt(sbi, INLINE_DATA);
 			break;
-- 
2.1.1

RE: [f2fs-dev] [PATCH 11/12] f2fs: add noextent_cache mount option

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 11/12] f2fs: add noextent_cache mount option
> 
> This patch adds noextent_cache mount option.

How about adding some descriptions in Documentation to help our user?

The other thing is we'd better to show 'noextent_cache' option when user
lookups the mount options.

How about adding codes as below?
if (test_opt(sbi, EXTENT_CACHE))
	seq_puts(seq, ",extent_cache");
+else
+	seq_puts(seq, ",noextent_cache");

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

The other looks good to me, so please add:

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

Re: [f2fs-dev] [PATCH 11/12] f2fs: add noextent_cache mount option

[Jaegeuk Kim]

On Thu, Jul 02, 2015 at 08:35:34PM +0800, Chao Yu wrote:
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Tuesday, June 30, 2015 2:40 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 11/12] f2fs: add noextent_cache mount option
> > 
> > This patch adds noextent_cache mount option.
> 
> How about adding some descriptions in Documentation to help our user?
> 
> The other thing is we'd better to show 'noextent_cache' option when user
> lookups the mount options.
> 
> How about adding codes as below?
> if (test_opt(sbi, EXTENT_CACHE))
> 	seq_puts(seq, ",extent_cache");
> +else
> +	seq_puts(seq, ",noextent_cache");
> 

Agreed.

Thanks,

> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> The other looks good to me, so please add:
> 
> Reviewed-by: Chao Yu <chao2.yu@samsung.com>
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 12/12] f2fs: use extent_cache by default

[Jaegeuk Kim]

We don't need to handle the duplicate extent infot showrmation.

The integrated rule is:
 - update on-disk extent with largest one tracked by in-memory extent_cache
 - destroy extent_tree for the truncation case
 - drop per-inode extent_cache by shrinker

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c     | 349 ++++++++++++++++-------------------------------------
 fs/f2fs/f2fs.h     |  20 ++-
 fs/f2fs/inode.c    |  18 ++-
 fs/f2fs/namei.c    |   2 +
 fs/f2fs/shrinker.c |   2 +
 fs/f2fs/super.c    |   8 +-
 6 files changed, 136 insertions(+), 263 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 18bd0ac..e90522a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -266,103 +266,6 @@ int f2fs_reserve_block(struct dnode_of_data *dn, pgoff_t index)
 	return err;
 }
 
-static bool lookup_extent_info(struct inode *inode, pgoff_t pgofs,
-							struct extent_info *ei)
-{
-	struct f2fs_inode_info *fi = F2FS_I(inode);
-	pgoff_t start_fofs, end_fofs;
-	block_t start_blkaddr;
-
-	read_lock(&fi->ext_lock);
-	if (fi->ext.len == 0) {
-		read_unlock(&fi->ext_lock);
-		return false;
-	}
-
-	stat_inc_total_hit(inode->i_sb);
-
-	start_fofs = fi->ext.fofs;
-	end_fofs = fi->ext.fofs + fi->ext.len - 1;
-	start_blkaddr = fi->ext.blk;
-
-	if (pgofs >= start_fofs && pgofs <= end_fofs) {
-		*ei = fi->ext;
-		stat_inc_read_hit(inode->i_sb);
-		read_unlock(&fi->ext_lock);
-		return true;
-	}
-	read_unlock(&fi->ext_lock);
-	return false;
-}
-
-static bool update_extent_info(struct inode *inode, pgoff_t fofs,
-								block_t blkaddr)
-{
-	struct f2fs_inode_info *fi = F2FS_I(inode);
-	pgoff_t start_fofs, end_fofs;
-	block_t start_blkaddr, end_blkaddr;
-	int need_update = true;
-
-	write_lock(&fi->ext_lock);
-
-	start_fofs = fi->ext.fofs;
-	end_fofs = fi->ext.fofs + fi->ext.len - 1;
-	start_blkaddr = fi->ext.blk;
-	end_blkaddr = fi->ext.blk + fi->ext.len - 1;
-
-	/* Drop and initialize the matched extent */
-	if (fi->ext.len == 1 && fofs == start_fofs)
-		fi->ext.len = 0;
-
-	/* Initial extent */
-	if (fi->ext.len == 0) {
-		if (blkaddr != NULL_ADDR) {
-			fi->ext.fofs = fofs;
-			fi->ext.blk = blkaddr;
-			fi->ext.len = 1;
-		}
-		goto end_update;
-	}
-
-	/* Front merge */
-	if (fofs == start_fofs - 1 && blkaddr == start_blkaddr - 1) {
-		fi->ext.fofs--;
-		fi->ext.blk--;
-		fi->ext.len++;
-		goto end_update;
-	}
-
-	/* Back merge */
-	if (fofs == end_fofs + 1 && blkaddr == end_blkaddr + 1) {
-		fi->ext.len++;
-		goto end_update;
-	}
-
-	/* Split the existing extent */
-	if (fi->ext.len > 1 &&
-		fofs >= start_fofs && fofs <= end_fofs) {
-		if ((end_fofs - fofs) < (fi->ext.len >> 1)) {
-			fi->ext.len = fofs - start_fofs;
-		} else {
-			fi->ext.fofs = fofs + 1;
-			fi->ext.blk = start_blkaddr + fofs - start_fofs + 1;
-			fi->ext.len -= fofs - start_fofs + 1;
-		}
-	} else {
-		need_update = false;
-	}
-
-	/* Finally, if the extent is very fragmented, let's drop the cache. */
-	if (fi->ext.len < F2FS_MIN_EXTENT_LEN) {
-		fi->ext.len = 0;
-		set_inode_flag(fi, FI_NO_EXTENT);
-		need_update = true;
-	}
-end_update:
-	write_unlock(&fi->ext_lock);
-	return need_update;
-}
-
 static struct extent_node *__attach_extent_node(struct f2fs_sb_info *sbi,
 				struct extent_tree *et, struct extent_info *ei,
 				struct rb_node *parent, struct rb_node **p)
@@ -394,23 +297,6 @@ static void __detach_extent_node(struct f2fs_sb_info *sbi,
 		et->cached_en = NULL;
 }
 
-static struct extent_tree *__find_extent_tree(struct f2fs_sb_info *sbi,
-							nid_t ino)
-{
-	struct extent_tree *et;
-
-	down_read(&sbi->extent_tree_lock);
-	et = radix_tree_lookup(&sbi->extent_tree_root, ino);
-	if (!et) {
-		up_read(&sbi->extent_tree_lock);
-		return NULL;
-	}
-	atomic_inc(&et->refcount);
-	up_read(&sbi->extent_tree_lock);
-
-	return et;
-}
-
 static struct extent_tree *__grab_extent_tree(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -434,6 +320,9 @@ static struct extent_tree *__grab_extent_tree(struct inode *inode)
 	atomic_inc(&et->refcount);
 	up_write(&sbi->extent_tree_lock);
 
+	/* never died untill evict_inode */
+	F2FS_I(inode)->extent_tree = et;
+
 	return et;
 }
 
@@ -522,7 +411,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 				en->ei.len += ei->len;
 				if (den)
 					*den = __try_back_merge(sbi, et, en);
-				return en;
+				goto update_out;
 			}
 			p = &(*p)->rb_left;
 		} else if (ei->fofs >= en->ei.fofs + en->ei.len) {
@@ -530,7 +419,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 				en->ei.len += ei->len;
 				if (den)
 					*den = __try_front_merge(sbi, et, en);
-				return en;
+				goto update_out;
 			}
 			p = &(*p)->rb_right;
 		} else {
@@ -538,7 +427,11 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 		}
 	}
 
-	return __attach_extent_node(sbi, et, ei, parent, p);
+	en = __attach_extent_node(sbi, et, ei, parent, p);
+update_out:
+	if (en && en->ei.len > et->largest.len)
+		et->largest = en->ei;
+	return en;
 }
 
 static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
@@ -570,26 +463,36 @@ static unsigned int __free_extent_tree(struct f2fs_sb_info *sbi,
 	return count - et->count;
 }
 
-static void f2fs_init_extent_tree(struct inode *inode,
-						struct f2fs_extent *i_ext)
+static void __drop_largest_extent(struct inode *inode, pgoff_t fofs)
+{
+	struct extent_info *largest = &F2FS_I(inode)->extent_tree->largest;
+
+	if (largest->fofs <= fofs && largest->fofs + largest->len > fofs)
+		largest->len = 0;
+}
+
+void f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct extent_tree *et;
 	struct extent_node *en;
 	struct extent_info ei;
 
-	if (le32_to_cpu(i_ext->len) < F2FS_MIN_EXTENT_LEN)
+	if (!f2fs_may_extent_tree(inode))
 		return;
 
 	et = __grab_extent_tree(inode);
 
-	write_lock(&et->lock);
-	if (et->count)
-		goto out;
+	if (!i_ext || le32_to_cpu(i_ext->len) < F2FS_MIN_EXTENT_LEN)
+		return;
 
 	set_extent_info(&ei, le32_to_cpu(i_ext->fofs),
 		le32_to_cpu(i_ext->blk), le32_to_cpu(i_ext->len));
 
+	write_lock(&et->lock);
+	if (et->count)
+		goto out;
+
 	en = __insert_extent_tree(sbi, et, &ei, NULL);
 	if (en) {
 		et->cached_en = en;
@@ -600,21 +503,18 @@ static void f2fs_init_extent_tree(struct inode *inode,
 	}
 out:
 	write_unlock(&et->lock);
-	atomic_dec(&et->refcount);
 }
 
 static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
 							struct extent_info *ei)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	struct extent_tree *et;
+	struct extent_tree *et = F2FS_I(inode)->extent_tree;
 	struct extent_node *en;
 
-	trace_f2fs_lookup_extent_tree_start(inode, pgofs);
+	f2fs_bug_on(sbi, !et);
 
-	et = __find_extent_tree(sbi, inode->i_ino);
-	if (!et)
-		return false;
+	trace_f2fs_lookup_extent_tree_start(inode, pgofs);
 
 	read_lock(&et->lock);
 	en = __lookup_extent_tree(et, pgofs);
@@ -631,27 +531,35 @@ static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
 	read_unlock(&et->lock);
 
 	trace_f2fs_lookup_extent_tree_end(inode, pgofs, en);
-
-	atomic_dec(&et->refcount);
 	return en ? true : false;
 }
 
-static void f2fs_update_extent_tree(struct inode *inode, pgoff_t fofs,
+/* return true, if on-disk extent should be updated */
+static bool f2fs_update_extent_tree(struct inode *inode, pgoff_t fofs,
 							block_t blkaddr)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	struct extent_tree *et;
+	struct extent_tree *et = F2FS_I(inode)->extent_tree;
 	struct extent_node *en = NULL, *en1 = NULL, *en2 = NULL, *en3 = NULL;
 	struct extent_node *den = NULL;
-	struct extent_info ei, dei;
+	struct extent_info ei, dei, prev;
 	unsigned int endofs;
 
-	trace_f2fs_update_extent_tree(inode, fofs, blkaddr);
+	if (!et)
+		return false;
 
-	et = __grab_extent_tree(inode);
+	trace_f2fs_update_extent_tree(inode, fofs, blkaddr);
 
 	write_lock(&et->lock);
 
+	if (is_inode_flag_set(F2FS_I(inode), FI_NO_EXTENT)) {
+		write_unlock(&et->lock);
+		return false;
+	}
+
+	prev = et->largest;
+	dei.len = 0;
+
 	/* 1. lookup and remove existing extent info in cache */
 	en = __lookup_extent_tree(et, fofs);
 	if (!en)
@@ -660,6 +568,8 @@ static void f2fs_update_extent_tree(struct inode *inode, pgoff_t fofs,
 	dei = en->ei;
 	__detach_extent_node(sbi, et, en);
 
+	__drop_largest_extent(inode, fofs);
+
 	/* 2. if extent can be split more, split and insert the left part */
 	if (dei.len > 1) {
 		/*  insert left part of split extent into cache */
@@ -683,6 +593,14 @@ update_extent:
 	if (blkaddr) {
 		set_extent_info(&ei, fofs, blkaddr, 1);
 		en3 = __insert_extent_tree(sbi, et, &ei, &den);
+
+		/* 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) {
+			et->largest.len = 0;
+			set_inode_flag(F2FS_I(inode), FI_NO_EXTENT);
+		}
 	}
 
 	/* 4. update in global extent list */
@@ -714,57 +632,12 @@ update_extent:
 	if (den)
 		kmem_cache_free(extent_node_slab, den);
 
-	write_unlock(&et->lock);
-	atomic_dec(&et->refcount);
-}
-
-void f2fs_preserve_extent_tree(struct inode *inode)
-{
-	struct extent_tree *et;
-	struct extent_info *ext = &F2FS_I(inode)->ext;
-	bool sync = false;
-
-	if (!test_opt(F2FS_I_SB(inode), EXTENT_CACHE))
-		return;
-
-	et = __find_extent_tree(F2FS_I_SB(inode), inode->i_ino);
-	if (!et) {
-		if (ext->len) {
-			ext->len = 0;
-			update_inode_page(inode);
-		}
-		return;
-	}
-
-	read_lock(&et->lock);
-	if (et->count) {
-		struct extent_node *en;
-
-		if (et->cached_en) {
-			en = et->cached_en;
-		} else {
-			struct rb_node *node = rb_first(&et->root);
-
-			if (!node)
-				node = rb_last(&et->root);
-			en = rb_entry(node, struct extent_node, rb_node);
-		}
-
-		if (__is_extent_same(ext, &en->ei))
-			goto out;
+	if (is_inode_flag_set(F2FS_I(inode), FI_NO_EXTENT))
+		__free_extent_tree(sbi, et, true);
 
-		*ext = en->ei;
-		sync = true;
-	} else if (ext->len) {
-		ext->len = 0;
-		sync = true;
-	}
-out:
-	read_unlock(&et->lock);
-	atomic_dec(&et->refcount);
+	write_unlock(&et->lock);
 
-	if (sync)
-		update_inode_page(inode);
+	return !__is_extent_same(&prev, &et->largest);
 }
 
 unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
@@ -772,8 +645,7 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 	struct extent_tree *treevec[EXT_TREE_VEC_SIZE];
 	struct extent_node *en, *tmp;
 	unsigned long ino = F2FS_ROOT_INO(sbi);
-	struct radix_tree_iter iter;
-	void **slot;
+	struct radix_tree_root *root = &sbi->extent_tree_root;
 	unsigned int found;
 	unsigned int node_cnt = 0, tree_cnt = 0;
 
@@ -788,10 +660,10 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 	}
 	spin_unlock(&sbi->extent_lock);
 
-	if (!down_read_trylock(&sbi->extent_tree_lock))
+	if (!down_write_trylock(&sbi->extent_tree_lock))
 		goto out;
 
-	while ((found = radix_tree_gang_lookup(&sbi->extent_tree_root,
+	while ((found = radix_tree_gang_lookup(root,
 				(void **)treevec, ino, EXT_TREE_VEC_SIZE))) {
 		unsigned i;
 
@@ -799,27 +671,15 @@ unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink)
 		for (i = 0; i < found; i++) {
 			struct extent_tree *et = treevec[i];
 
-			atomic_inc(&et->refcount);
 			write_lock(&et->lock);
 			node_cnt += __free_extent_tree(sbi, et, false);
 			write_unlock(&et->lock);
-			atomic_dec(&et->refcount);
-		}
-	}
-	up_read(&sbi->extent_tree_lock);
-
-	if (!down_write_trylock(&sbi->extent_tree_lock))
-		goto out;
-
-	radix_tree_for_each_slot(slot, &sbi->extent_tree_root, &iter,
-							F2FS_ROOT_INO(sbi)) {
-		struct extent_tree *et = (struct extent_tree *)*slot;
-
-		if (!atomic_read(&et->refcount) && !et->count) {
-			radix_tree_delete(&sbi->extent_tree_root, et->ino);
-			kmem_cache_free(extent_tree_slab, et);
-			sbi->total_ext_tree--;
-			tree_cnt++;
+			if (!atomic_read(&et->refcount) && !et->count) {
+				radix_tree_delete(root, et->ino);
+				kmem_cache_free(extent_tree_slab, et);
+				sbi->total_ext_tree--;
+				tree_cnt++;
+			}
 		}
 	}
 	up_write(&sbi->extent_tree_lock);
@@ -829,63 +689,59 @@ out:
 	return node_cnt + tree_cnt;
 }
 
-void f2fs_destroy_extent_tree(struct inode *inode)
+void f2fs_destroy_extent_node(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	struct extent_tree *et;
+	struct extent_tree *et = F2FS_I(inode)->extent_tree;
 	unsigned int node_cnt = 0;
 
-	if (!test_opt(sbi, EXTENT_CACHE))
-		return;
-
-	et = __find_extent_tree(sbi, inode->i_ino);
 	if (!et)
-		goto out;
+		return;
 
-	/* free all extent info belong to this extent tree */
 	write_lock(&et->lock);
 	node_cnt = __free_extent_tree(sbi, et, true);
 	write_unlock(&et->lock);
+}
 
-	atomic_dec(&et->refcount);
+void f2fs_destroy_extent_tree(struct inode *inode)
+{
+	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+	struct extent_tree *et = F2FS_I(inode)->extent_tree;
+	unsigned int node_cnt = 0;
 
-	/* try to find and delete extent tree entry in radix tree */
-	down_write(&sbi->extent_tree_lock);
-	et = radix_tree_lookup(&sbi->extent_tree_root, inode->i_ino);
-	if (!et) {
-		up_write(&sbi->extent_tree_lock);
-		goto out;
+	if (!et)
+		return;
+
+	if (inode->i_nlink && !is_bad_inode(inode) && et->count) {
+		atomic_dec(&et->refcount);
+		return;
 	}
+
+	/* free all extent info belong to this extent tree */
+	f2fs_destroy_extent_node(inode);
+
+	/* delete extent tree entry in radix tree */
+	down_write(&sbi->extent_tree_lock);
+	atomic_dec(&et->refcount);
 	f2fs_bug_on(sbi, atomic_read(&et->refcount) || et->count);
 	radix_tree_delete(&sbi->extent_tree_root, inode->i_ino);
 	kmem_cache_free(extent_tree_slab, et);
 	sbi->total_ext_tree--;
 	up_write(&sbi->extent_tree_lock);
-out:
-	trace_f2fs_destroy_extent_tree(inode, node_cnt);
-	return;
-}
 
-void f2fs_init_extent_cache(struct inode *inode, struct f2fs_extent *i_ext)
-{
-	if (test_opt(F2FS_I_SB(inode), EXTENT_CACHE))
-		f2fs_init_extent_tree(inode, i_ext);
+	F2FS_I(inode)->extent_tree = NULL;
 
-	write_lock(&F2FS_I(inode)->ext_lock);
-	get_extent_info(&F2FS_I(inode)->ext, *i_ext);
-	write_unlock(&F2FS_I(inode)->ext_lock);
+	trace_f2fs_destroy_extent_tree(inode, node_cnt);
+	return;
 }
 
 static bool f2fs_lookup_extent_cache(struct inode *inode, pgoff_t pgofs,
 							struct extent_info *ei)
 {
-	if (is_inode_flag_set(F2FS_I(inode), FI_NO_EXTENT))
+	if (!f2fs_may_extent_tree(inode))
 		return false;
 
-	if (test_opt(F2FS_I_SB(inode), EXTENT_CACHE))
-		return f2fs_lookup_extent_tree(inode, pgofs, ei);
-
-	return lookup_extent_info(inode, pgofs, ei);
+	return f2fs_lookup_extent_tree(inode, pgofs, ei);
 }
 
 void f2fs_update_extent_cache(struct dnode_of_data *dn)
@@ -893,19 +749,15 @@ void f2fs_update_extent_cache(struct dnode_of_data *dn)
 	struct f2fs_inode_info *fi = F2FS_I(dn->inode);
 	pgoff_t fofs;
 
-	f2fs_bug_on(F2FS_I_SB(dn->inode), dn->data_blkaddr == NEW_ADDR);
-
-	if (is_inode_flag_set(fi, FI_NO_EXTENT))
+	if (!f2fs_may_extent_tree(dn->inode))
 		return;
 
+	f2fs_bug_on(F2FS_I_SB(dn->inode), dn->data_blkaddr == NEW_ADDR);
+
 	fofs = start_bidx_of_node(ofs_of_node(dn->node_page), fi) +
 							dn->ofs_in_node;
 
-	/* we should call update_extent_info() to update on-disk extent */
-	if (test_opt(F2FS_I_SB(dn->inode), EXTENT_CACHE))
-		f2fs_update_extent_tree(dn->inode, fofs, dn->data_blkaddr);
-
-	if (update_extent_info(dn->inode, fofs, dn->data_blkaddr))
+	if (f2fs_update_extent_tree(dn->inode, fofs, dn->data_blkaddr))
 		sync_inode_page(dn);
 }
 
@@ -1109,8 +961,6 @@ alloc:
 
 	allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr,
 								&sum, seg);
-
-	/* direct IO doesn't use extent cache to maximize the performance */
 	set_data_blkaddr(dn);
 
 	/* update i_size */
@@ -1119,6 +969,9 @@ alloc:
 	if (i_size_read(dn->inode) < ((fofs + 1) << PAGE_CACHE_SHIFT))
 		i_size_write(dn->inode, ((fofs + 1) << PAGE_CACHE_SHIFT));
 
+	/* direct IO doesn't use extent cache to maximize the performance */
+	__drop_largest_extent(dn->inode, fofs);
+
 	return 0;
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eeef3eb..281343c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -349,6 +349,7 @@ struct extent_tree {
 	nid_t ino;			/* inode number */
 	struct rb_root root;		/* root of extent info rb-tree */
 	struct extent_node *cached_en;	/* recently accessed extent node */
+	struct extent_info largest;	/* largested extent info */
 	rwlock_t lock;			/* protect extent info rb-tree */
 	atomic_t refcount;		/* reference count of rb-tree */
 	unsigned int count;		/* # of extent node in rb-tree*/
@@ -420,14 +421,14 @@ struct f2fs_inode_info {
 	unsigned int clevel;		/* maximum level of given file name */
 	nid_t i_xattr_nid;		/* node id that contains xattrs */
 	unsigned long long xattr_ver;	/* cp version of xattr modification */
-	struct extent_info ext;		/* in-memory extent cache entry */
-	rwlock_t ext_lock;		/* rwlock for single extent cache */
 	struct inode_entry *dirty_dir;	/* the pointer of dirty dir */
 
 	struct radix_tree_root inmem_root;	/* radix tree for inmem pages */
 	struct list_head inmem_pages;	/* inmemory pages managed by f2fs */
 	struct mutex inmem_lock;	/* lock for inmemory pages */
 
+	struct extent_tree *extent_tree;	/* cached extent_tree entry */
+
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 	/* Encryption params */
 	struct f2fs_crypt_info *i_crypt_info;
@@ -1548,6 +1549,17 @@ static inline bool is_dot_dotdot(const struct qstr *str)
 	return false;
 }
 
+static inline bool f2fs_may_extent_tree(struct inode *inode)
+{
+	mode_t mode = inode->i_mode;
+
+	if (!test_opt(F2FS_I_SB(inode), EXTENT_CACHE) ||
+			is_inode_flag_set(F2FS_I(inode), FI_NO_EXTENT))
+		return false;
+
+	return S_ISREG(mode);
+}
+
 #define get_inode_mode(i) \
 	((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
 	 (F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
@@ -1755,10 +1767,10 @@ void set_data_blkaddr(struct dnode_of_data *);
 int reserve_new_block(struct dnode_of_data *);
 int f2fs_reserve_block(struct dnode_of_data *, pgoff_t);
 unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *, int);
+void f2fs_init_extent_tree(struct inode *, struct f2fs_extent *);
+void f2fs_destroy_extent_node(struct inode *);
 void f2fs_destroy_extent_tree(struct inode *);
-void f2fs_init_extent_cache(struct inode *, struct f2fs_extent *);
 void f2fs_update_extent_cache(struct dnode_of_data *);
-void f2fs_preserve_extent_tree(struct inode *);
 struct page *get_read_data_page(struct inode *, pgoff_t, int);
 struct page *find_data_page(struct inode *, pgoff_t);
 struct page *get_lock_data_page(struct inode *, pgoff_t);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 757fed2..978a726 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -139,7 +139,7 @@ static int do_read_inode(struct inode *inode)
 	fi->i_pino = le32_to_cpu(ri->i_pino);
 	fi->i_dir_level = ri->i_dir_level;
 
-	f2fs_init_extent_cache(inode, &ri->i_ext);
+	f2fs_init_extent_tree(inode, &ri->i_ext);
 
 	get_inline_info(fi, ri);
 
@@ -237,10 +237,11 @@ void update_inode(struct inode *inode, struct page *node_page)
 	ri->i_size = cpu_to_le64(i_size_read(inode));
 	ri->i_blocks = cpu_to_le64(inode->i_blocks);
 
-	read_lock(&F2FS_I(inode)->ext_lock);
-	set_raw_extent(&F2FS_I(inode)->ext, &ri->i_ext);
-	read_unlock(&F2FS_I(inode)->ext_lock);
-
+	if (F2FS_I(inode)->extent_tree)
+		set_raw_extent(&F2FS_I(inode)->extent_tree->largest,
+							&ri->i_ext);
+	else
+		memset(&ri->i_ext, 0, sizeof(ri->i_ext));
 	set_raw_inline(F2FS_I(inode), ri);
 
 	ri->i_atime = cpu_to_le64(inode->i_atime.tv_sec);
@@ -331,6 +332,8 @@ void f2fs_evict_inode(struct inode *inode)
 	f2fs_bug_on(sbi, get_dirty_pages(inode));
 	remove_dirty_dir_inode(inode);
 
+	f2fs_destroy_extent_tree(inode);
+
 	if (inode->i_nlink || is_bad_inode(inode))
 		goto no_delete;
 
@@ -350,11 +353,6 @@ no_delete:
 	stat_dec_inline_dir(inode);
 	stat_dec_inline_inode(inode);
 
-	/* update extent info in inode */
-	if (inode->i_nlink)
-		f2fs_preserve_extent_tree(inode);
-	f2fs_destroy_extent_tree(inode);
-
 	invalidate_mapping_pages(NODE_MAPPING(sbi), inode->i_ino, inode->i_ino);
 	if (xnid)
 		invalidate_mapping_pages(NODE_MAPPING(sbi), xnid, xnid);
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 08656fc..df315dc 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -65,6 +65,8 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
 	if (f2fs_may_inline_dentry(inode))
 		set_inode_flag(F2FS_I(inode), FI_INLINE_DENTRY);
 
+	f2fs_init_extent_tree(inode, NULL);
+
 	stat_inc_inline_inode(inode);
 	stat_inc_inline_dir(inode);
 
diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
index a7d7a7c..2dfb08d 100644
--- a/fs/f2fs/shrinker.c
+++ b/fs/f2fs/shrinker.c
@@ -117,6 +117,8 @@ void f2fs_join_shrinker(struct f2fs_sb_info *sbi)
 
 void f2fs_leave_shrinker(struct f2fs_sb_info *sbi)
 {
+	f2fs_shrink_extent_tree(sbi, __count_extent_cache(sbi));
+
 	spin_lock(&f2fs_list_lock);
 	list_del(&sbi->s_list);
 	spin_unlock(&f2fs_list_lock);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a9db896..31ac3c7 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -422,7 +422,6 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb)
 	atomic_set(&fi->dirty_pages, 0);
 	fi->i_current_depth = 1;
 	fi->i_advise = 0;
-	rwlock_init(&fi->ext_lock);
 	init_rwsem(&fi->i_sem);
 	INIT_RADIX_TREE(&fi->inmem_root, GFP_NOFS);
 	INIT_LIST_HEAD(&fi->inmem_pages);
@@ -453,12 +452,17 @@ static int f2fs_drop_inode(struct inode *inode)
 	 */
 	if (!inode_unhashed(inode) && inode->i_state & I_SYNC) {
 		if (!inode->i_nlink && !is_bad_inode(inode)) {
+			/* to avoid evict_inode call simultaneously */
+			atomic_inc(&inode->i_count);
 			spin_unlock(&inode->i_lock);
 
 			/* some remained atomic pages should discarded */
 			if (f2fs_is_atomic_file(inode))
 				commit_inmem_pages(inode, true);
 
+			/* should remain fi->extent_tree for writepage */
+			f2fs_destroy_extent_node(inode);
+
 			sb_start_intwrite(inode->i_sb);
 			i_size_write(inode, 0);
 
@@ -473,6 +477,7 @@ static int f2fs_drop_inode(struct inode *inode)
 					F2FS_I(inode)->i_crypt_info);
 #endif
 			spin_lock(&inode->i_lock);
+			atomic_dec(&inode->i_count);
 		}
 		return 0;
 	}
@@ -719,6 +724,7 @@ static void default_options(struct f2fs_sb_info *sbi)
 
 	set_opt(sbi, BG_GC);
 	set_opt(sbi, INLINE_DATA);
+	set_opt(sbi, EXTENT_CACHE);
 
 #ifdef CONFIG_F2FS_FS_XATTR
 	set_opt(sbi, XATTR_USER);
-- 
2.1.1

RE: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:40 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default
> 
> We don't need to handle the duplicate extent infot showrmation.

information?

> 
> The integrated rule is:
>  - update on-disk extent with largest one tracked by in-memory extent_cache
>  - destroy extent_tree for the truncation case
>  - drop per-inode extent_cache by shrinker
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

[snip]

> @@ -538,7 +427,11 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
>  		}
>  	}
> 
> -	return __attach_extent_node(sbi, et, ei, parent, p);
> +	en = __attach_extent_node(sbi, et, ei, parent, p);
> +update_out:
> +	if (en && en->ei.len > et->largest.len)
> +		et->largest = en->ei;

IMO, it's better to update cached_en here if it is invalid in
__detach_extent_node, then cached_en and largest may point different
extent info, it can expand our region of first level extent cache.

[snip]

> +
> +	/* free all extent info belong to this extent tree */
> +	f2fs_destroy_extent_node(inode);

How about returning number of freed extent node for tracing.

node_cnt = f2fs_destroy_extent_node(inode);

[snip]

> @@ -237,10 +237,11 @@ void update_inode(struct inode *inode, struct page *node_page)
>  	ri->i_size = cpu_to_le64(i_size_read(inode));
>  	ri->i_blocks = cpu_to_le64(inode->i_blocks);
> 
> -	read_lock(&F2FS_I(inode)->ext_lock);
> -	set_raw_extent(&F2FS_I(inode)->ext, &ri->i_ext);
> -	read_unlock(&F2FS_I(inode)->ext_lock);
> -
> +	if (F2FS_I(inode)->extent_tree)

Could extent cache destroy after above check?

Thanks,

Re: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

[Jaegeuk Kim]

Hi Chao,

On Thu, Jul 02, 2015 at 08:36:16PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Tuesday, June 30, 2015 2:40 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default
> > 
> > We don't need to handle the duplicate extent infot showrmation.
> 
> information?

Fixed.

> 
> > 
> > The integrated rule is:
> >  - update on-disk extent with largest one tracked by in-memory extent_cache
> >  - destroy extent_tree for the truncation case
> >  - drop per-inode extent_cache by shrinker
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> [snip]
> 
> > @@ -538,7 +427,11 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
> >  		}
> >  	}
> > 
> > -	return __attach_extent_node(sbi, et, ei, parent, p);
> > +	en = __attach_extent_node(sbi, et, ei, parent, p);
> > +update_out:
> > +	if (en && en->ei.len > et->largest.len)
> > +		et->largest = en->ei;
> 
> IMO, it's better to update cached_en here if it is invalid in
> __detach_extent_node, then cached_en and largest may point different
> extent info, it can expand our region of first level extent cache.

Agreed.

> 
> [snip]
> 
> > +
> > +	/* free all extent info belong to this extent tree */
> > +	f2fs_destroy_extent_node(inode);
> 
> How about returning number of freed extent node for tracing.
> 
> node_cnt = f2fs_destroy_extent_node(inode);

No problem.

> 
> [snip]
> 
> > @@ -237,10 +237,11 @@ void update_inode(struct inode *inode, struct page *node_page)
> >  	ri->i_size = cpu_to_le64(i_size_read(inode));
> >  	ri->i_blocks = cpu_to_le64(inode->i_blocks);
> > 
> > -	read_lock(&F2FS_I(inode)->ext_lock);
> > -	set_raw_extent(&F2FS_I(inode)->ext, &ri->i_ext);
> > -	read_unlock(&F2FS_I(inode)->ext_lock);
> > -
> > +	if (F2FS_I(inode)->extent_tree)
> 
> Could extent cache destroy after above check?

I don't think so.

The extent_tree is assigned as one way.
Once it is assigned, it will be deallocated only after evict_inode.

Thanks,

> 
> Thanks,
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

RE: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Saturday, July 04, 2015 1:16 PM
> To: Chao Yu
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

[snip]

> > > @@ -237,10 +237,11 @@ void update_inode(struct inode *inode, struct page *node_page)
> > >  	ri->i_size = cpu_to_le64(i_size_read(inode));
> > >  	ri->i_blocks = cpu_to_le64(inode->i_blocks);
> > >
> > > -	read_lock(&F2FS_I(inode)->ext_lock);
> > > -	set_raw_extent(&F2FS_I(inode)->ext, &ri->i_ext);
> > > -	read_unlock(&F2FS_I(inode)->ext_lock);
> > > -
> > > +	if (F2FS_I(inode)->extent_tree)
> >
> > Could extent cache destroy after above check?
> 
> I don't think so.
> 
> The extent_tree is assigned as one way.
> Once it is assigned, it will be deallocated only after evict_inode.

Previously, I suspected that ->write_inode and ->evict will be executed
concurrently.

After checking the code, I find that would not happen, so we are safe.

Thanks,

> 
> Thanks,
> 
> >
> > Thanks,

RE: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

[Chao Yu]

> -----Original Message-----
> From: Chao Yu [mailto:yuchaochina@hotmail.com]
> Sent: Saturday, July 04, 2015 2:30 PM
> To: 'Jaegeuk Kim'
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default
> 
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Saturday, July 04, 2015 1:16 PM
> > To: Chao Yu
> > Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH 12/12] f2fs: use extent_cache by default

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

RE: [f2fs-dev] [PATCH 01/12] f2fs: avoid freed stat information

[Chao Yu]

 -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, June 30, 2015 2:39 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 01/12] f2fs: avoid freed stat information
> 
> The write_checkpoint can update stat information, so we should destroy the stat
> structure after it.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>