[PATCH 1/4] f2fs: check inline_data flag at converting time

[PATCH 1/4] f2fs: check inline_data flag at converting time

[Jaegeuk Kim]

We can check inode's inline_data flag  when calling to convert it.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c   |  8 +++-----
 fs/f2fs/file.c   | 58 ++++++++++++++++++++++----------------------------------
 fs/f2fs/inline.c |  3 +++
 3 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index e34b1bd..cf0c9dd 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1573,11 +1573,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	int err;
 
 	/* we don't need to use inline_data strictly */
-	if (f2fs_has_inline_data(inode)) {
-		err = f2fs_convert_inline_inode(inode);
-		if (err)
-			return err;
-	}
+	err = f2fs_convert_inline_inode(inode);
+	if (err)
+		return err;
 
 	if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode))
 		return 0;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 7f8ca47..f2effe1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -418,19 +418,18 @@ static loff_t f2fs_llseek(struct file *file, loff_t offset, int whence)
 static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct inode *inode = file_inode(file);
+	int err;
 
 	if (f2fs_encrypted_inode(inode)) {
-		int err = f2fs_get_encryption_info(inode);
+		err = f2fs_get_encryption_info(inode);
 		if (err)
 			return 0;
 	}
 
 	/* we don't need to use inline_data strictly */
-	if (f2fs_has_inline_data(inode)) {
-		int err = f2fs_convert_inline_inode(inode);
-		if (err)
-			return err;
-	}
+	err = f2fs_convert_inline_inode(inode);
+	if (err)
+		return err;
 
 	file_accessed(file);
 	vma->vm_ops = &f2fs_file_vm_ops;
@@ -604,7 +603,7 @@ int f2fs_truncate(struct inode *inode, bool lock)
 	trace_f2fs_truncate(inode);
 
 	/* we should check inline_data size */
-	if (f2fs_has_inline_data(inode) && !f2fs_may_inline_data(inode)) {
+	if (!f2fs_may_inline_data(inode)) {
 		err = f2fs_convert_inline_inode(inode);
 		if (err)
 			return err;
@@ -688,8 +687,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 			truncate_setsize(inode, attr->ia_size);
 
 			/* should convert inline inode here */
-			if (f2fs_has_inline_data(inode) &&
-					!f2fs_may_inline_data(inode)) {
+			if (!f2fs_may_inline_data(inode)) {
 				err = f2fs_convert_inline_inode(inode);
 				if (err)
 					return err;
@@ -786,13 +784,11 @@ static int punch_hole(struct inode *inode, loff_t offset, loff_t len)
 {
 	pgoff_t pg_start, pg_end;
 	loff_t off_start, off_end;
-	int ret = 0;
+	int ret;
 
-	if (f2fs_has_inline_data(inode)) {
-		ret = f2fs_convert_inline_inode(inode);
-		if (ret)
-			return ret;
-	}
+	ret = f2fs_convert_inline_inode(inode);
+	if (ret)
+		return ret;
 
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
@@ -951,11 +947,9 @@ static int f2fs_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 
 	f2fs_balance_fs(F2FS_I_SB(inode));
 
-	if (f2fs_has_inline_data(inode)) {
-		ret = f2fs_convert_inline_inode(inode);
-		if (ret)
-			return ret;
-	}
+	ret = f2fs_convert_inline_inode(inode);
+	if (ret)
+		return ret;
 
 	pg_start = offset >> PAGE_CACHE_SHIFT;
 	pg_end = (offset + len) >> PAGE_CACHE_SHIFT;
@@ -1001,11 +995,9 @@ static int f2fs_zero_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 = f2fs_convert_inline_inode(inode);
+	if (ret)
+		return ret;
 
 	ret = filemap_write_and_wait_range(mapping, offset, offset + len - 1);
 	if (ret)
@@ -1114,11 +1106,9 @@ 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 = f2fs_convert_inline_inode(inode);
+	if (ret)
+		return ret;
 
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
@@ -1168,11 +1158,9 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
-	if (f2fs_has_inline_data(inode)) {
-		ret = f2fs_convert_inline_inode(inode);
-		if (ret)
-			return ret;
-	}
+	ret = f2fs_convert_inline_inode(inode);
+	if (ret)
+		return ret;
 
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index bda7126..8090854 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -177,6 +177,9 @@ int f2fs_convert_inline_inode(struct inode *inode)
 	struct page *ipage, *page;
 	int err = 0;
 
+	if (!f2fs_has_inline_data(inode))
+		return 0;
+
 	page = grab_cache_page(inode->i_mapping, 0);
 	if (!page)
 		return -ENOMEM;
-- 
2.5.4 (Apple Git-61)

[PATCH 2/4] f2fs: avoid unnecessary f2fs_gc for dir operations

[Jaegeuk Kim]

The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
works.

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

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 2c32110..8d2616f 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	nid_t ino = 0;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	int err = -ENOENT;
 
 	trace_f2fs_unlink_enter(dir, dentry);
-	f2fs_balance_fs(sbi);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
 	if (!de)
 		goto fail;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err) {
@@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	if (len > dir->i_sb->s_blocksize)
 		return -ENAMETOOLONG;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFDIR | mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
+	f2fs_balance_fs(sbi);
+
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
-- 
2.5.4 (Apple Git-61)


------------------------------------------------------------------------------

[PATCH 2/4] f2fs: avoid unnecessary f2fs_gc for dir operations

[Jaegeuk Kim]

The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
works.

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

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 2c32110..8d2616f 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	nid_t ino = 0;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	int err = -ENOENT;
 
 	trace_f2fs_unlink_enter(dir, dentry);
-	f2fs_balance_fs(sbi);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
 	if (!de)
 		goto fail;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err) {
@@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	if (len > dir->i_sb->s_blocksize)
 		return -ENAMETOOLONG;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFDIR | mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
+	f2fs_balance_fs(sbi);
+
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
-- 
2.5.4 (Apple Git-61)

RE: [f2fs-dev] [PATCH 2/4] f2fs: avoid unnecessary f2fs_gc for dir operations

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 2/4] f2fs: avoid unnecessary f2fs_gc for dir operations
> 
> The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
> works.

So this avoids some unneeded overhead in f2fs_balance_fs for scenario where
f2fs_new_inode and f2fs_find_entry will fail for most of the time?

Shouldn't cover f2fs_find_entry in rename or rename2 case?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/namei.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 2c32110..8d2616f 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	nid_t ino = 0;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	ino = inode->i_ino;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>  	int err = -ENOENT;
> 
>  	trace_f2fs_unlink_enter(dir, dentry);
> -	f2fs_balance_fs(sbi);
> 
>  	de = f2fs_find_entry(dir, &dentry->d_name, &page);
>  	if (!de)
>  		goto fail;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = acquire_orphan_inode(sbi);
>  	if (err) {
> @@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  	if (len > dir->i_sb->s_blocksize)
>  		return -ENAMETOOLONG;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  		inode->i_op = &f2fs_symlink_inode_operations;
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	struct inode *inode;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFDIR | mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
> 
> +	f2fs_balance_fs(sbi);
> +
>  	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
> --
> 2.5.4 (Apple Git-61)
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations

[Jaegeuk Kim]

Chang log from v1:
 - add more cases

>From 9fea6346f5dd2992525e853e27a4fe899d122379 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 11:56:08 -0800
Subject: [PATCH] f2fs: avoid unnecessary f2fs_gc for dir operations

The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
works.

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

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 2c32110..e4588f7 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	nid_t ino = 0;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	int err = -ENOENT;
 
 	trace_f2fs_unlink_enter(dir, dentry);
-	f2fs_balance_fs(sbi);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
 	if (!de)
 		goto fail;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err) {
@@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	if (len > dir->i_sb->s_blocksize)
 		return -ENAMETOOLONG;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFDIR | mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
+	f2fs_balance_fs(sbi);
+
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
@@ -481,8 +482,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err = 0;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -490,6 +489,8 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	init_special_inode(inode, inode->i_mode, rdev);
 	inode->i_op = &f2fs_special_inode_operations;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -516,9 +517,6 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err;
 
-	if (!whiteout)
-		f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -530,6 +528,8 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_file_inode_operations;
 		inode->i_fop = &f2fs_file_operations;
 		inode->i_mapping->a_ops = &f2fs_dblock_aops;
+
+		f2fs_balance_fs(sbi);
 	}
 
 	f2fs_lock_op(sbi);
@@ -604,8 +604,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		goto out;
 	}
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -635,6 +633,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		if (!new_entry)
 			goto out_whiteout;
 
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = acquire_orphan_inode(sbi);
@@ -666,6 +666,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		update_inode_page(old_inode);
 		update_inode_page(new_inode);
 	} else {
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = f2fs_add_link(new_dentry, old_inode);
@@ -763,8 +765,6 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 								new_inode)))
 		return -EPERM;
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -807,6 +807,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_new_dir;
 	}
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 
 	err = update_dent_inode(old_inode, new_inode, &new_dentry->d_name);
-- 
2.5.4 (Apple Git-61)

RE: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 24, 2015 2:55 AM
> To: Chao Yu
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations
> 
> Chang log from v1:
>  - add more cases
> 
> From 9fea6346f5dd2992525e853e27a4fe899d122379 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 22 Dec 2015 11:56:08 -0800
> Subject: [PATCH] f2fs: avoid unnecessary f2fs_gc for dir operations
> 
> The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
> works.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/namei.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 2c32110..e4588f7 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	nid_t ino = 0;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	ino = inode->i_ino;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>  	int err = -ENOENT;
> 
>  	trace_f2fs_unlink_enter(dir, dentry);
> -	f2fs_balance_fs(sbi);
> 
>  	de = f2fs_find_entry(dir, &dentry->d_name, &page);
>  	if (!de)
>  		goto fail;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = acquire_orphan_inode(sbi);
>  	if (err) {
> @@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  	if (len > dir->i_sb->s_blocksize)
>  		return -ENAMETOOLONG;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  		inode->i_op = &f2fs_symlink_inode_operations;
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	struct inode *inode;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFDIR | mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
> 
> +	f2fs_balance_fs(sbi);
> +
>  	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
> @@ -481,8 +482,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode;
>  	int err = 0;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -490,6 +489,8 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
>  	init_special_inode(inode, inode->i_mode, rdev);
>  	inode->i_op = &f2fs_special_inode_operations;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -516,9 +517,6 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode;
>  	int err;
> 
> -	if (!whiteout)
> -		f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -530,6 +528,8 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
>  		inode->i_op = &f2fs_file_inode_operations;
>  		inode->i_fop = &f2fs_file_operations;
>  		inode->i_mapping->a_ops = &f2fs_dblock_aops;
> +
> +		f2fs_balance_fs(sbi);
>  	}
> 
>  	f2fs_lock_op(sbi);
> @@ -604,8 +604,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		goto out;
>  	}
> 
> -	f2fs_balance_fs(sbi);
> -
>  	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
>  	if (!old_entry)
>  		goto out;
> @@ -635,6 +633,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		if (!new_entry)
>  			goto out_whiteout;
> 
> +		f2fs_balance_fs(sbi);

If we are in RENAME_WHITEOUT case, move f2fs_balance_fs here seems a bit late.
How about let __f2fs_tmpfile do reclaim for both whiteout and tmpfile case
itself, and do reclaim exclude whiteout case here?

Thanks,

> +
>  		f2fs_lock_op(sbi);
> 
>  		err = acquire_orphan_inode(sbi);
> @@ -666,6 +666,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		update_inode_page(old_inode);
>  		update_inode_page(new_inode);
>  	} else {
> +		f2fs_balance_fs(sbi);
> +
>  		f2fs_lock_op(sbi);
> 
>  		err = f2fs_add_link(new_dentry, old_inode);
> @@ -763,8 +765,6 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry
> *old_dentry,
>  								new_inode)))
>  		return -EPERM;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
>  	if (!old_entry)
>  		goto out;
> @@ -807,6 +807,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry
> *old_dentry,
>  			goto out_new_dir;
>  	}
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
> 
>  	err = update_dent_inode(old_inode, new_inode, &new_dentry->d_name);
> --
> 2.5.4 (Apple Git-61)

Re: [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations

[Jaegeuk Kim]

Hi Chao,

Right.
But, in the rename path, we still need to do f2fs_balance_fs, since it produces
another dirty node page in the mean time.

How about this?

>From bbc5bf8f6c940cd75a4d71ce40ce4bd3f647a823 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 11:56:08 -0800
Subject: [PATCH] f2fs: avoid unnecessary f2fs_gc for dir operations

The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
works.

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

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 2c32110..4e27c5c 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	nid_t ino = 0;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	int err = -ENOENT;
 
 	trace_f2fs_unlink_enter(dir, dentry);
-	f2fs_balance_fs(sbi);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
 	if (!de)
 		goto fail;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err) {
@@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	if (len > dir->i_sb->s_blocksize)
 		return -ENAMETOOLONG;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFDIR | mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
+	f2fs_balance_fs(sbi);
+
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
@@ -481,8 +482,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err = 0;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -490,6 +489,8 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	init_special_inode(inode, inode->i_mode, rdev);
 	inode->i_op = &f2fs_special_inode_operations;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -516,9 +517,6 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err;
 
-	if (!whiteout)
-		f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -532,6 +530,8 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 		inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	}
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err)
@@ -604,8 +604,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		goto out;
 	}
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -635,6 +633,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		if (!new_entry)
 			goto out_whiteout;
 
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = acquire_orphan_inode(sbi);
@@ -666,6 +666,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		update_inode_page(old_inode);
 		update_inode_page(new_inode);
 	} else {
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = f2fs_add_link(new_dentry, old_inode);
@@ -763,8 +765,6 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 								new_inode)))
 		return -EPERM;
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -807,6 +807,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_new_dir;
 	}
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 
 	err = update_dent_inode(old_inode, new_inode, &new_dentry->d_name);
-- 
2.5.4 (Apple Git-61)


------------------------------------------------------------------------------

Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations

[Jaegeuk Kim]

Hi Chao,

Right.
But, in the rename path, we still need to do f2fs_balance_fs, since it produces
another dirty node page in the mean time.

How about this?

>From bbc5bf8f6c940cd75a4d71ce40ce4bd3f647a823 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 11:56:08 -0800
Subject: [PATCH] f2fs: avoid unnecessary f2fs_gc for dir operations

The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
works.

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

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 2c32110..4e27c5c 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	nid_t ino = 0;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	ino = inode->i_ino;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
 	int err = -ENOENT;
 
 	trace_f2fs_unlink_enter(dir, dentry);
-	f2fs_balance_fs(sbi);
 
 	de = f2fs_find_entry(dir, &dentry->d_name, &page);
 	if (!de)
 		goto fail;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err) {
@@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 	if (len > dir->i_sb->s_blocksize)
 		return -ENAMETOOLONG;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
 		inode->i_op = &f2fs_symlink_inode_operations;
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, S_IFDIR | mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
 
+	f2fs_balance_fs(sbi);
+
 	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
@@ -481,8 +482,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err = 0;
 
-	f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -490,6 +489,8 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
 	init_special_inode(inode, inode->i_mode, rdev);
 	inode->i_op = &f2fs_special_inode_operations;
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = f2fs_add_link(dentry, inode);
 	if (err)
@@ -516,9 +517,6 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	int err;
 
-	if (!whiteout)
-		f2fs_balance_fs(sbi);
-
 	inode = f2fs_new_inode(dir, mode);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
@@ -532,6 +530,8 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 		inode->i_mapping->a_ops = &f2fs_dblock_aops;
 	}
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 	err = acquire_orphan_inode(sbi);
 	if (err)
@@ -604,8 +604,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		goto out;
 	}
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -635,6 +633,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		if (!new_entry)
 			goto out_whiteout;
 
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = acquire_orphan_inode(sbi);
@@ -666,6 +666,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		update_inode_page(old_inode);
 		update_inode_page(new_inode);
 	} else {
+		f2fs_balance_fs(sbi);
+
 		f2fs_lock_op(sbi);
 
 		err = f2fs_add_link(new_dentry, old_inode);
@@ -763,8 +765,6 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 								new_inode)))
 		return -EPERM;
 
-	f2fs_balance_fs(sbi);
-
 	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
 	if (!old_entry)
 		goto out;
@@ -807,6 +807,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_new_dir;
 	}
 
+	f2fs_balance_fs(sbi);
+
 	f2fs_lock_op(sbi);
 
 	err = update_dent_inode(old_inode, new_inode, &new_dentry->d_name);
-- 
2.5.4 (Apple Git-61)

RE: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 24, 2015 10:13 AM
> To: Chao Yu
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: avoid unnecessary f2fs_gc for dir operations
> 
> Hi Chao,
> 
> Right.
> But, in the rename path, we still need to do f2fs_balance_fs, since it produces
> another dirty node page in the mean time.

That's right.

> 
> How about this?
> 
> From bbc5bf8f6c940cd75a4d71ce40ce4bd3f647a823 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 22 Dec 2015 11:56:08 -0800
> Subject: [PATCH] f2fs: avoid unnecessary f2fs_gc for dir operations
> 
> The f2fs_balance_fs doesn't need to cover f2fs_new_inode or f2fs_find_entry
> works.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

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

Thanks,

> ---
>  fs/f2fs/namei.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 2c32110..4e27c5c 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -128,8 +128,6 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	nid_t ino = 0;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -142,6 +140,8 @@ static int f2fs_create(struct inode *dir, struct dentry *dentry, umode_t
> mode,
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	ino = inode->i_ino;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -288,12 +288,13 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
>  	int err = -ENOENT;
> 
>  	trace_f2fs_unlink_enter(dir, dentry);
> -	f2fs_balance_fs(sbi);
> 
>  	de = f2fs_find_entry(dir, &dentry->d_name, &page);
>  	if (!de)
>  		goto fail;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = acquire_orphan_inode(sbi);
>  	if (err) {
> @@ -341,8 +342,6 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  	if (len > dir->i_sb->s_blocksize)
>  		return -ENAMETOOLONG;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFLNK | S_IRWXUGO);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -353,6 +352,8 @@ static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
>  		inode->i_op = &f2fs_symlink_inode_operations;
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -433,8 +434,6 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	struct inode *inode;
>  	int err;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, S_IFDIR | mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -444,6 +443,8 @@ static int f2fs_mkdir(struct inode *dir, struct dentry *dentry, umode_t
> mode)
>  	inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
> 
> +	f2fs_balance_fs(sbi);
> +
>  	set_inode_flag(F2FS_I(inode), FI_INC_LINK);
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
> @@ -481,8 +482,6 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode;
>  	int err = 0;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -490,6 +489,8 @@ static int f2fs_mknod(struct inode *dir, struct dentry *dentry,
>  	init_special_inode(inode, inode->i_mode, rdev);
>  	inode->i_op = &f2fs_special_inode_operations;
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = f2fs_add_link(dentry, inode);
>  	if (err)
> @@ -516,9 +517,6 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode;
>  	int err;
> 
> -	if (!whiteout)
> -		f2fs_balance_fs(sbi);
> -
>  	inode = f2fs_new_inode(dir, mode);
>  	if (IS_ERR(inode))
>  		return PTR_ERR(inode);
> @@ -532,6 +530,8 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
>  		inode->i_mapping->a_ops = &f2fs_dblock_aops;
>  	}
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
>  	err = acquire_orphan_inode(sbi);
>  	if (err)
> @@ -604,8 +604,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		goto out;
>  	}
> 
> -	f2fs_balance_fs(sbi);
> -
>  	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
>  	if (!old_entry)
>  		goto out;
> @@ -635,6 +633,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		if (!new_entry)
>  			goto out_whiteout;
> 
> +		f2fs_balance_fs(sbi);
> +
>  		f2fs_lock_op(sbi);
> 
>  		err = acquire_orphan_inode(sbi);
> @@ -666,6 +666,8 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		update_inode_page(old_inode);
>  		update_inode_page(new_inode);
>  	} else {
> +		f2fs_balance_fs(sbi);
> +
>  		f2fs_lock_op(sbi);
> 
>  		err = f2fs_add_link(new_dentry, old_inode);
> @@ -763,8 +765,6 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry
> *old_dentry,
>  								new_inode)))
>  		return -EPERM;
> 
> -	f2fs_balance_fs(sbi);
> -
>  	old_entry = f2fs_find_entry(old_dir, &old_dentry->d_name, &old_page);
>  	if (!old_entry)
>  		goto out;
> @@ -807,6 +807,8 @@ static int f2fs_cross_rename(struct inode *old_dir, struct dentry
> *old_dentry,
>  			goto out_new_dir;
>  	}
> 
> +	f2fs_balance_fs(sbi);
> +
>  	f2fs_lock_op(sbi);
> 
>  	err = update_dent_inode(old_inode, new_inode, &new_dentry->d_name);
> --
> 2.5.4 (Apple Git-61)

[PATCH 3/4] f2fs: record node block allocation in dnode_of_data

[Jaegeuk Kim]

This patch introduces recording node block allocation in dnode_of_data.
This information helps to figure out whether any node block is allocated during
specific file operations.

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

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cf0c9dd..a7a9a05 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 	addr_array = blkaddr_in_node(rn);
 	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
 	set_page_dirty(node_page);
+	dn->node_changed = true;
 }
 
 int reserve_new_block(struct dnode_of_data *dn)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 90fb970..0f4d329 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -547,6 +547,7 @@ struct dnode_of_data {
 	unsigned int ofs_in_node;	/* data offset in the node page */
 	bool inode_page_locked;		/* inode page is locked or not */
 	block_t	data_blkaddr;		/* block address of the node block */
+	bool node_changed;		/* is node block changed */
 };
 
 static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f2effe1..10ed357 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -484,6 +484,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
 		dec_valid_block_count(sbi, dn->inode, nr_free);
 		set_page_dirty(dn->node_page);
 		sync_inode_page(dn);
+		dn->node_changed = true;
 	}
 	dn->ofs_in_node = ofs;
 
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 6cc8ac7..ff2acb1 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
 
 			set_nid(parent, offset[i - 1], nids[i], i == 1);
 			alloc_nid_done(sbi, nids[i]);
+			dn->node_changed = true;
 			done = true;
 		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
 			npage[i] = get_node_page_ra(parent, offset[i - 1]);
@@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			if (ret < 0)
 				goto out_err;
 			set_nid(page, i, 0, false);
+			dn->node_changed = true;
 		}
 	} else {
 		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
@@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
 			if (ret == (NIDS_PER_BLOCK + 1)) {
 				set_nid(page, i, 0, false);
+				dn->node_changed = true;
 				child_nofs += ret;
 			} else if (ret < 0 && ret != -ENOENT) {
 				goto out_err;
@@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
 		if (err < 0)
 			goto fail;
 		set_nid(pages[idx], i, 0, false);
+		dn->node_changed = true;
 	}
 
 	if (offset[idx + 1] == 0) {
-- 
2.5.4 (Apple Git-61)

Re: [PATCH 3/4] f2fs: record node block allocation in dnode_of_data

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 3/4] f2fs: record node block allocation in dnode_of_data
> 
> This patch introduces recording node block allocation in dnode_of_data.
> This information helps to figure out whether any node block is allocated during
> specific file operations.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c | 1 +
>  fs/f2fs/f2fs.h | 1 +
>  fs/f2fs/file.c | 1 +
>  fs/f2fs/node.c | 4 ++++
>  4 files changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index cf0c9dd..a7a9a05 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
>  	addr_array = blkaddr_in_node(rn);
>  	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
>  	set_page_dirty(node_page);
> +	dn->node_changed = true;
>  }
> 
>  int reserve_new_block(struct dnode_of_data *dn)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 90fb970..0f4d329 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -547,6 +547,7 @@ struct dnode_of_data {
>  	unsigned int ofs_in_node;	/* data offset in the node page */
>  	bool inode_page_locked;		/* inode page is locked or not */

Better to add node_changed here to avoid holes generated by compiler due
to alignment.

>  	block_t	data_blkaddr;		/* block address of the node block */
> +	bool node_changed;		/* is node block changed */

How about reset it in set_new_dnode?

>  };
> 
>  static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f2effe1..10ed357 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -484,6 +484,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  		dec_valid_block_count(sbi, dn->inode, nr_free);
>  		set_page_dirty(dn->node_page);
>  		sync_inode_page(dn);
> +		dn->node_changed = true;

dn->node_changed should have been set in set_data_blkaddr, so no needed to
set here.

Thanks,

>  	}
>  	dn->ofs_in_node = ofs;
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 6cc8ac7..ff2acb1 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
> 
>  			set_nid(parent, offset[i - 1], nids[i], i == 1);
>  			alloc_nid_done(sbi, nids[i]);
> +			dn->node_changed = true;
>  			done = true;
>  		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
>  			npage[i] = get_node_page_ra(parent, offset[i - 1]);
> @@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
>  			if (ret < 0)
>  				goto out_err;
>  			set_nid(page, i, 0, false);
> +			dn->node_changed = true;
>  		}
>  	} else {
>  		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
> @@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
>  			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
>  			if (ret == (NIDS_PER_BLOCK + 1)) {
>  				set_nid(page, i, 0, false);
> +				dn->node_changed = true;
>  				child_nofs += ret;
>  			} else if (ret < 0 && ret != -ENOENT) {
>  				goto out_err;
> @@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
>  		if (err < 0)
>  			goto fail;
>  		set_nid(pages[idx], i, 0, false);
> +		dn->node_changed = true;
>  	}
> 
>  	if (offset[idx + 1] == 0) {
> --
> 2.5.4 (Apple Git-61)
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


------------------------------------------------------------------------------

RE: [f2fs-dev] [PATCH 3/4] f2fs: record node block allocation in dnode_of_data

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 3/4] f2fs: record node block allocation in dnode_of_data
> 
> This patch introduces recording node block allocation in dnode_of_data.
> This information helps to figure out whether any node block is allocated during
> specific file operations.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c | 1 +
>  fs/f2fs/f2fs.h | 1 +
>  fs/f2fs/file.c | 1 +
>  fs/f2fs/node.c | 4 ++++
>  4 files changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index cf0c9dd..a7a9a05 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
>  	addr_array = blkaddr_in_node(rn);
>  	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
>  	set_page_dirty(node_page);
> +	dn->node_changed = true;
>  }
> 
>  int reserve_new_block(struct dnode_of_data *dn)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 90fb970..0f4d329 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -547,6 +547,7 @@ struct dnode_of_data {
>  	unsigned int ofs_in_node;	/* data offset in the node page */
>  	bool inode_page_locked;		/* inode page is locked or not */

Better to add node_changed here to avoid holes generated by compiler due
to alignment.

>  	block_t	data_blkaddr;		/* block address of the node block */
> +	bool node_changed;		/* is node block changed */

How about reset it in set_new_dnode?

>  };
> 
>  static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f2effe1..10ed357 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -484,6 +484,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  		dec_valid_block_count(sbi, dn->inode, nr_free);
>  		set_page_dirty(dn->node_page);
>  		sync_inode_page(dn);
> +		dn->node_changed = true;

dn->node_changed should have been set in set_data_blkaddr, so no needed to
set here.

Thanks,

>  	}
>  	dn->ofs_in_node = ofs;
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 6cc8ac7..ff2acb1 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
> 
>  			set_nid(parent, offset[i - 1], nids[i], i == 1);
>  			alloc_nid_done(sbi, nids[i]);
> +			dn->node_changed = true;
>  			done = true;
>  		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
>  			npage[i] = get_node_page_ra(parent, offset[i - 1]);
> @@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
>  			if (ret < 0)
>  				goto out_err;
>  			set_nid(page, i, 0, false);
> +			dn->node_changed = true;
>  		}
>  	} else {
>  		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
> @@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
>  			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
>  			if (ret == (NIDS_PER_BLOCK + 1)) {
>  				set_nid(page, i, 0, false);
> +				dn->node_changed = true;
>  				child_nofs += ret;
>  			} else if (ret < 0 && ret != -ENOENT) {
>  				goto out_err;
> @@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
>  		if (err < 0)
>  			goto fail;
>  		set_nid(pages[idx], i, 0, false);
> +		dn->node_changed = true;
>  	}
> 
>  	if (offset[idx + 1] == 0) {
> --
> 2.5.4 (Apple Git-61)
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 3/4] f2fs: record node block allocation in dnode_of_data

[Jaegeuk Kim]

Hi Chao,

On Wed, Dec 23, 2015 at 04:00:36PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Wednesday, December 23, 2015 9:00 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 3/4] f2fs: record node block allocation in dnode_of_data
> > 
> > This patch introduces recording node block allocation in dnode_of_data.
> > This information helps to figure out whether any node block is allocated during
> > specific file operations.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/data.c | 1 +
> >  fs/f2fs/f2fs.h | 1 +
> >  fs/f2fs/file.c | 1 +
> >  fs/f2fs/node.c | 4 ++++
> >  4 files changed, 7 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index cf0c9dd..a7a9a05 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
> >  	addr_array = blkaddr_in_node(rn);
> >  	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
> >  	set_page_dirty(node_page);
> > +	dn->node_changed = true;
> >  }
> > 
> >  int reserve_new_block(struct dnode_of_data *dn)
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 90fb970..0f4d329 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -547,6 +547,7 @@ struct dnode_of_data {
> >  	unsigned int ofs_in_node;	/* data offset in the node page */
> >  	bool inode_page_locked;		/* inode page is locked or not */
> 
> Better to add node_changed here to avoid holes generated by compiler due
> to alignment.

Ok.

> 
> >  	block_t	data_blkaddr;		/* block address of the node block */
> > +	bool node_changed;		/* is node block changed */
> 
> How about reset it in set_new_dnode?

In set_new_dnode(), memset() is called.

> 
> >  };
> > 
> >  static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index f2effe1..10ed357 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -484,6 +484,7 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
> >  		dec_valid_block_count(sbi, dn->inode, nr_free);
> >  		set_page_dirty(dn->node_page);
> >  		sync_inode_page(dn);
> > +		dn->node_changed = true;
> 
> dn->node_changed should have been set in set_data_blkaddr, so no needed to
> set here.

Right. :)

Thanks,

> 
> Thanks,
> 
> >  	}
> >  	dn->ofs_in_node = ofs;
> > 
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 6cc8ac7..ff2acb1 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
> > 
> >  			set_nid(parent, offset[i - 1], nids[i], i == 1);
> >  			alloc_nid_done(sbi, nids[i]);
> > +			dn->node_changed = true;
> >  			done = true;
> >  		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
> >  			npage[i] = get_node_page_ra(parent, offset[i - 1]);
> > @@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
> >  			if (ret < 0)
> >  				goto out_err;
> >  			set_nid(page, i, 0, false);
> > +			dn->node_changed = true;
> >  		}
> >  	} else {
> >  		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
> > @@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
> >  			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
> >  			if (ret == (NIDS_PER_BLOCK + 1)) {
> >  				set_nid(page, i, 0, false);
> > +				dn->node_changed = true;
> >  				child_nofs += ret;
> >  			} else if (ret < 0 && ret != -ENOENT) {
> >  				goto out_err;
> > @@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
> >  		if (err < 0)
> >  			goto fail;
> >  		set_nid(pages[idx], i, 0, false);
> > +		dn->node_changed = true;
> >  	}
> > 
> >  	if (offset[idx + 1] == 0) {
> > --
> > 2.5.4 (Apple Git-61)
> > 
> > 
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 3/4 v2] f2fs: record node block allocation in dnode_of_data

[Jaegeuk Kim]

Change log from v1:
 - remove redundant set
 - adjust memory alignment

>From 60c6a898094535e850268dd77701255a37cce3d3 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 12:59:54 -0800
Subject: [PATCH] f2fs: record node block allocation in dnode_of_data

This patch introduces recording node block allocation in dnode_of_data.
This information helps to figure out whether any node block is allocated during
specific file operations.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 1 +
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/node.c | 4 ++++
 3 files changed, 6 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cf0c9dd..a7a9a05 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 	addr_array = blkaddr_in_node(rn);
 	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
 	set_page_dirty(node_page);
+	dn->node_changed = true;
 }
 
 int reserve_new_block(struct dnode_of_data *dn)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 90fb970..3e4a60d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -546,6 +546,7 @@ struct dnode_of_data {
 	nid_t nid;			/* node id of the direct node block */
 	unsigned int ofs_in_node;	/* data offset in the node page */
 	bool inode_page_locked;		/* inode page is locked or not */
+	bool node_changed;		/* is node block changed */
 	block_t	data_blkaddr;		/* block address of the node block */
 };
 
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 6cc8ac7..ff2acb1 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
 
 			set_nid(parent, offset[i - 1], nids[i], i == 1);
 			alloc_nid_done(sbi, nids[i]);
+			dn->node_changed = true;
 			done = true;
 		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
 			npage[i] = get_node_page_ra(parent, offset[i - 1]);
@@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			if (ret < 0)
 				goto out_err;
 			set_nid(page, i, 0, false);
+			dn->node_changed = true;
 		}
 	} else {
 		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
@@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
 			if (ret == (NIDS_PER_BLOCK + 1)) {
 				set_nid(page, i, 0, false);
+				dn->node_changed = true;
 				child_nofs += ret;
 			} else if (ret < 0 && ret != -ENOENT) {
 				goto out_err;
@@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
 		if (err < 0)
 			goto fail;
 		set_nid(pages[idx], i, 0, false);
+		dn->node_changed = true;
 	}
 
 	if (offset[idx + 1] == 0) {
-- 
2.5.4 (Apple Git-61)


------------------------------------------------------------------------------

Re: [PATCH 3/4 v2] f2fs: record node block allocation in dnode_of_data

[Jaegeuk Kim]

Change log from v1:
 - remove redundant set
 - adjust memory alignment

>From 60c6a898094535e850268dd77701255a37cce3d3 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 12:59:54 -0800
Subject: [PATCH] f2fs: record node block allocation in dnode_of_data

This patch introduces recording node block allocation in dnode_of_data.
This information helps to figure out whether any node block is allocated during
specific file operations.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 1 +
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/node.c | 4 ++++
 3 files changed, 6 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cf0c9dd..a7a9a05 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -226,6 +226,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 	addr_array = blkaddr_in_node(rn);
 	addr_array[ofs_in_node] = cpu_to_le32(dn->data_blkaddr);
 	set_page_dirty(node_page);
+	dn->node_changed = true;
 }
 
 int reserve_new_block(struct dnode_of_data *dn)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 90fb970..3e4a60d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -546,6 +546,7 @@ struct dnode_of_data {
 	nid_t nid;			/* node id of the direct node block */
 	unsigned int ofs_in_node;	/* data offset in the node page */
 	bool inode_page_locked;		/* inode page is locked or not */
+	bool node_changed;		/* is node block changed */
 	block_t	data_blkaddr;		/* block address of the node block */
 };
 
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 6cc8ac7..ff2acb1 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -542,6 +542,7 @@ int get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode)
 
 			set_nid(parent, offset[i - 1], nids[i], i == 1);
 			alloc_nid_done(sbi, nids[i]);
+			dn->node_changed = true;
 			done = true;
 		} else if (mode == LOOKUP_NODE_RA && i == level && level > 1) {
 			npage[i] = get_node_page_ra(parent, offset[i - 1]);
@@ -678,6 +679,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			if (ret < 0)
 				goto out_err;
 			set_nid(page, i, 0, false);
+			dn->node_changed = true;
 		}
 	} else {
 		child_nofs = nofs + ofs * (NIDS_PER_BLOCK + 1) + 1;
@@ -691,6 +693,7 @@ static int truncate_nodes(struct dnode_of_data *dn, unsigned int nofs,
 			ret = truncate_nodes(&rdn, child_nofs, 0, depth - 1);
 			if (ret == (NIDS_PER_BLOCK + 1)) {
 				set_nid(page, i, 0, false);
+				dn->node_changed = true;
 				child_nofs += ret;
 			} else if (ret < 0 && ret != -ENOENT) {
 				goto out_err;
@@ -752,6 +755,7 @@ static int truncate_partial_nodes(struct dnode_of_data *dn,
 		if (err < 0)
 			goto fail;
 		set_nid(pages[idx], i, 0, false);
+		dn->node_changed = true;
 	}
 
 	if (offset[idx + 1] == 0) {
-- 
2.5.4 (Apple Git-61)

RE: [f2fs-dev] [PATCH 3/4 v2] f2fs: record node block allocation in dnode_of_data

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 24, 2015 3:00 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 3/4 v2] f2fs: record node block allocation in dnode_of_data
> 
> Change log from v1:
>  - remove redundant set
>  - adjust memory alignment
> 
> >From 60c6a898094535e850268dd77701255a37cce3d3 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 22 Dec 2015 12:59:54 -0800
> Subject: [PATCH] f2fs: record node block allocation in dnode_of_data
> 
> This patch introduces recording node block allocation in dnode_of_data.
> This information helps to figure out whether any node block is allocated during
> specific file operations.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

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

[PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Jaegeuk Kim]

If user tries to update or read data, we don't need to call f2fs_balance_fs
which triggers f2fs_gc, which increases unnecessary long latency.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c   | 19 ++++++++++++++++---
 fs/f2fs/file.c   | 26 +++++++++-----------------
 fs/f2fs/inline.c |  4 ++++
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a7a9a05..8f8f8b0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -509,7 +509,6 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 	u64 end_offset;
 
 	while (len) {
-		f2fs_balance_fs(sbi);
 		f2fs_lock_op(sbi);
 
 		/* When reading holes, we need its node page */
@@ -542,6 +541,9 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 
 		f2fs_put_dnode(&dn);
 		f2fs_unlock_op(sbi);
+
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
 	}
 	return;
 
@@ -551,6 +553,8 @@ sync_out:
 	f2fs_put_dnode(&dn);
 out:
 	f2fs_unlock_op(sbi);
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
 	return;
 }
 
@@ -1410,8 +1414,6 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * We should check this at this moment to avoid deadlock on inode page
 	 * and #0 page. The locking rule for inline_data conversion should be:
@@ -1461,6 +1463,17 @@ put_next:
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed && has_not_enough_free_secs(sbi, 0)) {
+		unlock_page(page);
+		f2fs_balance_fs(sbi);
+		lock_page(page);
+		if (page->mapping != mapping) {
+			/* The page got truncated from under us */
+			f2fs_put_page(page, 1);
+			goto repeat;
+		}
+	}
+
 	f2fs_wait_on_page_writeback(page, DATA);
 
 	/* wait for GCed encrypted page writeback */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 10ed357..dbc08bb 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -40,8 +40,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	struct dnode_of_data dn;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	sb_start_pagefault(inode->i_sb);
 
 	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
@@ -57,6 +55,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	file_update_time(vma->vm_file);
 	lock_page(page);
 	if (unlikely(page->mapping != inode->i_mapping ||
@@ -233,9 +234,6 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 		goto out;
 	}
 go_write:
-	/* guarantee free sections for fsync */
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * Both of fdatasync() and fsync() are able to be recovered from
 	 * sudden-power-off.
@@ -267,6 +265,8 @@ sync_nodes:
 	if (need_inode_block_update(sbi, ino)) {
 		mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
+
+		f2fs_balance_fs(sbi);
 		goto sync_nodes;
 	}
 
@@ -946,8 +946,6 @@ 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));
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -994,8 +992,6 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -1105,12 +1101,12 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (offset & (F2FS_BLKSIZE - 1) || len & (F2FS_BLKSIZE - 1))
 		return -EINVAL;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
 		return ret;
@@ -1153,8 +1149,6 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	loff_t off_start, off_end;
 	int ret = 0;
 
-	f2fs_balance_fs(sbi);
-
 	ret = inode_newsize_ok(inode, (len + offset));
 	if (ret)
 		return ret;
@@ -1163,6 +1157,8 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -1350,8 +1346,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	if (f2fs_is_atomic_file(inode))
 		return 0;
 
@@ -1438,8 +1432,6 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	clear_inode_flag(F2FS_I(inode), FI_ATOMIC_FILE);
 	clear_inode_flag(F2FS_I(inode), FI_VOLATILE_FILE);
 	commit_inmem_pages(inode, true);
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 8090854..c24e5d9 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -202,6 +202,10 @@ out:
 	f2fs_unlock_op(sbi);
 
 	f2fs_put_page(page, 1);
+
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	return err;
 }
 
-- 
2.5.4 (Apple Git-61)


------------------------------------------------------------------------------

[PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Jaegeuk Kim]

If user tries to update or read data, we don't need to call f2fs_balance_fs
which triggers f2fs_gc, which increases unnecessary long latency.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c   | 19 ++++++++++++++++---
 fs/f2fs/file.c   | 26 +++++++++-----------------
 fs/f2fs/inline.c |  4 ++++
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a7a9a05..8f8f8b0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -509,7 +509,6 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 	u64 end_offset;
 
 	while (len) {
-		f2fs_balance_fs(sbi);
 		f2fs_lock_op(sbi);
 
 		/* When reading holes, we need its node page */
@@ -542,6 +541,9 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 
 		f2fs_put_dnode(&dn);
 		f2fs_unlock_op(sbi);
+
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
 	}
 	return;
 
@@ -551,6 +553,8 @@ sync_out:
 	f2fs_put_dnode(&dn);
 out:
 	f2fs_unlock_op(sbi);
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
 	return;
 }
 
@@ -1410,8 +1414,6 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * We should check this at this moment to avoid deadlock on inode page
 	 * and #0 page. The locking rule for inline_data conversion should be:
@@ -1461,6 +1463,17 @@ put_next:
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed && has_not_enough_free_secs(sbi, 0)) {
+		unlock_page(page);
+		f2fs_balance_fs(sbi);
+		lock_page(page);
+		if (page->mapping != mapping) {
+			/* The page got truncated from under us */
+			f2fs_put_page(page, 1);
+			goto repeat;
+		}
+	}
+
 	f2fs_wait_on_page_writeback(page, DATA);
 
 	/* wait for GCed encrypted page writeback */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 10ed357..dbc08bb 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -40,8 +40,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	struct dnode_of_data dn;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	sb_start_pagefault(inode->i_sb);
 
 	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
@@ -57,6 +55,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	file_update_time(vma->vm_file);
 	lock_page(page);
 	if (unlikely(page->mapping != inode->i_mapping ||
@@ -233,9 +234,6 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 		goto out;
 	}
 go_write:
-	/* guarantee free sections for fsync */
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * Both of fdatasync() and fsync() are able to be recovered from
 	 * sudden-power-off.
@@ -267,6 +265,8 @@ sync_nodes:
 	if (need_inode_block_update(sbi, ino)) {
 		mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
+
+		f2fs_balance_fs(sbi);
 		goto sync_nodes;
 	}
 
@@ -946,8 +946,6 @@ 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));
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -994,8 +992,6 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -1105,12 +1101,12 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (offset & (F2FS_BLKSIZE - 1) || len & (F2FS_BLKSIZE - 1))
 		return -EINVAL;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
 		return ret;
@@ -1153,8 +1149,6 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	loff_t off_start, off_end;
 	int ret = 0;
 
-	f2fs_balance_fs(sbi);
-
 	ret = inode_newsize_ok(inode, (len + offset));
 	if (ret)
 		return ret;
@@ -1163,6 +1157,8 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -1350,8 +1346,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	if (f2fs_is_atomic_file(inode))
 		return 0;
 
@@ -1438,8 +1432,6 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	clear_inode_flag(F2FS_I(inode), FI_ATOMIC_FILE);
 	clear_inode_flag(F2FS_I(inode), FI_VOLATILE_FILE);
 	commit_inmem_pages(inode, true);
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 8090854..c24e5d9 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -202,6 +202,10 @@ out:
 	f2fs_unlock_op(sbi);
 
 	f2fs_put_page(page, 1);
+
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	return err;
 }
 
-- 
2.5.4 (Apple Git-61)

Re: [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> If user tries to update or read data, we don't need to call f2fs_balance_fs
> which triggers f2fs_gc, which increases unnecessary long latency.

One missing case is get_data_block_dio, how about also covering it based on
following patch?


>From 7175efac7473e7a04285055c69edfb7432f8ca4e Mon Sep 17 00:00:00 2001
From: Chao Yu <chao2.yu@samsung.com>
Date: Wed, 23 Dec 2015 17:11:43 +0800
Subject: [PATCH] f2fs: reduce covered region of sbi->cp_rwsem in
 f2fs_map_blocks

Only cover sbi->cp_rwsem on one dnode page's allocation and modification
instead of multiple's in f2fs_map_blocks, it can reduce the covered region
of cp_rwsem, then we can avoid potential long time delay for concurrent
checkpointer.

Signed-off-by: Chao Yu <chao2.yu@samsung.com>
---
 fs/f2fs/data.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8f8f8b0..3c83b16 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -594,7 +594,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	}
 
 	if (create)
-		f2fs_lock_op(F2FS_I_SB(inode));
+		f2fs_lock_op(sbi);
 
 	/* When reading holes, we need its node page */
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
@@ -651,6 +651,11 @@ get_next:
 		allocated = false;
 		f2fs_put_dnode(&dn);
 
+		if (create) {
+			f2fs_unlock_op(sbi);
+			f2fs_lock_op(sbi);
+		}
+
 		set_new_dnode(&dn, inode, NULL, NULL, 0);
 		err = get_dnode_of_data(&dn, pgofs, mode);
 		if (err) {
@@ -706,7 +711,7 @@ put_out:
 	f2fs_put_dnode(&dn);
 unlock_out:
 	if (create)
-		f2fs_unlock_op(F2FS_I_SB(inode));
+		f2fs_unlock_op(sbi);
 out:
 	trace_f2fs_map_blocks(inode, map, err);
 	return err;
-- 
2.6.3



------------------------------------------------------------------------------

RE: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> If user tries to update or read data, we don't need to call f2fs_balance_fs
> which triggers f2fs_gc, which increases unnecessary long latency.

One missing case is get_data_block_dio, how about also covering it based on
following patch?


>From 7175efac7473e7a04285055c69edfb7432f8ca4e Mon Sep 17 00:00:00 2001
From: Chao Yu <chao2.yu@samsung.com>
Date: Wed, 23 Dec 2015 17:11:43 +0800
Subject: [PATCH] f2fs: reduce covered region of sbi->cp_rwsem in
 f2fs_map_blocks

Only cover sbi->cp_rwsem on one dnode page's allocation and modification
instead of multiple's in f2fs_map_blocks, it can reduce the covered region
of cp_rwsem, then we can avoid potential long time delay for concurrent
checkpointer.

Signed-off-by: Chao Yu <chao2.yu@samsung.com>
---
 fs/f2fs/data.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8f8f8b0..3c83b16 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -594,7 +594,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	}
 
 	if (create)
-		f2fs_lock_op(F2FS_I_SB(inode));
+		f2fs_lock_op(sbi);
 
 	/* When reading holes, we need its node page */
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
@@ -651,6 +651,11 @@ get_next:
 		allocated = false;
 		f2fs_put_dnode(&dn);
 
+		if (create) {
+			f2fs_unlock_op(sbi);
+			f2fs_lock_op(sbi);
+		}
+
 		set_new_dnode(&dn, inode, NULL, NULL, 0);
 		err = get_dnode_of_data(&dn, pgofs, mode);
 		if (err) {
@@ -706,7 +711,7 @@ put_out:
 	f2fs_put_dnode(&dn);
 unlock_out:
 	if (create)
-		f2fs_unlock_op(F2FS_I_SB(inode));
+		f2fs_unlock_op(sbi);
 out:
 	trace_f2fs_map_blocks(inode, map, err);
 	return err;
-- 
2.6.3

Re: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Jaegeuk Kim]

Hi Chao,

On Wed, Dec 23, 2015 at 05:46:59PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Wednesday, December 23, 2015 9:00 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> > 
> > If user tries to update or read data, we don't need to call f2fs_balance_fs
> > which triggers f2fs_gc, which increases unnecessary long latency.
> 
> One missing case is get_data_block_dio, how about also covering it based on
> following patch?

It makes sense.
I'll submit v2.

Thanks,

> 
> 
> >From 7175efac7473e7a04285055c69edfb7432f8ca4e Mon Sep 17 00:00:00 2001
> From: Chao Yu <chao2.yu@samsung.com>
> Date: Wed, 23 Dec 2015 17:11:43 +0800
> Subject: [PATCH] f2fs: reduce covered region of sbi->cp_rwsem in
>  f2fs_map_blocks
> 
> Only cover sbi->cp_rwsem on one dnode page's allocation and modification
> instead of multiple's in f2fs_map_blocks, it can reduce the covered region
> of cp_rwsem, then we can avoid potential long time delay for concurrent
> checkpointer.
> 
> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> ---
>  fs/f2fs/data.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8f8f8b0..3c83b16 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -594,7 +594,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	}
>  
>  	if (create)
> -		f2fs_lock_op(F2FS_I_SB(inode));
> +		f2fs_lock_op(sbi);
>  
>  	/* When reading holes, we need its node page */
>  	set_new_dnode(&dn, inode, NULL, NULL, 0);
> @@ -651,6 +651,11 @@ get_next:
>  		allocated = false;
>  		f2fs_put_dnode(&dn);
>  
> +		if (create) {
> +			f2fs_unlock_op(sbi);
> +			f2fs_lock_op(sbi);
> +		}
> +
>  		set_new_dnode(&dn, inode, NULL, NULL, 0);
>  		err = get_dnode_of_data(&dn, pgofs, mode);
>  		if (err) {
> @@ -706,7 +711,7 @@ put_out:
>  	f2fs_put_dnode(&dn);
>  unlock_out:
>  	if (create)
> -		f2fs_unlock_op(F2FS_I_SB(inode));
> +		f2fs_unlock_op(sbi);
>  out:
>  	trace_f2fs_map_blocks(inode, map, err);
>  	return err;
> -- 
> 2.6.3
> 

Re: [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Chao Yu [mailto:chao2.yu@samsung.com]
> Sent: Wednesday, December 23, 2015 5:47 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Wednesday, December 23, 2015 9:00 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> >
> > If user tries to update or read data, we don't need to call f2fs_balance_fs
> > which triggers f2fs_gc, which increases unnecessary long latency.
> 
> One missing case is get_data_block_dio, how about also covering it based on
> following patch?
> 
> 
> >From 7175efac7473e7a04285055c69edfb7432f8ca4e Mon Sep 17 00:00:00 2001
> From: Chao Yu <chao2.yu@samsung.com>
> Date: Wed, 23 Dec 2015 17:11:43 +0800
> Subject: [PATCH] f2fs: reduce covered region of sbi->cp_rwsem in
>  f2fs_map_blocks

Title of this commit log was not merged into dev-test branch correctly,
could you please reedit it? :)

Thanks,



------------------------------------------------------------------------------

RE: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Chao Yu [mailto:chao2.yu@samsung.com]
> Sent: Wednesday, December 23, 2015 5:47 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Wednesday, December 23, 2015 9:00 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 4/4] f2fs: call f2fs_balance_fs only when node was changed
> >
> > If user tries to update or read data, we don't need to call f2fs_balance_fs
> > which triggers f2fs_gc, which increases unnecessary long latency.
> 
> One missing case is get_data_block_dio, how about also covering it based on
> following patch?
> 
> 
> >From 7175efac7473e7a04285055c69edfb7432f8ca4e Mon Sep 17 00:00:00 2001
> From: Chao Yu <chao2.yu@samsung.com>
> Date: Wed, 23 Dec 2015 17:11:43 +0800
> Subject: [PATCH] f2fs: reduce covered region of sbi->cp_rwsem in
>  f2fs_map_blocks

Title of this commit log was not merged into dev-test branch correctly,
could you please reedit it? :)

Thanks,

Re: [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Jaegeuk Kim]

Change log v2:
 - add dio case

>From c2d16a526371954671f9c8cff5f09f9d230f7993 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 13:23:35 -0800
Subject: [PATCH] f2fs: call f2fs_balance_fs only when node was changed

If user tries to update or read data, we don't need to call f2fs_balance_fs
which triggers f2fs_gc, which increases unnecessary long latency.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c   | 26 ++++++++++++++++++++++----
 fs/f2fs/file.c   | 26 +++++++++-----------------
 fs/f2fs/inline.c |  4 ++++
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 82ecaa30..958d826 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -509,7 +509,6 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 	u64 end_offset;
 
 	while (len) {
-		f2fs_balance_fs(sbi);
 		f2fs_lock_op(sbi);
 
 		/* When reading holes, we need its node page */
@@ -542,6 +541,9 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 
 		f2fs_put_dnode(&dn);
 		f2fs_unlock_op(sbi);
+
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
 	}
 	return;
 
@@ -551,6 +553,8 @@ sync_out:
 	f2fs_put_dnode(&dn);
 out:
 	f2fs_unlock_op(sbi);
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
 	return;
 }
 
@@ -649,6 +653,8 @@ get_next:
 
 		if (create) {
 			f2fs_unlock_op(sbi);
+			if (dn.node_changed)
+				f2fs_balance_fs(sbi);
 			f2fs_lock_op(sbi);
 		}
 
@@ -706,8 +712,11 @@ sync_out:
 put_out:
 	f2fs_put_dnode(&dn);
 unlock_out:
-	if (create)
+	if (create) {
 		f2fs_unlock_op(sbi);
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
+	}
 out:
 	trace_f2fs_map_blocks(inode, map, err);
 	return err;
@@ -1415,8 +1424,6 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * We should check this at this moment to avoid deadlock on inode page
 	 * and #0 page. The locking rule for inline_data conversion should be:
@@ -1466,6 +1473,17 @@ put_next:
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed && has_not_enough_free_secs(sbi, 0)) {
+		unlock_page(page);
+		f2fs_balance_fs(sbi);
+		lock_page(page);
+		if (page->mapping != mapping) {
+			/* The page got truncated from under us */
+			f2fs_put_page(page, 1);
+			goto repeat;
+		}
+	}
+
 	f2fs_wait_on_page_writeback(page, DATA);
 
 	/* wait for GCed encrypted page writeback */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f2effe1..888ce47 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -40,8 +40,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	struct dnode_of_data dn;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	sb_start_pagefault(inode->i_sb);
 
 	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
@@ -57,6 +55,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	file_update_time(vma->vm_file);
 	lock_page(page);
 	if (unlikely(page->mapping != inode->i_mapping ||
@@ -233,9 +234,6 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 		goto out;
 	}
 go_write:
-	/* guarantee free sections for fsync */
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * Both of fdatasync() and fsync() are able to be recovered from
 	 * sudden-power-off.
@@ -267,6 +265,8 @@ sync_nodes:
 	if (need_inode_block_update(sbi, ino)) {
 		mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
+
+		f2fs_balance_fs(sbi);
 		goto sync_nodes;
 	}
 
@@ -945,8 +945,6 @@ 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));
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -993,8 +991,6 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -1104,12 +1100,12 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (offset & (F2FS_BLKSIZE - 1) || len & (F2FS_BLKSIZE - 1))
 		return -EINVAL;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
 		return ret;
@@ -1152,8 +1148,6 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	loff_t off_start, off_end;
 	int ret = 0;
 
-	f2fs_balance_fs(sbi);
-
 	ret = inode_newsize_ok(inode, (len + offset));
 	if (ret)
 		return ret;
@@ -1162,6 +1156,8 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -1349,8 +1345,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	if (f2fs_is_atomic_file(inode))
 		return 0;
 
@@ -1437,8 +1431,6 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	clear_inode_flag(F2FS_I(inode), FI_ATOMIC_FILE);
 	clear_inode_flag(F2FS_I(inode), FI_VOLATILE_FILE);
 	commit_inmem_pages(inode, true);
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 8090854..c24e5d9 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -202,6 +202,10 @@ out:
 	f2fs_unlock_op(sbi);
 
 	f2fs_put_page(page, 1);
+
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	return err;
 }
 
-- 
2.5.4 (Apple Git-61)


------------------------------------------------------------------------------

Re: [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Jaegeuk Kim]

Change log v2:
 - add dio case

>From c2d16a526371954671f9c8cff5f09f9d230f7993 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 22 Dec 2015 13:23:35 -0800
Subject: [PATCH] f2fs: call f2fs_balance_fs only when node was changed

If user tries to update or read data, we don't need to call f2fs_balance_fs
which triggers f2fs_gc, which increases unnecessary long latency.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c   | 26 ++++++++++++++++++++++----
 fs/f2fs/file.c   | 26 +++++++++-----------------
 fs/f2fs/inline.c |  4 ++++
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 82ecaa30..958d826 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -509,7 +509,6 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 	u64 end_offset;
 
 	while (len) {
-		f2fs_balance_fs(sbi);
 		f2fs_lock_op(sbi);
 
 		/* When reading holes, we need its node page */
@@ -542,6 +541,9 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset,
 
 		f2fs_put_dnode(&dn);
 		f2fs_unlock_op(sbi);
+
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
 	}
 	return;
 
@@ -551,6 +553,8 @@ sync_out:
 	f2fs_put_dnode(&dn);
 out:
 	f2fs_unlock_op(sbi);
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
 	return;
 }
 
@@ -649,6 +653,8 @@ get_next:
 
 		if (create) {
 			f2fs_unlock_op(sbi);
+			if (dn.node_changed)
+				f2fs_balance_fs(sbi);
 			f2fs_lock_op(sbi);
 		}
 
@@ -706,8 +712,11 @@ sync_out:
 put_out:
 	f2fs_put_dnode(&dn);
 unlock_out:
-	if (create)
+	if (create) {
 		f2fs_unlock_op(sbi);
+		if (dn.node_changed)
+			f2fs_balance_fs(sbi);
+	}
 out:
 	trace_f2fs_map_blocks(inode, map, err);
 	return err;
@@ -1415,8 +1424,6 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * We should check this at this moment to avoid deadlock on inode page
 	 * and #0 page. The locking rule for inline_data conversion should be:
@@ -1466,6 +1473,17 @@ put_next:
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed && has_not_enough_free_secs(sbi, 0)) {
+		unlock_page(page);
+		f2fs_balance_fs(sbi);
+		lock_page(page);
+		if (page->mapping != mapping) {
+			/* The page got truncated from under us */
+			f2fs_put_page(page, 1);
+			goto repeat;
+		}
+	}
+
 	f2fs_wait_on_page_writeback(page, DATA);
 
 	/* wait for GCed encrypted page writeback */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f2effe1..888ce47 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -40,8 +40,6 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	struct dnode_of_data dn;
 	int err;
 
-	f2fs_balance_fs(sbi);
-
 	sb_start_pagefault(inode->i_sb);
 
 	f2fs_bug_on(sbi, f2fs_has_inline_data(inode));
@@ -57,6 +55,9 @@ static int f2fs_vm_page_mkwrite(struct vm_area_struct *vma,
 	f2fs_put_dnode(&dn);
 	f2fs_unlock_op(sbi);
 
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	file_update_time(vma->vm_file);
 	lock_page(page);
 	if (unlikely(page->mapping != inode->i_mapping ||
@@ -233,9 +234,6 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 		goto out;
 	}
 go_write:
-	/* guarantee free sections for fsync */
-	f2fs_balance_fs(sbi);
-
 	/*
 	 * Both of fdatasync() and fsync() are able to be recovered from
 	 * sudden-power-off.
@@ -267,6 +265,8 @@ sync_nodes:
 	if (need_inode_block_update(sbi, ino)) {
 		mark_inode_dirty_sync(inode);
 		f2fs_write_inode(inode, NULL);
+
+		f2fs_balance_fs(sbi);
 		goto sync_nodes;
 	}
 
@@ -945,8 +945,6 @@ 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));
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -993,8 +991,6 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
@@ -1104,12 +1100,12 @@ static int f2fs_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (offset & (F2FS_BLKSIZE - 1) || len & (F2FS_BLKSIZE - 1))
 		return -EINVAL;
 
-	f2fs_balance_fs(sbi);
-
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	ret = truncate_blocks(inode, i_size_read(inode), true);
 	if (ret)
 		return ret;
@@ -1152,8 +1148,6 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	loff_t off_start, off_end;
 	int ret = 0;
 
-	f2fs_balance_fs(sbi);
-
 	ret = inode_newsize_ok(inode, (len + offset));
 	if (ret)
 		return ret;
@@ -1162,6 +1156,8 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (ret)
 		return ret;
 
+	f2fs_balance_fs(sbi);
+
 	pg_start = ((unsigned long long) offset) >> PAGE_CACHE_SHIFT;
 	pg_end = ((unsigned long long) offset + len) >> PAGE_CACHE_SHIFT;
 
@@ -1349,8 +1345,6 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	if (f2fs_is_atomic_file(inode))
 		return 0;
 
@@ -1437,8 +1431,6 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 	if (ret)
 		return ret;
 
-	f2fs_balance_fs(F2FS_I_SB(inode));
-
 	clear_inode_flag(F2FS_I(inode), FI_ATOMIC_FILE);
 	clear_inode_flag(F2FS_I(inode), FI_VOLATILE_FILE);
 	commit_inmem_pages(inode, true);
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 8090854..c24e5d9 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -202,6 +202,10 @@ out:
 	f2fs_unlock_op(sbi);
 
 	f2fs_put_page(page, 1);
+
+	if (dn.node_changed)
+		f2fs_balance_fs(sbi);
+
 	return err;
 }
 
-- 
2.5.4 (Apple Git-61)

Re: [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 24, 2015 3:14 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> Change log v2:
>  - add dio case
> 
> >From c2d16a526371954671f9c8cff5f09f9d230f7993 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 22 Dec 2015 13:23:35 -0800
> Subject: [PATCH] f2fs: call f2fs_balance_fs only when node was changed
> 
> If user tries to update or read data, we don't need to call f2fs_balance_fs
> which triggers f2fs_gc, which increases unnecessary long latency.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

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


------------------------------------------------------------------------------

RE: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 24, 2015 3:14 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 4/4] f2fs: call f2fs_balance_fs only when node was changed
> 
> Change log v2:
>  - add dio case
> 
> >From c2d16a526371954671f9c8cff5f09f9d230f7993 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 22 Dec 2015 13:23:35 -0800
> Subject: [PATCH] f2fs: call f2fs_balance_fs only when node was changed
> 
> If user tries to update or read data, we don't need to call f2fs_balance_fs
> which triggers f2fs_gc, which increases unnecessary long latency.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

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

RE: [f2fs-dev] [PATCH 1/4] f2fs: check inline_data flag at converting time

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 23, 2015 9:00 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 1/4] f2fs: check inline_data flag at converting time
> 
> We can check inode's inline_data flag  when calling to convert it.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

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