[PATCH 1/3] f2fs: reconstruct code to write a data page

[PATCH 1/3] f2fs: reconstruct code to write a data page

[Hou Pengyang]

This patch introduces two functions:

	1) f2fs_encrypt_page -- encrypt data page before submit_bio
	2) do_rewrite_data_page -- IPU rewrite

The two functions are extraced from do_write_data_page, logic in
do_write_data_page stays unchanged.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/data.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b8dcd1e..4ba11a6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1302,26 +1302,11 @@ static int f2fs_read_data_pages(struct file *file,
 	return f2fs_mpage_readpages(mapping, pages, NULL, nr_pages);
 }
 
-int do_write_data_page(struct f2fs_io_info *fio)
+static int f2fs_encrypt_page(struct f2fs_io_info *fio, struct page *page)
 {
-	struct page *page = fio->page;
 	struct inode *inode = page->mapping->host;
-	struct dnode_of_data dn;
 	int err = 0;
 
-	set_new_dnode(&dn, inode, NULL, NULL, 0);
-	err = get_dnode_of_data(&dn, page->index, LOOKUP_NODE);
-	if (err)
-		return err;
-
-	fio->old_blkaddr = dn.data_blkaddr;
-
-	/* This page is already truncated */
-	if (fio->old_blkaddr == NULL_ADDR) {
-		ClearPageUptodate(page);
-		goto out_writepage;
-	}
-
 	if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) {
 		gfp_t gfp_flags = GFP_NOFS;
 
@@ -1343,9 +1328,50 @@ int do_write_data_page(struct f2fs_io_info *fio)
 				err = 0;
 				goto retry_encrypt;
 			}
-			goto out_writepage;
+			return err;
 		}
 	}
+	return err;
+}
+
+static int do_rewrite_data_page(struct f2fs_io_info *fio, struct page *page)
+{
+	struct inode *inode = page->mapping->host;
+	int err;
+
+	f2fs_bug_on(F2FS_P_SB(page), !fio->cp_rwsem_locked);
+
+	f2fs_unlock_op(F2FS_I_SB(inode));
+	fio->cp_rwsem_locked = false;
+	err = rewrite_data_page(fio);
+	set_inode_flag(inode, FI_UPDATE_WRITE);
+	trace_f2fs_do_write_data_page(page, IPU);
+	return err;
+
+}
+
+int do_write_data_page(struct f2fs_io_info *fio)
+{
+	struct page *page = fio->page;
+	struct inode *inode = page->mapping->host;
+	struct dnode_of_data dn;
+	int err = 0;
+
+	set_new_dnode(&dn, inode, NULL, NULL, 0);
+	err = get_dnode_of_data(&dn, page->index, LOOKUP_NODE);
+	if (err)
+		return err;
+
+	fio->old_blkaddr = dn.data_blkaddr;
+
+	/* This page is already truncated */
+	if (fio->old_blkaddr == NULL_ADDR) {
+		ClearPageUptodate(page);
+		goto out_writepage;
+	}
+
+	if (f2fs_encrypt_page(fio, page))
+		goto out_writepage;
 
 	set_page_writeback(page);
 
@@ -1357,11 +1383,7 @@ int do_write_data_page(struct f2fs_io_info *fio)
 			!is_cold_data(page) &&
 			!IS_ATOMIC_WRITTEN_PAGE(page) &&
 			need_inplace_update(inode, fio))) {
-		f2fs_unlock_op(F2FS_I_SB(inode));
-		fio->cp_rwsem_locked = false;
-		err = rewrite_data_page(fio);
-		set_inode_flag(inode, FI_UPDATE_WRITE);
-		trace_f2fs_do_write_data_page(page, IPU);
+		err = do_rewrite_data_page(fio, page);
 	} else {
 		write_data_page(&dn, fio);
 		trace_f2fs_do_write_data_page(page, OPU);
-- 
2.10.1


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

[PATCH 0/3] optimize f2fs IPU v2

[Hou Pengyang]

For IPU, there is no need to read dnode, these three patches are to fix this
issue.

patch 1): Reconstruct code in do_write_data_page, code logic stays unchanged,
patch 2): Under high-probability IPU scenario, we check extent_tree first
patch 3): Unlock dnode before IPU

v1->v2:
1) fix 
		if(f2fs_encrypt_page())
			XXX
	to 
		err = f2fs_encrypt_page() 
		if (err)
			XXX

	otherwise do_write_data_page would return a wrong return value.
		
2) extract need_in_place_block(fio, page) for IPU checking.

Hou Pengyang (3):
  f2fs: reconstruct code to write a data page
  f2fs: lookup extent cache first under IPU scenario
  f2fs: unlock dnode before IPU

 fs/f2fs/data.c | 109 ++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 84 insertions(+), 25 deletions(-)

-- 
2.10.1


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

[PATCH 1/3] f2fs: reconstruct code to write a data page

[Hou Pengyang]

This patch introduces two functions:

	1) f2fs_encrypt_page -- encrypt data page before submit_bio
	2) do_rewrite_data_page -- IPU rewrite

The two functions are extraced from do_write_data_page, logic in
do_write_data_page stays unchanged.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/data.c | 83 ++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 25 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b8dcd1e..6b18750 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1302,26 +1302,11 @@ static int f2fs_read_data_pages(struct file *file,
 	return f2fs_mpage_readpages(mapping, pages, NULL, nr_pages);
 }
 
-int do_write_data_page(struct f2fs_io_info *fio)
+static int f2fs_encrypt_page(struct f2fs_io_info *fio, struct page *page)
 {
-	struct page *page = fio->page;
 	struct inode *inode = page->mapping->host;
-	struct dnode_of_data dn;
 	int err = 0;
 
-	set_new_dnode(&dn, inode, NULL, NULL, 0);
-	err = get_dnode_of_data(&dn, page->index, LOOKUP_NODE);
-	if (err)
-		return err;
-
-	fio->old_blkaddr = dn.data_blkaddr;
-
-	/* This page is already truncated */
-	if (fio->old_blkaddr == NULL_ADDR) {
-		ClearPageUptodate(page);
-		goto out_writepage;
-	}
-
 	if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) {
 		gfp_t gfp_flags = GFP_NOFS;
 
@@ -1343,9 +1328,63 @@ int do_write_data_page(struct f2fs_io_info *fio)
 				err = 0;
 				goto retry_encrypt;
 			}
-			goto out_writepage;
+			return err;
 		}
 	}
+	return err;
+}
+
+static int do_rewrite_data_page(struct f2fs_io_info *fio, struct page *page)
+{
+	struct inode *inode = page->mapping->host;
+	int err;
+
+	f2fs_bug_on(F2FS_P_SB(page), !fio->cp_rwsem_locked);
+
+	f2fs_unlock_op(F2FS_I_SB(inode));
+	fio->cp_rwsem_locked = false;
+	err = rewrite_data_page(fio);
+	set_inode_flag(inode, FI_UPDATE_WRITE);
+	trace_f2fs_do_write_data_page(page, IPU);
+	return err;
+
+}
+
+static inline bool need_inplace_update_block(struct f2fs_io_info *fio, struct page *page)
+{
+	struct inode *inode = page->mapping->host;
+
+	if (!is_cold_data(page) &&
+			!IS_ATOMIC_WRITTEN_PAGE(page) &&
+			need_inplace_update(inode, fio))
+		return true;
+
+	return false;
+}
+
+int do_write_data_page(struct f2fs_io_info *fio)
+{
+	struct page *page = fio->page;
+	struct inode *inode = page->mapping->host;
+	struct dnode_of_data dn;
+	int err = 0;
+
+	set_new_dnode(&dn, inode, NULL, NULL, 0);
+	err = get_dnode_of_data(&dn, page->index, LOOKUP_NODE);
+	if (err)
+		return err;
+
+	fio->old_blkaddr = dn.data_blkaddr;
+
+	/* This page is already truncated */
+	if (fio->old_blkaddr == NULL_ADDR) {
+		ClearPageUptodate(page);
+		goto out_writepage;
+	}
+
+	err = f2fs_encrypt_page(fio, page);
+	if (err)
+		goto out_writepage;
 
 	set_page_writeback(page);
 
@@ -1354,14 +1393,8 @@ int do_write_data_page(struct f2fs_io_info *fio)
 	 * it had better in-place writes for updated data.
 	 */
 	if (unlikely(fio->old_blkaddr != NEW_ADDR &&
-			!is_cold_data(page) &&
-			!IS_ATOMIC_WRITTEN_PAGE(page) &&
-			need_inplace_update(inode, fio))) {
-		f2fs_unlock_op(F2FS_I_SB(inode));
-		fio->cp_rwsem_locked = false;
-		err = rewrite_data_page(fio);
-		set_inode_flag(inode, FI_UPDATE_WRITE);
-		trace_f2fs_do_write_data_page(page, IPU);
+			need_inplace_update_block(fio, page))) {
+		err = do_rewrite_data_page(fio, page);
 	} else {
 		write_data_page(&dn, fio);
 		trace_f2fs_do_write_data_page(page, OPU);
-- 
2.10.1


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

[PATCH 2/3] f2fs: lookup extent cache first under IPU scenario

[Hou Pengyang]

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/data.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 6b18750..1c99ae1 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1367,8 +1367,32 @@ int do_write_data_page(struct f2fs_io_info *fio)
 	struct page *page = fio->page;
 	struct inode *inode = page->mapping->host;
 	struct dnode_of_data dn;
+	struct extent_info ei = {0,0,0};
 	int err = 0;
 
+	/*
+	 * If a page is cold, NOT atomit written and need_ipu now, there is
+	 * a high probability that IPU should be adapted. For IPU, we try to
+	 * check extent tree to get the block index first, instead of reading
+	 * the dnode page, where may lead to an useless dnode IO, since no need to
+	 * update the dnode index for IPU.
+	 */
+	if (unlikely(need_inplace_update_block(fio, page) &&
+			f2fs_lookup_extent_cache(inode, page->index, &ei))) {
+
+		fio->old_blkaddr = ei.blk + page->index - ei.fofs;
+
+		if (fio->old_blkaddr != NEW_ADDR &&
+				fio->old_blkaddr != NULL_ADDR) {
+			err = f2fs_encrypt_page(fio, page);
+			if (err)
+				return err;
+			set_page_writeback(page);
+			err = do_rewrite_data_page(fio, page);
+			return err;
+		}
+	}
+
 	set_new_dnode(&dn, inode, NULL, NULL, 0);
 	err = get_dnode_of_data(&dn, page->index, LOOKUP_NODE);
 	if (err)
-- 
2.10.1


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

[PATCH 3/3] f2fs: unlock dnode before IPU

[Hou Pengyang]

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/data.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1c99ae1..a0df451 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1418,7 +1418,9 @@ int do_write_data_page(struct f2fs_io_info *fio)
 	 */
 	if (unlikely(fio->old_blkaddr != NEW_ADDR &&
 			need_inplace_update_block(fio, page))) {
+		f2fs_put_dnode(&dn);
 		err = do_rewrite_data_page(fio, page);
+		return err;
 	} else {
 		write_data_page(&dn, fio);
 		trace_f2fs_do_write_data_page(page, OPU);
-- 
2.10.1


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

Re: [PATCH 0/3] optimize f2fs IPU v2

[Chao Yu]

The patchset looks good to me.

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

On 2017/4/22 17:22, Hou Pengyang wrote:
> For IPU, there is no need to read dnode, these three patches are to fix this
> issue.
> 
> patch 1): Reconstruct code in do_write_data_page, code logic stays unchanged,
> patch 2): Under high-probability IPU scenario, we check extent_tree first
> patch 3): Unlock dnode before IPU
> 
> v1->v2:
> 1) fix 
> 		if(f2fs_encrypt_page())
> 			XXX
> 	to 
> 		err = f2fs_encrypt_page() 
> 		if (err)
> 			XXX
> 
> 	otherwise do_write_data_page would return a wrong return value.
> 		
> 2) extract need_in_place_block(fio, page) for IPU checking.
> 
> Hou Pengyang (3):
>   f2fs: reconstruct code to write a data page
>   f2fs: lookup extent cache first under IPU scenario
>   f2fs: unlock dnode before IPU
> 
>  fs/f2fs/data.c | 109 ++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 84 insertions(+), 25 deletions(-)
> 


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

Re: [PATCH 1/3] f2fs: reconstruct code to write a data page

[Jaegeuk Kim]

On 04/22, Hou Pengyang wrote:
> This patch introduces two functions:
> 
> 	1) f2fs_encrypt_page -- encrypt data page before submit_bio
> 	2) do_rewrite_data_page -- IPU rewrite

I feel this makes the codes more complicated.
How about this?

>From 939bfa8bdd9a75f4c83914ed84c50e3c35ac556d Mon Sep 17 00:00:00 2001
From: Hou Pengyang <houpengyang@huawei.com>
Date: Sat, 22 Apr 2017 09:22:21 +0000
Subject: [PATCH] f2fs: reconstruct code to write a data page

This patch introduces encrypt_one_page which encrypts one data page before
submit_bio, and change the use of need_inplace_update.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c    | 77 +++++++++++++++++++++++++++++++++----------------------
 fs/f2fs/file.c    |  2 +-
 fs/f2fs/segment.h |  2 +-
 3 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b8dcd1e224e8..02400b0c4993 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1302,6 +1302,45 @@ static int f2fs_read_data_pages(struct file *file,
 	return f2fs_mpage_readpages(mapping, pages, NULL, nr_pages);
 }
 
+static int encrypt_one_page(struct f2fs_io_info *fio)
+{
+	struct inode *inode = fio->page->mapping->host;
+	gfp_t gfp_flags = GFP_NOFS;
+
+	if (!f2fs_encrypted_inode(inode) || !S_ISREG(inode->i_mode))
+		return 0;
+
+	/* wait for GCed encrypted page writeback */
+	f2fs_wait_on_encrypted_page_writeback(fio->sbi, fio->old_blkaddr);
+
+retry_encrypt:
+	fio->encrypted_page = fscrypt_encrypt_page(inode, fio->page,
+			PAGE_SIZE, 0, fio->page->index, gfp_flags);
+	if (!IS_ERR(fio->encrypted_page))
+		return 0;
+
+	/* flush pending IOs and wait for a while in the ENOMEM case */
+	if (PTR_ERR(fio->encrypted_page) == -ENOMEM) {
+		f2fs_flush_merged_bios(fio->sbi);
+		congestion_wait(BLK_RW_ASYNC, HZ/50);
+		gfp_flags |= __GFP_NOFAIL;
+		goto retry_encrypt;
+	}
+	return PTR_ERR(fio->encrypted_page);
+}
+
+static inline bool need_inplace_update(struct f2fs_io_info *fio)
+{
+	if (fio->old_blkaddr == NEW_ADDR)
+		return false;
+	if (is_cold_data(fio->page))
+		return false;
+	if (IS_ATOMIC_WRITTEN_PAGE(fio->page))
+		return false;
+
+	return need_inplace_update_policy(fio->page->mapping->host, fio);
+}
+
 int do_write_data_page(struct f2fs_io_info *fio)
 {
 	struct page *page = fio->page;
@@ -1322,30 +1361,9 @@ int do_write_data_page(struct f2fs_io_info *fio)
 		goto out_writepage;
 	}
 
-	if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) {
-		gfp_t gfp_flags = GFP_NOFS;
-
-		/* wait for GCed encrypted page writeback */
-		f2fs_wait_on_encrypted_page_writeback(F2FS_I_SB(inode),
-							fio->old_blkaddr);
-retry_encrypt:
-		fio->encrypted_page = fscrypt_encrypt_page(inode, fio->page,
-							PAGE_SIZE, 0,
-							fio->page->index,
-							gfp_flags);
-		if (IS_ERR(fio->encrypted_page)) {
-			err = PTR_ERR(fio->encrypted_page);
-			if (err == -ENOMEM) {
-				/* flush pending ios and wait for a while */
-				f2fs_flush_merged_bios(F2FS_I_SB(inode));
-				congestion_wait(BLK_RW_ASYNC, HZ/50);
-				gfp_flags |= __GFP_NOFAIL;
-				err = 0;
-				goto retry_encrypt;
-			}
-			goto out_writepage;
-		}
-	}
+	err = encrypt_one_page(fio);
+	if (err)
+		goto out_writepage;
 
 	set_page_writeback(page);
 
@@ -1353,15 +1371,14 @@ int do_write_data_page(struct f2fs_io_info *fio)
 	 * If current allocation needs SSR,
 	 * it had better in-place writes for updated data.
 	 */
-	if (unlikely(fio->old_blkaddr != NEW_ADDR &&
-			!is_cold_data(page) &&
-			!IS_ATOMIC_WRITTEN_PAGE(page) &&
-			need_inplace_update(inode, fio))) {
-		f2fs_unlock_op(F2FS_I_SB(inode));
+	if (need_inplace_update(fio)) {
+		f2fs_bug_on(fio->sbi, !fio->cp_rwsem_locked);
+		f2fs_unlock_op(fio->sbi);
 		fio->cp_rwsem_locked = false;
+
 		err = rewrite_data_page(fio);
+		trace_f2fs_do_write_data_page(fio->page, IPU);
 		set_inode_flag(inode, FI_UPDATE_WRITE);
-		trace_f2fs_do_write_data_page(page, IPU);
 	} else {
 		write_data_page(&dn, fio);
 		trace_f2fs_do_write_data_page(page, OPU);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 32050f4c3592..68472902ba7d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1898,7 +1898,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
 	int err;
 
 	/* if in-place-update policy is enabled, don't waste time here */
-	if (need_inplace_update(inode, NULL))
+	if (need_inplace_update_policy(inode, NULL))
 		return -EINVAL;
 
 	pg_start = range->start >> PAGE_SHIFT;
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 8ad22b8cbba7..c51273c0c1ef 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -564,7 +564,7 @@ enum {
 	F2FS_IPU_ASYNC,
 };
 
-static inline bool need_inplace_update(struct inode *inode,
+static inline bool need_inplace_update_policy(struct inode *inode,
 				struct f2fs_io_info *fio)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-- 
2.11.0


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