[f2fs-dev] [PATCH v2 0/2] add a new fio cache for IPU.

[f2fs-dev] [PATCH v2 0/2] add a new fio cache for IPU.

[Lihong Kou]

Introdce a new fio cache for IPU.
After commit 8648de2c581(f2fs: add bio cache for IPU)
in the mainline, we still have the problem in SQLite. So I
reuse the fio data structs to cache the mergeale write IO in
wirtepages().

Lihong Kou (1):
  Revert "f2fs: add bio cache for IPU"

Lihong Kou (1):
  f2fs: introduce a new fio cache for IPU.

 fs/f2fs/checkpoint.c |  1 +
 fs/f2fs/data.c       | 93 ++++++++--------------------------------------------
 fs/f2fs/f2fs.h       |  4 +--
 fs/f2fs/segment.c    | 22 ++++++-------
 fs/f2fs/super.c      |  2 ++
 5 files changed, 29 insertions(+), 93 deletions(-)

-- 
2.7.4



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2 1/2] Revert "f2fs: add bio cache for IPU"

[Lihong Kou]

This reverts commit 8648de2c581eeda7e412d6e38bf19e25bbb795ba.
---
 fs/f2fs/data.c    | 88 ++++++-------------------------------------------------
 fs/f2fs/f2fs.h    |  3 --
 fs/f2fs/segment.c |  5 +---
 3 files changed, 10 insertions(+), 86 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 0ca530a..5af9810 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -348,20 +348,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
 	io->bio = NULL;
 }
 
-static bool __has_merged_page(struct bio *bio, struct inode *inode,
+static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
 						struct page *page, nid_t ino)
 {
 	struct bio_vec *bvec;
 	struct page *target;
 	struct bvec_iter_all iter_all;
 
-	if (!bio)
+	if (!io->bio)
 		return false;
 
 	if (!inode && !page && !ino)
 		return true;
 
-	bio_for_each_segment_all(bvec, bio, iter_all) {
+	bio_for_each_segment_all(bvec, io->bio, iter_all) {
 
 		target = bvec->bv_page;
 		if (fscrypt_is_bounce_page(target))
@@ -411,7 +411,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
 			struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
 
 			down_read(&io->io_rwsem);
-			ret = __has_merged_page(io->bio, inode, page, ino);
+			ret = __has_merged_page(io, inode, page, ino);
 			up_read(&io->io_rwsem);
 		}
 		if (ret)
@@ -481,61 +481,6 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
 	return 0;
 }
 
-int f2fs_merge_page_bio(struct f2fs_io_info *fio)
-{
-	struct bio *bio = *fio->bio;
-	struct page *page = fio->encrypted_page ?
-			fio->encrypted_page : fio->page;
-
-	if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
-			__is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
-		return -EFSCORRUPTED;
-
-	trace_f2fs_submit_page_bio(page, fio);
-	f2fs_trace_ios(fio, 0);
-
-	if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
-			!__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
-		__submit_bio(fio->sbi, bio, fio->type);
-		bio = NULL;
-	}
-alloc_new:
-	if (!bio) {
-		bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
-				BIO_MAX_PAGES, false, fio->type, fio->temp);
-		bio_set_op_attrs(bio, fio->op, fio->op_flags);
-	}
-
-	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
-		__submit_bio(fio->sbi, bio, fio->type);
-		bio = NULL;
-		goto alloc_new;
-	}
-
-	if (fio->io_wbc)
-		wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
-
-	inc_page_count(fio->sbi, WB_DATA_TYPE(page));
-
-	*fio->last_block = fio->new_blkaddr;
-	*fio->bio = bio;
-
-	return 0;
-}
-
-static void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
-							struct page *page)
-{
-	if (!bio)
-		return;
-
-	if (!__has_merged_page(*bio, NULL, page, 0))
-		return;
-
-	__submit_bio(sbi, *bio, DATA);
-	*bio = NULL;
-}
-
 void f2fs_submit_page_write(struct f2fs_io_info *fio)
 {
 	struct f2fs_sb_info *sbi = fio->sbi;
@@ -2004,8 +1949,6 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
 }
 
 static int __write_data_page(struct page *page, bool *submitted,
-				struct bio **bio,
-				sector_t *last_block,
 				struct writeback_control *wbc,
 				enum iostat_type io_type)
 {
@@ -2031,8 +1974,6 @@ static int __write_data_page(struct page *page, bool *submitted,
 		.need_lock = LOCK_RETRY,
 		.io_type = io_type,
 		.io_wbc = wbc,
-		.bio = bio,
-		.last_block = last_block,
 	};
 
 	trace_f2fs_writepage(page, DATA);
@@ -2131,13 +2072,10 @@ static int __write_data_page(struct page *page, bool *submitted,
 
 	unlock_page(page);
 	if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode) &&
-					!F2FS_I(inode)->cp_task) {
-		f2fs_submit_ipu_bio(sbi, bio, page);
+					!F2FS_I(inode)->cp_task)
 		f2fs_balance_fs(sbi, need_balance_fs);
-	}
 
 	if (unlikely(f2fs_cp_error(sbi))) {
-		f2fs_submit_ipu_bio(sbi, bio, page);
 		f2fs_submit_merged_write(sbi, DATA);
 		submitted = NULL;
 	}
@@ -2164,7 +2102,7 @@ static int __write_data_page(struct page *page, bool *submitted,
 static int f2fs_write_data_page(struct page *page,
 					struct writeback_control *wbc)
 {
-	return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
+	return __write_data_page(page, NULL, wbc, FS_DATA_IO);
 }
 
 /*
@@ -2180,8 +2118,6 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 	int done = 0;
 	struct pagevec pvec;
 	struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
-	struct bio *bio = NULL;
-	sector_t last_block;
 	int nr_pages;
 	pgoff_t uninitialized_var(writeback_index);
 	pgoff_t index;
@@ -2258,20 +2194,17 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 			}
 
 			if (PageWriteback(page)) {
-				if (wbc->sync_mode != WB_SYNC_NONE) {
+				if (wbc->sync_mode != WB_SYNC_NONE)
 					f2fs_wait_on_page_writeback(page,
 							DATA, true, true);
-					f2fs_submit_ipu_bio(sbi, &bio, page);
-				} else {
+				else
 					goto continue_unlock;
-				}
 			}
 
 			if (!clear_page_dirty_for_io(page))
 				goto continue_unlock;
 
-			ret = __write_data_page(page, &submitted, &bio,
-					&last_block, wbc, io_type);
+			ret = __write_data_page(page, &submitted, wbc, io_type);
 			if (unlikely(ret)) {
 				/*
 				 * keep nr_to_write, since vfs uses this to
@@ -2320,9 +2253,6 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 	if (nwritten)
 		f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
 								NULL, 0, DATA);
-	/* submit cached bio of IPU write */
-	if (bio)
-		__submit_bio(sbi, bio, DATA);
 
 	return ret;
 }
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 17382da..f1cf2843 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1056,8 +1056,6 @@ struct f2fs_io_info {
 	bool retry;		/* need to reallocate block address */
 	enum iostat_type io_type;	/* io type */
 	struct writeback_control *io_wbc; /* writeback control */
-	struct bio **bio;		/* bio for ipu */
-	sector_t *last_block;		/* last block number in bio */
 	unsigned char version;		/* version of the node */
 };
 
@@ -3164,7 +3162,6 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
 				nid_t ino, enum page_type type);
 void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
 int f2fs_submit_page_bio(struct f2fs_io_info *fio);
-int f2fs_merge_page_bio(struct f2fs_io_info *fio);
 void f2fs_submit_page_write(struct f2fs_io_info *fio);
 struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
 			block_t blk_addr, struct bio *bio);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a661ac3..380e0ab 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3253,10 +3253,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
 
 	stat_inc_inplace_blocks(fio->sbi);
 
-	if (fio->bio)
-		err = f2fs_merge_page_bio(fio);
-	else
-		err = f2fs_submit_page_bio(fio);
+	err = f2fs_submit_page_bio(fio);
 	if (!err) {
 		update_device_state(fio);
 		f2fs_update_iostat(fio->sbi, fio->io_type, F2FS_BLKSIZE);
-- 
2.7.4



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2 2/2] f2fs: introduce a new fio cache for IPU.

[Lihong Kou]

From: koulihong <koulihong@huawei.com>

Description: introduce a new fio cache for IPU sequential
writes. After commit 8648de2c581(f2fs: add bio cache for IPU)
in the mainline, we still have the problem in SQLite. After
discuss the issue with chao, we know there are still some bugs
in temporary bio cache for IPU. This patch introdce a new fio cache
to fix this bug.

Test case:
fio -filename=/data/1.txt -direct=0 -ioengine=psync -fdatasync=1 -rw=write -bs=256K -size=256M -numjobs=1 -group_reporting -name=test1

Before:
             fio-2635  [015] ....  2017.735514: block_rq_issue: 7,0 WS 4096 () 548888 + 8 [fio]
             fio-2635  [015] ....  2017.735515: block_rq_issue: 7,0 WS 4096 () 548896 + 8 [fio]
             fio-2635  [015] ....  2017.735516: block_rq_issue: 7,0 WS 4096 () 548904 + 8 [fio]
             fio-2635  [015] ....  2017.735516: block_rq_issue: 7,0 WS 4096 () 548912 + 8 [fio]
             fio-2635  [015] ....  2017.735517: block_rq_issue: 7,0 WS 4096 () 548920 + 8 [fio]
             fio-2635  [015] ....  2017.735517: block_rq_issue: 7,0 WS 4096 () 548928 + 8 [fio]
             fio-2635  [015] ....  2017.735518: block_rq_issue: 7,0 WS 4096 () 548936 + 8 [fio]
             fio-2635  [015] ....  2017.735518: block_rq_issue: 7,0 WS 4096 () 548944 + 8 [fio]
             fio-2635  [015] ....  2017.735519: block_rq_issue: 7,0 WS 4096 () 548952 + 8 [fio]
             fio-2635  [015] ....  2017.735519: block_rq_issue: 7,0 WS 4096 () 548960 + 8 [fio]
             fio-2635  [015] ....  2017.735520: block_rq_issue: 7,0 WS 4096 () 548968 + 8 [fio]
             fio-2635  [015] ....  2017.735520: block_rq_issue: 7,0 WS 4096 () 548976 + 8 [fio]
             fio-2635  [015] ....  2017.735521: block_rq_issue: 7,0 WS 4096 () 548984 + 8 [fio]
After:
             fio-14692 [003] ....  1473.946721: block_rq_issue: 7,0 WS 262144 () 195072 + 512 [fio]
             fio-14692 [003] ....  1473.947722: block_rq_issue: 7,0 WS 262144 () 195584 + 512 [fio]
             fio-14692 [003] ....  1473.948716: block_rq_issue: 7,0 WS 262144 () 196096 + 512 [fio]
             fio-14692 [003] ....  1473.949711: block_rq_issue: 7,0 WS 262144 () 196608 + 512 [fio]
             fio-14692 [003] ....  1473.950704: block_rq_issue: 7,0 WS 262144 () 197120 + 512 [fio]
             fio-14692 [003] ....  1473.951700: block_rq_issue: 7,0 WS 262144 () 197632 + 512 [fio]
             fio-14692 [003] ....  1473.952687: block_rq_issue: 7,0 WS 262144 () 198144 + 512 [fio]
             fio-14692 [003] ....  1473.953684: block_rq_issue: 7,0 WS 262144 () 198656 + 512 [fio]
             fio-14692 [003] ....  1473.954683: block_rq_issue: 7,0 WS 262144 () 199168 + 512 [fio]
             fio-14692 [003] ....  1473.955686: block_rq_issue: 7,0 WS 262144 () 199680 + 512 [fio]
             fio-14692 [003] ....  1473.956732: block_rq_issue: 7,0 WS 262144 () 200192 + 512 [fio]
             fio-14692 [003] ....  1473.957749: block_rq_issue: 7,0 WS 262144 () 200704 + 512 [fio]
             fio-14692 [003] ....  1473.958741: block_rq_issue: 7,0 WS 262144 () 201216 + 512 [fio]

Signed-off-by: koulihong <koulihong@huawei.com>
---
 fs/f2fs/checkpoint.c |  1 +
 fs/f2fs/data.c       |  5 +++++
 fs/f2fs/f2fs.h       |  1 +
 fs/f2fs/segment.c    | 19 +++++++++++--------
 fs/f2fs/super.c      |  2 ++
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index a0eef95..78d5db0 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1081,6 +1081,7 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type)
 		 * wribacking dentry pages in the freeing inode.
 		 */
 		f2fs_submit_merged_write(sbi, DATA);
+		f2fs_submit_merged_write(sbi, DATA_IPU);
 		cond_resched();
 	}
 	goto retry;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5af9810..16fdab2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -440,6 +440,7 @@ void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi)
 	f2fs_submit_merged_write(sbi, DATA);
 	f2fs_submit_merged_write(sbi, NODE);
 	f2fs_submit_merged_write(sbi, META);
+	f2fs_submit_merged_write(sbi, DATA_IPU);
 }
 
 /*
@@ -2065,6 +2066,7 @@ static int __write_data_page(struct page *page, bool *submitted,
 
 	if (wbc->for_reclaim) {
 		f2fs_submit_merged_write_cond(sbi, NULL, page, 0, DATA);
+		f2fs_submit_merged_write_cond(sbi, NULL, page, 0, DATA_IPU);
 		clear_inode_flag(inode, FI_HOT_DATA);
 		f2fs_remove_dirty_inode(inode);
 		submitted = NULL;
@@ -2077,6 +2079,7 @@ static int __write_data_page(struct page *page, bool *submitted,
 
 	if (unlikely(f2fs_cp_error(sbi))) {
 		f2fs_submit_merged_write(sbi, DATA);
+		f2fs_submit_merged_write(sbi, DATA_IPU);
 		submitted = NULL;
 	}
 
@@ -2253,6 +2256,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
 	if (nwritten)
 		f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
 								NULL, 0, DATA);
+		f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
+								NULL, 0, DATA_IPU);
 
 	return ret;
 }
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index f1cf2843..79bf996 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -983,6 +983,7 @@ enum count_type {
 enum page_type {
 	DATA,
 	NODE,
+	DATA_IPU,
 	META,
 	NR_PAGE_TYPE,
 	META_FLUSH,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 380e0ab..9c67b55 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -430,8 +430,10 @@ static int __f2fs_commit_inmem_pages(struct inode *inode)
 		list_move_tail(&cur->list, &revoke_list);
 	}
 
-	if (submit_bio)
+	if (submit_bio) {
 		f2fs_submit_merged_write_cond(sbi, inode, NULL, 0, DATA);
+		f2fs_submit_merged_write_cond(sbi, inode, NULL, 0, DATA_IPU);
+	}
 
 	if (err) {
 		/*
@@ -3234,7 +3236,6 @@ void f2fs_outplace_write_data(struct dnode_of_data *dn,
 
 int f2fs_inplace_write_data(struct f2fs_io_info *fio)
 {
-	int err;
 	struct f2fs_sb_info *sbi = fio->sbi;
 	unsigned int segno;
 
@@ -3253,13 +3254,12 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
 
 	stat_inc_inplace_blocks(fio->sbi);
 
-	err = f2fs_submit_page_bio(fio);
-	if (!err) {
-		update_device_state(fio);
-		f2fs_update_iostat(fio->sbi, fio->io_type, F2FS_BLKSIZE);
-	}
+	fio->type = DATA_IPU;
+	f2fs_submit_page_write(fio);
+	update_device_state(fio);
+	f2fs_update_iostat(fio->sbi, fio->io_type, F2FS_BLKSIZE);
 
-	return err;
+	return 0;
 }
 
 static inline int __f2fs_get_curseg(struct f2fs_sb_info *sbi,
@@ -3375,6 +3375,9 @@ void f2fs_wait_on_page_writeback(struct page *page,
 		struct f2fs_sb_info *sbi = F2FS_P_SB(page);
 
 		f2fs_submit_merged_write_cond(sbi, NULL, page, 0, type);
+		if (type == DATA)
+			f2fs_submit_merged_write_cond(sbi, NULL, page, 0, DATA_IPU);
+
 		if (ordered) {
 			wait_on_page_writeback(page);
 			f2fs_bug_on(sbi, locked && PageWriteback(page));
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d95a681..62e1799 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -899,6 +899,8 @@ static int f2fs_drop_inode(struct inode *inode)
 
 			f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
 					inode, NULL, 0, DATA);
+			f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
+					inode, NULL, 0, DATA_IPU);
 			truncate_inode_pages_final(inode->i_mapping);
 
 			if (F2FS_HAS_BLOCKS(inode))
-- 
2.7.4



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2 0/2] add a new fio cache for IPU.

[Chao Yu]

Hi Lihong,

On 2019/10/16 18:11, Lihong Kou wrote:
> Introdce a new fio cache for IPU.
> After commit 8648de2c581(f2fs: add bio cache for IPU)
> in the mainline, we still have the problem in SQLite. So I
> reuse the fio data structs to cache the mergeale write IO in
> wirtepages().

I have discussed with Jaegeuk about this in below thread,

https://lore.kernel.org/linux-f2fs-devel/20190219081529.5106-1-yuchao0@huawei.com/T/#md5d6dcf60052201994e6df4a915a07a04869d427

IMO, we need a per data type global list to cache all bios to enhance its
scalability, as single bio cache may suffer performance regression due to race
of multiple threads, I've sent a patch [1] for that previously.

For any thoughts, we can discuss in above thread.

[1]
https://lore.kernel.org/linux-f2fs-devel/20190930105325.42870-1-yuchao0@huawei.com/T/#u

Thanks,

> 
> Lihong Kou (1):
>   Revert "f2fs: add bio cache for IPU"
> 
> Lihong Kou (1):
>   f2fs: introduce a new fio cache for IPU.
> 
>  fs/f2fs/checkpoint.c |  1 +
>  fs/f2fs/data.c       | 93 ++++++++--------------------------------------------
>  fs/f2fs/f2fs.h       |  4 +--
>  fs/f2fs/segment.c    | 22 ++++++-------
>  fs/f2fs/super.c      |  2 ++
>  5 files changed, 29 insertions(+), 93 deletions(-)
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel