[PATCH 1/3] f2fs: check if inode state is dirty at fsync

[PATCH 1/3] f2fs: check if inode state is dirty at fsync

[Changman Lee]

If inode state is dirty, go straight to write.

Suggested-by: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Changman Lee <cm224.lee@samsung.com>
---
 fs/f2fs/file.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b6f3fbf..0b97002 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -138,6 +138,17 @@ static inline bool need_do_checkpoint(struct inode *inode)
 	return need_cp;
 }
 
+static bool need_inode_page_update(struct f2fs_sb_info *sbi, nid_t ino)
+{
+	struct page *i = find_get_page(NODE_MAPPING(sbi), ino);
+	bool ret = false;
+	/* But we need to avoid that there are some inode updates */
+	if ((i && PageDirty(i)) || need_inode_block_update(sbi, ino))
+		ret = true;
+	f2fs_put_page(i, 0);
+	return ret;
+}
+
 int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 {
 	struct inode *inode = file->f_mapping->host;
@@ -168,19 +179,21 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 		return ret;
 	}
 
+	/* if the inode is dirty, let's recover all the time */
+	if (!datasync && is_inode_flag_set(fi, FI_DIRTY_INODE)) {
+		update_inode_page(inode);
+		goto go_write;
+	}
+
 	/*
 	 * if there is no written data, don't waste time to write recovery info.
 	 */
 	if (!is_inode_flag_set(fi, FI_APPEND_WRITE) &&
 			!exist_written_data(sbi, ino, APPEND_INO)) {
-		struct page *i = find_get_page(NODE_MAPPING(sbi), ino);
 
-		/* But we need to avoid that there are some inode updates */
-		if ((i && PageDirty(i)) || need_inode_block_update(sbi, ino)) {
-			f2fs_put_page(i, 0);
+		/* it may call write_inode just prior to fsync */
+		if (need_inode_page_update(sbi, ino))
 			goto go_write;
-		}
-		f2fs_put_page(i, 0);
 
 		if (is_inode_flag_set(fi, FI_UPDATE_WRITE) ||
 				exist_written_data(sbi, ino, UPDATE_INO))
-- 
1.9.1

[PATCH 2/3] f2fs: cleanup path to need cp at fsync

[Changman Lee]

Added some commentaries for code readability and cleaned up if-statement
clearly.

Signed-off-by: Changman Lee <cm224.lee@samsung.com>
---
 fs/f2fs/file.c | 79 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0b97002..3c27e0e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -149,6 +149,26 @@ static bool need_inode_page_update(struct f2fs_sb_info *sbi, nid_t ino)
 	return ret;
 }
 
+static void try_to_fix_pino(struct inode *inode)
+{
+	struct f2fs_inode_info *fi = F2FS_I(inode);
+	nid_t pino;
+
+	down_write(&fi->i_sem);
+	fi->xattr_ver = 0;
+	if (file_wrong_pino(inode) && inode->i_nlink == 1 &&
+			get_parent_ino(inode, &pino)) {
+		fi->i_pino = pino;
+		file_got_pino(inode);
+		up_write(&fi->i_sem);
+
+		mark_inode_dirty_sync(inode);
+		f2fs_write_inode(inode, NULL);
+	} else {
+		up_write(&fi->i_sem);
+	}
+}
+
 int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 {
 	struct inode *inode = file->f_mapping->host;
@@ -213,49 +233,36 @@ go_write:
 	up_read(&fi->i_sem);
 
 	if (need_cp) {
-		nid_t pino;
-
 		/* all the dirty node pages should be flushed for POR */
 		ret = f2fs_sync_fs(inode->i_sb, 1);
 
-		down_write(&fi->i_sem);
-		fi->xattr_ver = 0;
-		if (file_wrong_pino(inode) && inode->i_nlink == 1 &&
-					get_parent_ino(inode, &pino)) {
-			fi->i_pino = pino;
-			file_got_pino(inode);
-			up_write(&fi->i_sem);
-			mark_inode_dirty_sync(inode);
-			ret = f2fs_write_inode(inode, NULL);
-			if (ret)
-				goto out;
-		} else {
-			up_write(&fi->i_sem);
-		}
-	} else {
+		/*
+		 * We've secured consistency through sync_fs. Following pino
+		 * will be used only for fsynced inodes after checkpoint.
+		 */
+		try_to_fix_pino(inode);
+		goto out;
+	}
 sync_nodes:
-		sync_node_pages(sbi, ino, &wbc);
-
-		if (need_inode_block_update(sbi, ino)) {
-			mark_inode_dirty_sync(inode);
-			ret = f2fs_write_inode(inode, NULL);
-			if (ret)
-				goto out;
-			goto sync_nodes;
-		}
+	sync_node_pages(sbi, ino, &wbc);
 
-		ret = wait_on_node_pages_writeback(sbi, ino);
-		if (ret)
-			goto out;
+	if (need_inode_block_update(sbi, ino)) {
+		mark_inode_dirty_sync(inode);
+		f2fs_write_inode(inode, NULL);
+		goto sync_nodes;
+	}
 
-		/* once recovery info is written, don't need to tack this */
-		remove_dirty_inode(sbi, ino, APPEND_INO);
-		clear_inode_flag(fi, FI_APPEND_WRITE);
+	ret = wait_on_node_pages_writeback(sbi, ino);
+	if (ret)
+		goto out;
+
+	/* once recovery info is written, don't need to tack this */
+	remove_dirty_inode(sbi, ino, APPEND_INO);
+	clear_inode_flag(fi, FI_APPEND_WRITE);
 flush_out:
-		remove_dirty_inode(sbi, ino, UPDATE_INO);
-		clear_inode_flag(fi, FI_UPDATE_WRITE);
-		ret = f2fs_issue_flush(sbi);
-	}
+	remove_dirty_inode(sbi, ino, UPDATE_INO);
+	clear_inode_flag(fi, FI_UPDATE_WRITE);
+	ret = f2fs_issue_flush(sbi);
 out:
 	trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret);
 	return ret;
-- 
1.9.1

[PATCH 3/3] f2fs: clear track info related to write mode all the time

[Changman Lee]

In case of checkpoint is called at fsync, we should clear track info.
Otherwise, remained track info may influence to inode flag next
eviction.

Signed-off-by: Changman Lee <cm224.lee@samsung.com>
---
 fs/f2fs/file.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 3c27e0e..3860fb8 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -177,6 +177,7 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	nid_t ino = inode->i_ino;
 	int ret = 0;
 	bool need_cp = false;
+	bool flush = true;
 	struct writeback_control wbc = {
 		.sync_mode = WB_SYNC_ALL,
 		.nr_to_write = LONG_MAX,
@@ -238,10 +239,11 @@ go_write:
 
 		/*
 		 * We've secured consistency through sync_fs. Following pino
-		 * will be used only for fsynced inodes after checkpoint.
+		 * will be used only for inodes to be fsynced after checkpoint.
 		 */
 		try_to_fix_pino(inode);
-		goto out;
+		flush = false;
+		goto flush_cleanup;
 	}
 sync_nodes:
 	sync_node_pages(sbi, ino, &wbc);
@@ -256,13 +258,15 @@ sync_nodes:
 	if (ret)
 		goto out;
 
-	/* once recovery info is written, don't need to tack this */
+	/* once recovery info is written, don't need to track this */
+flush_cleanup:
 	remove_dirty_inode(sbi, ino, APPEND_INO);
 	clear_inode_flag(fi, FI_APPEND_WRITE);
 flush_out:
 	remove_dirty_inode(sbi, ino, UPDATE_INO);
 	clear_inode_flag(fi, FI_UPDATE_WRITE);
-	ret = f2fs_issue_flush(sbi);
+	if (flush)
+		ret = f2fs_issue_flush(sbi);
 out:
 	trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret);
 	return ret;
-- 
1.9.1

Re: [f2fs-dev] [PATCH 3/3] f2fs: clear track info related to write mode all the time

[Jaegeuk Kim]

Hi,

On Mon, Dec 08, 2014 at 03:29:42PM +0900, Changman Lee wrote:
> In case of checkpoint is called at fsync, we should clear track info.
> Otherwise, remained track info may influence to inode flag next
> eviction.

In do_checkpoint, release_dirty_inode() already removed UPDATE|APPEND_INO.
So, let's just drop the inode flags.

Thanks,

> 
> Signed-off-by: Changman Lee <cm224.lee@samsung.com>
> ---
>  fs/f2fs/file.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 3c27e0e..3860fb8 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -177,6 +177,7 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
>  	nid_t ino = inode->i_ino;
>  	int ret = 0;
>  	bool need_cp = false;
> +	bool flush = true;
>  	struct writeback_control wbc = {
>  		.sync_mode = WB_SYNC_ALL,
>  		.nr_to_write = LONG_MAX,
> @@ -238,10 +239,11 @@ go_write:
>  
>  		/*
>  		 * We've secured consistency through sync_fs. Following pino
> -		 * will be used only for fsynced inodes after checkpoint.
> +		 * will be used only for inodes to be fsynced after checkpoint.
>  		 */
>  		try_to_fix_pino(inode);
> -		goto out;
> +		flush = false;
> +		goto flush_cleanup;
>  	}
>  sync_nodes:
>  	sync_node_pages(sbi, ino, &wbc);
> @@ -256,13 +258,15 @@ sync_nodes:
>  	if (ret)
>  		goto out;
>  
> -	/* once recovery info is written, don't need to tack this */
> +	/* once recovery info is written, don't need to track this */
> +flush_cleanup:
>  	remove_dirty_inode(sbi, ino, APPEND_INO);
>  	clear_inode_flag(fi, FI_APPEND_WRITE);
>  flush_out:
>  	remove_dirty_inode(sbi, ino, UPDATE_INO);
>  	clear_inode_flag(fi, FI_UPDATE_WRITE);
> -	ret = f2fs_issue_flush(sbi);
> +	if (flush)
> +		ret = f2fs_issue_flush(sbi);
>  out:
>  	trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret);
>  	return ret;
> -- 
> 1.9.1
> 
> 
> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel