[PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write

linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed

* [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write
@ 2018-04-23  2:36 Chao Yu
  2018-04-23  2:36 ` [PATCH 2/3] f2fs: clean up commit_inmem_pages() Chao Yu
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Chao Yu @ 2018-04-23  2:36 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

After revoking atomic write, related LBA can be reused by others, so we
need to wait page writeback before reusing the LBA, in order to avoid
interference between old atomic written in-flight IO and new IO.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index cba59fc58eb6..24299f81f80d 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -231,6 +231,8 @@ static int __revoke_inmem_pages(struct inode *inode,
 
 		lock_page(page);
 
+		f2fs_wait_on_page_writeback(page, DATA, true);
+
 		if (recover) {
 			struct dnode_of_data dn;
 			struct node_info ni;
-- 
2.15.0.55.gc2ece9dc4de6

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

* [PATCH 2/3] f2fs: clean up commit_inmem_pages()
  2018-04-23  2:36 [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
@ 2018-04-23  2:36 ` Chao Yu
  2018-04-23  2:36 ` [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages() Chao Yu
  2018-05-18  1:26 ` [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
  2 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2018-04-23  2:36 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

This patch moves error handling from commit_inmem_pages() into
__commit_inmem_page() for cleanup, no logic change.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 24299f81f80d..dcb4fcb8b1b0 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -347,8 +347,7 @@ void drop_inmem_page(struct inode *inode, struct page *page)
 	trace_f2fs_commit_inmem_page(page, INMEM_INVALIDATE);
 }
 
-static int __commit_inmem_pages(struct inode *inode,
-					struct list_head *revoke_list)
+static int __commit_inmem_pages(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct f2fs_inode_info *fi = F2FS_I(inode);
@@ -361,9 +360,12 @@ static int __commit_inmem_pages(struct inode *inode,
 		.op_flags = REQ_SYNC | REQ_PRIO,
 		.io_type = FS_DATA_IO,
 	};
+	struct list_head revoke_list;
 	pgoff_t last_idx = ULONG_MAX;
 	int err = 0;
 
+	INIT_LIST_HEAD(&revoke_list);
+
 	list_for_each_entry_safe(cur, tmp, &fi->inmem_pages, list) {
 		struct page *page = cur->page;
 
@@ -397,14 +399,28 @@ static int __commit_inmem_pages(struct inode *inode,
 			last_idx = page->index;
 		}
 		unlock_page(page);
-		list_move_tail(&cur->list, revoke_list);
+		list_move_tail(&cur->list, &revoke_list);
 	}
 
 	if (last_idx != ULONG_MAX)
 		f2fs_submit_merged_write_cond(sbi, inode, 0, last_idx, DATA);
 
-	if (!err)
-		__revoke_inmem_pages(inode, revoke_list, false, false);
+	if (err) {
+		/*
+		 * try to revoke all committed pages, but still we could fail
+		 * due to no memory or other reason, if that happened, EAGAIN
+		 * will be returned, which means in such case, transaction is
+		 * already not integrity, caller should use journal to do the
+		 * recovery or rewrite & commit last transaction. For other
+		 * error number, revoking was done by filesystem itself.
+		 */
+		err = __revoke_inmem_pages(inode, &revoke_list, false, true);
+
+		/* drop all uncommitted pages */
+		__revoke_inmem_pages(inode, &fi->inmem_pages, true, false);
+	} else {
+		__revoke_inmem_pages(inode, &revoke_list, false, false);
+	}
 
 	return err;
 }
@@ -413,34 +429,16 @@ int commit_inmem_pages(struct inode *inode)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct f2fs_inode_info *fi = F2FS_I(inode);
-	struct list_head revoke_list;
 	int err;
 
-	INIT_LIST_HEAD(&revoke_list);
 	f2fs_balance_fs(sbi, true);
 	f2fs_lock_op(sbi);
 
 	set_inode_flag(inode, FI_ATOMIC_COMMIT);
 
 	mutex_lock(&fi->inmem_lock);
-	err = __commit_inmem_pages(inode, &revoke_list);
-	if (err) {
-		int ret;
-		/*
-		 * try to revoke all committed pages, but still we could fail
-		 * due to no memory or other reason, if that happened, EAGAIN
-		 * will be returned, which means in such case, transaction is
-		 * already not integrity, caller should use journal to do the
-		 * recovery or rewrite & commit last transaction. For other
-		 * error number, revoking was done by filesystem itself.
-		 */
-		ret = __revoke_inmem_pages(inode, &revoke_list, false, true);
-		if (ret)
-			err = ret;
+	err = __commit_inmem_pages(inode);
 
-		/* drop all uncommitted pages */
-		__revoke_inmem_pages(inode, &fi->inmem_pages, true, false);
-	}
 	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
 	if (!list_empty(&fi->inmem_ilist))
 		list_del_init(&fi->inmem_ilist);
-- 
2.15.0.55.gc2ece9dc4de6


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

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

* [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages()
  2018-04-23  2:36 [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
  2018-04-23  2:36 ` [PATCH 2/3] f2fs: clean up commit_inmem_pages() Chao Yu
@ 2018-04-23  2:36 ` Chao Yu
  2018-04-23 15:01   ` Jaegeuk Kim
  2018-05-18  1:26 ` [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
  2 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2018-04-23  2:36 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Thread A				Thread B
- f2fs_ioc_commit_atomic_write
 - commit_inmem_pages
  - f2fs_submit_merged_write_cond
  : write data
					- write_checkpoint
					 - do_checkpoint
					 : commit all node within CP
					 -> SPO
  - f2fs_do_sync_file
   - file_write_and_wait_range
   : wait data writeback

In above race condition, data/node can be flushed in reversed order when
coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
atomic written data being corrupted.

This patch adds filemap_fdatawait() in commit_inmem_pages() to keep data
and node of atomic file being flushed orderly.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/file.c    | 21 ++++++++++++---------
 fs/f2fs/segment.c |  2 ++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 5d710f7f2684..4ea418cbd189 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -217,15 +217,18 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
 
 	trace_f2fs_sync_file_enter(inode);
 
-	/* if fdatasync is triggered, let's do in-place-update */
-	if (datasync || get_dirty_pages(inode) <= SM_I(sbi)->min_fsync_blocks)
-		set_inode_flag(inode, FI_NEED_IPU);
-	ret = file_write_and_wait_range(file, start, end);
-	clear_inode_flag(inode, FI_NEED_IPU);
-
-	if (ret) {
-		trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret);
-		return ret;
+	if (!atomic) {
+		/* if fdatasync is triggered, let's do in-place-update */
+		if (datasync || get_dirty_pages(inode) <=
+					SM_I(sbi)->min_fsync_blocks)
+			set_inode_flag(inode, FI_NEED_IPU);
+		ret = file_write_and_wait_range(file, start, end);
+		clear_inode_flag(inode, FI_NEED_IPU);
+
+		if (ret) {
+			trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret);
+			return ret;
+		}
 	}
 
 	/* if the inode is dirty, let's recover all the time */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index dcb4fcb8b1b0..3b66c4aa04a6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -420,6 +420,8 @@ static int __commit_inmem_pages(struct inode *inode)
 		__revoke_inmem_pages(inode, &fi->inmem_pages, true, false);
 	} else {
 		__revoke_inmem_pages(inode, &revoke_list, false, false);
+
+		err = filemap_fdatawait(inode->i_mapping);
 	}
 
 	return err;
-- 
2.15.0.55.gc2ece9dc4de6

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

* Re: [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages()
  2018-04-23  2:36 ` [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages() Chao Yu
@ 2018-04-23 15:01   ` Jaegeuk Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Jaegeuk Kim @ 2018-04-23 15:01 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 04/23, Chao Yu wrote:
> Thread A				Thread B
> - f2fs_ioc_commit_atomic_write
>  - commit_inmem_pages
>   - f2fs_submit_merged_write_cond
>   : write data
> 					- write_checkpoint
> 					 - do_checkpoint
> 					 : commit all node within CP
> 					 -> SPO
>   - f2fs_do_sync_file
>    - file_write_and_wait_range
>    : wait data writeback
> 
> In above race condition, data/node can be flushed in reversed order when
> coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
> atomic written data being corrupted.
> 
> This patch adds filemap_fdatawait() in commit_inmem_pages() to keep data
> and node of atomic file being flushed orderly.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/file.c    | 21 ++++++++++++---------
>  fs/f2fs/segment.c |  2 ++
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 5d710f7f2684..4ea418cbd189 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -217,15 +217,18 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>  
>  	trace_f2fs_sync_file_enter(inode);

if (atomic)
	goto write_done;

>  
> -	/* if fdatasync is triggered, let's do in-place-update */
> -	if (datasync || get_dirty_pages(inode) <= SM_I(sbi)->min_fsync_blocks)
> -		set_inode_flag(inode, FI_NEED_IPU);
> -	ret = file_write_and_wait_range(file, start, end);
> -	clear_inode_flag(inode, FI_NEED_IPU);
> -
> -	if (ret) {
> -		trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret);
> -		return ret;
> +	if (!atomic) {
> +		/* if fdatasync is triggered, let's do in-place-update */
> +		if (datasync || get_dirty_pages(inode) <=
> +					SM_I(sbi)->min_fsync_blocks)
> +			set_inode_flag(inode, FI_NEED_IPU);
> +		ret = file_write_and_wait_range(file, start, end);
> +		clear_inode_flag(inode, FI_NEED_IPU);
> +
> +		if (ret) {
> +			trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret);
> +			return ret;
> +		}
>  	}

write_done:

>  
>  	/* if the inode is dirty, let's recover all the time */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index dcb4fcb8b1b0..3b66c4aa04a6 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -420,6 +420,8 @@ static int __commit_inmem_pages(struct inode *inode)
>  		__revoke_inmem_pages(inode, &fi->inmem_pages, true, false);
>  	} else {
>  		__revoke_inmem_pages(inode, &revoke_list, false, false);
> +
> +		err = filemap_fdatawait(inode->i_mapping);
>  	}
>  
>  	return err;
> -- 
> 2.15.0.55.gc2ece9dc4de6

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

* Re: [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write
  2018-04-23  2:36 [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
  2018-04-23  2:36 ` [PATCH 2/3] f2fs: clean up commit_inmem_pages() Chao Yu
  2018-04-23  2:36 ` [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages() Chao Yu
@ 2018-05-18  1:26 ` Chao Yu
  2 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2018-05-18  1:26 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao

Hi Jaegeuk,

Could you recheck this patch?

On 2018/4/23 10:36, Chao Yu wrote:
> After revoking atomic write, related LBA can be reused by others, so we
> need to wait page writeback before reusing the LBA, in order to avoid
> interference between old atomic written in-flight IO and new IO.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/segment.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index cba59fc58eb6..24299f81f80d 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -231,6 +231,8 @@ static int __revoke_inmem_pages(struct inode *inode,
>  
>  		lock_page(page);
>  
> +		f2fs_wait_on_page_writeback(page, DATA, true);
> +
>  		if (recover) {
>  			struct dnode_of_data dn;
>  			struct node_info ni;
> 

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

end of thread, other threads:[~2018-05-18  1:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-23  2:36 [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu
2018-04-23  2:36 ` [PATCH 2/3] f2fs: clean up commit_inmem_pages() Chao Yu
2018-04-23  2:36 ` [PATCH 3/3] f2fs: wait IO writeback in commit_inmem_pages() Chao Yu
2018-04-23 15:01   ` Jaegeuk Kim
2018-05-18  1:26 ` [PATCH 1/3] f2fs: fix to wait page writeback during revoking atomic write Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).