[PATCH 1/3] f2fs: split wio_mutex

[PATCH 1/3] f2fs: split wio_mutex

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

Split wio_mutex to adjust different temperature bio cache.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    | 2 +-
 fs/f2fs/segment.c | 6 ++----
 fs/f2fs/super.c   | 4 ++--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4ab42749fd5f..500a4c21cfe8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -882,7 +882,7 @@ struct f2fs_sb_info {
 	/* for bio operations */
 	struct f2fs_bio_info meta_io[2];		/* for meta bios */
 	struct f2fs_bio_info log_io[NR_CURSEG_TYPE];	/* for log bios */
-	struct mutex wio_mutex[NODE + 1];	/* bio ordering for NODE/DATA */
+	struct mutex wio_mutex[NR_CURSEG_TYPE];	/* bio ordering for NODE/DATA */
 	int write_io_size_bits;			/* Write IO size bits */
 	mempool_t *write_io_dummy;		/* Dummy pages */
 
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index cdf7d61ac213..9fa6b0682d94 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2145,8 +2145,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 
 	fio->seg_type = __get_segment_type(fio->page, fio->type);
 
-	if (fio->type == NODE || fio->type == DATA)
-		mutex_lock(&fio->sbi->wio_mutex[fio->type]);
+	mutex_lock(&fio->sbi->wio_mutex[fio->seg_type]);
 reallocate:
 	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
 					&fio->new_blkaddr, sum, fio->seg_type);
@@ -2158,8 +2157,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 		goto reallocate;
 	}
 
-	if (fio->type == NODE || fio->type == DATA)
-		mutex_unlock(&fio->sbi->wio_mutex[fio->type]);
+	mutex_unlock(&fio->sbi->wio_mutex[fio->seg_type]);
 }
 
 void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fed25ca609e4..8fb16e9e6eb4 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1584,8 +1584,8 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	INIT_LIST_HEAD(&sbi->s_list);
 	mutex_init(&sbi->umount_mutex);
-	mutex_init(&sbi->wio_mutex[NODE]);
-	mutex_init(&sbi->wio_mutex[DATA]);
+	for (i = 0; i < NR_CURSEG_TYPE; i++)
+		mutex_init(&sbi->wio_mutex[i]);
 	spin_lock_init(&sbi->cp_lock);
 }
 
-- 
2.12.2.575.gb14f27f

[PATCH 2/3] f2fs: move f2fs_sb_info.{log_io,wio_mutex} into struct curseg_info

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

Just cleanup, no logic changed.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/data.c    |  4 ++--
 fs/f2fs/f2fs.h    |  2 --
 fs/f2fs/segment.c | 10 ++++++++--
 fs/f2fs/segment.h |  2 ++
 fs/f2fs/super.c   |  7 -------
 5 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 37d0896021c7..5f001b471252 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -289,7 +289,7 @@ static struct f2fs_bio_info *__get_bio_info(struct f2fs_sb_info *sbi, int rw,
 
 		io = &sbi->meta_io[io_type];
 	} else {
-		io = &sbi->log_io[seg_type];
+		io = &CURSEG_I(sbi, seg_type)->bio_info;
 	}
 
 	return io;
@@ -326,7 +326,7 @@ void f2fs_submit_log_bio_cond(struct f2fs_sb_info *sbi,
 	int max = is_data ? CURSEG_COLD_DATA : CURSEG_COLD_NODE;
 
 	for (; i <= max; i++)
-		__f2fs_submit_merged_bio(&sbi->log_io[i],
+		__f2fs_submit_merged_bio(&CURSEG_I(sbi, i)->bio_info,
 					inode, ino, idx, type);
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 500a4c21cfe8..9129a6229bc8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -881,8 +881,6 @@ struct f2fs_sb_info {
 
 	/* for bio operations */
 	struct f2fs_bio_info meta_io[2];		/* for meta bios */
-	struct f2fs_bio_info log_io[NR_CURSEG_TYPE];	/* for log bios */
-	struct mutex wio_mutex[NR_CURSEG_TYPE];	/* bio ordering for NODE/DATA */
 	int write_io_size_bits;			/* Write IO size bits */
 	mempool_t *write_io_dummy;		/* Dummy pages */
 
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9fa6b0682d94..c047b5d8b9d3 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2145,7 +2145,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 
 	fio->seg_type = __get_segment_type(fio->page, fio->type);
 
-	mutex_lock(&fio->sbi->wio_mutex[fio->seg_type]);
+	mutex_lock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
 reallocate:
 	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
 					&fio->new_blkaddr, sum, fio->seg_type);
@@ -2157,7 +2157,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 		goto reallocate;
 	}
 
-	mutex_unlock(&fio->sbi->wio_mutex[fio->seg_type]);
+	mutex_unlock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
 }
 
 void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
@@ -2973,6 +2973,12 @@ static int build_curseg(struct f2fs_sb_info *sbi)
 			return -ENOMEM;
 		array[i].segno = NULL_SEGNO;
 		array[i].next_blkoff = 0;
+
+		init_rwsem(&array[i].bio_info.io_rwsem);
+		array[i].bio_info.sbi = sbi;
+		array[i].bio_info.bio = NULL;
+
+		mutex_init(&array[i].wio_mutex);
 	}
 	return restore_curseg_summaries(sbi);
 }
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 10bf05d4cff4..701944b462cd 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -282,6 +282,8 @@ struct curseg_info {
 	struct f2fs_summary_block *sum_blk;	/* cached summary block */
 	struct rw_semaphore journal_rwsem;	/* protect journal area */
 	struct f2fs_journal *journal;		/* cached journal info */
+	struct f2fs_bio_info bio_info;		/* for log bios */
+	struct mutex wio_mutex;			/* serialize DATA/NODE IOs */
 	unsigned char alloc_type;		/* current allocation type */
 	unsigned int segno;			/* current segment number */
 	unsigned short next_blkoff;		/* next block offset to write */
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8fb16e9e6eb4..5c2bb3851b89 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1584,8 +1584,6 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	INIT_LIST_HEAD(&sbi->s_list);
 	mutex_init(&sbi->umount_mutex);
-	for (i = 0; i < NR_CURSEG_TYPE; i++)
-		mutex_init(&sbi->wio_mutex[i]);
 	spin_lock_init(&sbi->cp_lock);
 }
 
@@ -1955,11 +1953,6 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		sbi->meta_io[i].sbi = sbi;
 		sbi->meta_io[i].bio = NULL;
 	}
-	for (i = 0; i < NR_CURSEG_TYPE; i++) {
-		init_rwsem(&sbi->log_io[i].io_rwsem);
-		sbi->log_io[i].sbi = sbi;
-		sbi->log_io[i].bio = NULL;
-	}
 
 	init_rwsem(&sbi->cp_rwsem);
 	init_waitqueue_head(&sbi->cp_wait);
-- 
2.12.2.575.gb14f27f

[PATCH 3/3] f2fs: introduce io_list for serialize data/node IOs

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

Serialize data/node IOs by using fifo list instead of mutex lock,
it will help to enhance concurrency of f2fs, meanwhile keeping LFS
IO semantics.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c |  1 +
 fs/f2fs/data.c       | 28 ++++++++++++++++++++++++----
 fs/f2fs/f2fs.h       |  5 ++++-
 fs/f2fs/gc.c         |  3 ++-
 fs/f2fs/segment.c    | 20 ++++++++++++++------
 fs/f2fs/segment.h    |  3 ++-
 6 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 2a475e83a092..7b3393474f6b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -162,6 +162,7 @@ int ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
 		.op = REQ_OP_READ,
 		.op_flags = sync ? (REQ_META | REQ_PRIO) : REQ_RAHEAD,
 		.encrypted_page = NULL,
+		.in_list = false,
 	};
 	struct blk_plug plug;
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5f001b471252..89eaa8aaa97b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -393,10 +393,28 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
 	struct f2fs_bio_info *io;
 	bool is_read = is_read_io(fio->op);
 	struct page *bio_page;
+	struct curseg_info *curseg;
 	int err = 0;
 
+	if (fio->in_list)
+		curseg = CURSEG_I(sbi, fio->seg_type);
+
 	io = __get_bio_info(sbi, fio->op, fio->type, fio->seg_type);
 
+	down_write(&io->io_rwsem);
+next:
+	if (fio->in_list) {
+		spin_lock(&curseg->io_lock);
+		if (list_empty(&curseg->io_list)) {
+			spin_unlock(&curseg->io_lock);
+			goto out_fail;
+		}
+		fio = list_first_entry(&curseg->io_list,
+						struct f2fs_io_info, list);
+		list_del(&fio->list);
+		spin_unlock(&curseg->io_lock);
+	}
+
 	if (fio->old_blkaddr != NEW_ADDR)
 		verify_block_addr(sbi, fio->old_blkaddr);
 	verify_block_addr(sbi, fio->new_blkaddr);
@@ -409,8 +427,6 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
 	if (!is_read)
 		inc_page_count(sbi, WB_DATA_TYPE(bio_page));
 
-	down_write(&io->io_rwsem);
-
 	if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
 	    (io->fio.op != fio->op || io->fio.op_flags != fio->op_flags) ||
 			!__same_bdev(sbi, fio->new_blkaddr, io->bio)))
@@ -437,9 +453,13 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
 
 	io->last_block_in_bio = fio->new_blkaddr;
 	f2fs_trace_ios(fio, 0);
+
+	trace_f2fs_submit_page_mbio(fio->page, fio);
+
+	if (fio->in_list)
+		goto next;
 out_fail:
 	up_write(&io->io_rwsem);
-	trace_f2fs_submit_page_mbio(fio->page, fio);
 	return err;
 }
 
@@ -752,7 +772,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
 	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
 
 	allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr,
-						&sum, CURSEG_WARM_DATA);
+					&sum, CURSEG_WARM_DATA, NULL, false);
 	set_data_blkaddr(dn);
 
 	/* update i_size */
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9129a6229bc8..6b8e9f051aa2 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -802,8 +802,10 @@ struct f2fs_io_info {
 	block_t old_blkaddr;	/* old block address before Cow */
 	struct page *page;	/* page to be written */
 	struct page *encrypted_page;	/* encrypted page */
+	struct list_head list;		/* serialize IOs */
 	bool submitted;		/* indicate IO submission */
 	bool need_lock;		/* indicate we need to lock cp_rwsem */
+	bool in_list;		/* indicate fio is in io_list */
 };
 
 #define is_read_io(rw) ((rw) == READ)
@@ -2274,7 +2276,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
 			bool recover_newaddr);
 void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 			block_t old_blkaddr, block_t *new_blkaddr,
-			struct f2fs_summary *sum, int type);
+			struct f2fs_summary *sum, int type,
+			struct f2fs_io_info *fio, bool add_list);
 void f2fs_wait_on_page_writeback(struct page *page,
 			enum page_type type, bool ordered);
 void f2fs_wait_on_encrypted_page_writeback(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 8b267ca30926..ac2f74e40eea 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -590,6 +590,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
 		.op = REQ_OP_READ,
 		.op_flags = 0,
 		.encrypted_page = NULL,
+		.in_list = false,
 	};
 	struct dnode_of_data dn;
 	struct f2fs_summary sum;
@@ -633,7 +634,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
 	fio.new_blkaddr = fio.old_blkaddr = dn.data_blkaddr;
 
 	allocate_data_block(fio.sbi, NULL, fio.old_blkaddr, &newaddr,
-							&sum, fio.seg_type);
+					&sum, fio.seg_type, NULL, false);
 
 	fio.encrypted_page = pagecache_get_page(META_MAPPING(fio.sbi), newaddr,
 					FGP_LOCK | FGP_CREAT, GFP_NOFS);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c047b5d8b9d3..d4975b8f4620 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2100,7 +2100,8 @@ static int __get_segment_type(struct page *page, enum page_type p_type)
 
 void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 		block_t old_blkaddr, block_t *new_blkaddr,
-		struct f2fs_summary *sum, int type)
+		struct f2fs_summary *sum, int type,
+		struct f2fs_io_info *fio, bool add_list)
 {
 	struct sit_info *sit_i = SIT_I(sbi);
 	struct curseg_info *curseg = CURSEG_I(sbi, type);
@@ -2136,6 +2137,14 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 	if (page && IS_NODESEG(type))
 		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
 
+	if (add_list) {
+		INIT_LIST_HEAD(&fio->list);
+		fio->in_list = true;
+		spin_lock(&curseg->io_lock);
+		list_add_tail(&fio->list, &curseg->io_list);
+		spin_unlock(&curseg->io_lock);
+	}
+
 	mutex_unlock(&curseg->curseg_mutex);
 }
 
@@ -2145,10 +2154,9 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 
 	fio->seg_type = __get_segment_type(fio->page, fio->type);
 
-	mutex_lock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
 reallocate:
 	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
-					&fio->new_blkaddr, sum, fio->seg_type);
+			&fio->new_blkaddr, sum, fio->seg_type, fio, true);
 
 	/* writeout dirty page into bdev */
 	err = f2fs_submit_page_mbio(fio);
@@ -2156,8 +2164,6 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 		fio->old_blkaddr = fio->new_blkaddr;
 		goto reallocate;
 	}
-
-	mutex_unlock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
 }
 
 void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
@@ -2171,6 +2177,7 @@ void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
 		.new_blkaddr = page->index,
 		.page = page,
 		.encrypted_page = NULL,
+		.in_list = false,
 	};
 
 	if (unlikely(page->index >= MAIN_BLKADDR(sbi)))
@@ -2978,7 +2985,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
 		array[i].bio_info.sbi = sbi;
 		array[i].bio_info.bio = NULL;
 
-		mutex_init(&array[i].wio_mutex);
+		spin_lock_init(&array[i].io_lock);
+		INIT_LIST_HEAD(&array[i].io_list);
 	}
 	return restore_curseg_summaries(sbi);
 }
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 701944b462cd..b6f5dffeaa61 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -283,7 +283,8 @@ struct curseg_info {
 	struct rw_semaphore journal_rwsem;	/* protect journal area */
 	struct f2fs_journal *journal;		/* cached journal info */
 	struct f2fs_bio_info bio_info;		/* for log bios */
-	struct mutex wio_mutex;			/* serialize DATA/NODE IOs */
+	spinlock_t io_lock;			/* serialize DATA/NODE IOs */
+	struct list_head io_list;		/* tracking fios */
 	unsigned char alloc_type;		/* current allocation type */
 	unsigned int segno;			/* current segment number */
 	unsigned short next_blkoff;		/* next block offset to write */
-- 
2.12.2.575.gb14f27f

Re: [PATCH 3/3] f2fs: introduce io_list for serialize data/node IOs

[Jaegeuk Kim]

Hi Chao,

On 05/09, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> Serialize data/node IOs by using fifo list instead of mutex lock,
> it will help to enhance concurrency of f2fs, meanwhile keeping LFS
> IO semantics.

I'm not against to give it a try, but not sure how much we can get a benefit
from this approach frankly. Have you got a trouble on any lock contention from
the below io_rwsem or mutex?

Thanks,

> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/checkpoint.c |  1 +
>  fs/f2fs/data.c       | 28 ++++++++++++++++++++++++----
>  fs/f2fs/f2fs.h       |  5 ++++-
>  fs/f2fs/gc.c         |  3 ++-
>  fs/f2fs/segment.c    | 20 ++++++++++++++------
>  fs/f2fs/segment.h    |  3 ++-
>  6 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 2a475e83a092..7b3393474f6b 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -162,6 +162,7 @@ int ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
>  		.op = REQ_OP_READ,
>  		.op_flags = sync ? (REQ_META | REQ_PRIO) : REQ_RAHEAD,
>  		.encrypted_page = NULL,
> +		.in_list = false,
>  	};
>  	struct blk_plug plug;
>  
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 5f001b471252..89eaa8aaa97b 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -393,10 +393,28 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>  	struct f2fs_bio_info *io;
>  	bool is_read = is_read_io(fio->op);
>  	struct page *bio_page;
> +	struct curseg_info *curseg;
>  	int err = 0;
>  
> +	if (fio->in_list)
> +		curseg = CURSEG_I(sbi, fio->seg_type);
> +
>  	io = __get_bio_info(sbi, fio->op, fio->type, fio->seg_type);
>  
> +	down_write(&io->io_rwsem);
> +next:
> +	if (fio->in_list) {
> +		spin_lock(&curseg->io_lock);
> +		if (list_empty(&curseg->io_list)) {
> +			spin_unlock(&curseg->io_lock);
> +			goto out_fail;
> +		}
> +		fio = list_first_entry(&curseg->io_list,
> +						struct f2fs_io_info, list);
> +		list_del(&fio->list);
> +		spin_unlock(&curseg->io_lock);
> +	}
> +
>  	if (fio->old_blkaddr != NEW_ADDR)
>  		verify_block_addr(sbi, fio->old_blkaddr);
>  	verify_block_addr(sbi, fio->new_blkaddr);
> @@ -409,8 +427,6 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>  	if (!is_read)
>  		inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>  
> -	down_write(&io->io_rwsem);
> -
>  	if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
>  	    (io->fio.op != fio->op || io->fio.op_flags != fio->op_flags) ||
>  			!__same_bdev(sbi, fio->new_blkaddr, io->bio)))
> @@ -437,9 +453,13 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>  
>  	io->last_block_in_bio = fio->new_blkaddr;
>  	f2fs_trace_ios(fio, 0);
> +
> +	trace_f2fs_submit_page_mbio(fio->page, fio);
> +
> +	if (fio->in_list)
> +		goto next;
>  out_fail:
>  	up_write(&io->io_rwsem);
> -	trace_f2fs_submit_page_mbio(fio->page, fio);
>  	return err;
>  }
>  
> @@ -752,7 +772,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
>  	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
>  
>  	allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr,
> -						&sum, CURSEG_WARM_DATA);
> +					&sum, CURSEG_WARM_DATA, NULL, false);
>  	set_data_blkaddr(dn);
>  
>  	/* update i_size */
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9129a6229bc8..6b8e9f051aa2 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -802,8 +802,10 @@ struct f2fs_io_info {
>  	block_t old_blkaddr;	/* old block address before Cow */
>  	struct page *page;	/* page to be written */
>  	struct page *encrypted_page;	/* encrypted page */
> +	struct list_head list;		/* serialize IOs */
>  	bool submitted;		/* indicate IO submission */
>  	bool need_lock;		/* indicate we need to lock cp_rwsem */
> +	bool in_list;		/* indicate fio is in io_list */
>  };
>  
>  #define is_read_io(rw) ((rw) == READ)
> @@ -2274,7 +2276,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
>  			bool recover_newaddr);
>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  			block_t old_blkaddr, block_t *new_blkaddr,
> -			struct f2fs_summary *sum, int type);
> +			struct f2fs_summary *sum, int type,
> +			struct f2fs_io_info *fio, bool add_list);
>  void f2fs_wait_on_page_writeback(struct page *page,
>  			enum page_type type, bool ordered);
>  void f2fs_wait_on_encrypted_page_writeback(struct f2fs_sb_info *sbi,
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 8b267ca30926..ac2f74e40eea 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -590,6 +590,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>  		.op = REQ_OP_READ,
>  		.op_flags = 0,
>  		.encrypted_page = NULL,
> +		.in_list = false,
>  	};
>  	struct dnode_of_data dn;
>  	struct f2fs_summary sum;
> @@ -633,7 +634,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>  	fio.new_blkaddr = fio.old_blkaddr = dn.data_blkaddr;
>  
>  	allocate_data_block(fio.sbi, NULL, fio.old_blkaddr, &newaddr,
> -							&sum, fio.seg_type);
> +					&sum, fio.seg_type, NULL, false);
>  
>  	fio.encrypted_page = pagecache_get_page(META_MAPPING(fio.sbi), newaddr,
>  					FGP_LOCK | FGP_CREAT, GFP_NOFS);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index c047b5d8b9d3..d4975b8f4620 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2100,7 +2100,8 @@ static int __get_segment_type(struct page *page, enum page_type p_type)
>  
>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  		block_t old_blkaddr, block_t *new_blkaddr,
> -		struct f2fs_summary *sum, int type)
> +		struct f2fs_summary *sum, int type,
> +		struct f2fs_io_info *fio, bool add_list)
>  {
>  	struct sit_info *sit_i = SIT_I(sbi);
>  	struct curseg_info *curseg = CURSEG_I(sbi, type);
> @@ -2136,6 +2137,14 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  	if (page && IS_NODESEG(type))
>  		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>  
> +	if (add_list) {
> +		INIT_LIST_HEAD(&fio->list);
> +		fio->in_list = true;
> +		spin_lock(&curseg->io_lock);
> +		list_add_tail(&fio->list, &curseg->io_list);
> +		spin_unlock(&curseg->io_lock);
> +	}
> +
>  	mutex_unlock(&curseg->curseg_mutex);
>  }
>  
> @@ -2145,10 +2154,9 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>  
>  	fio->seg_type = __get_segment_type(fio->page, fio->type);
>  
> -	mutex_lock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
>  reallocate:
>  	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
> -					&fio->new_blkaddr, sum, fio->seg_type);
> +			&fio->new_blkaddr, sum, fio->seg_type, fio, true);
>  
>  	/* writeout dirty page into bdev */
>  	err = f2fs_submit_page_mbio(fio);
> @@ -2156,8 +2164,6 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>  		fio->old_blkaddr = fio->new_blkaddr;
>  		goto reallocate;
>  	}
> -
> -	mutex_unlock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
>  }
>  
>  void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
> @@ -2171,6 +2177,7 @@ void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
>  		.new_blkaddr = page->index,
>  		.page = page,
>  		.encrypted_page = NULL,
> +		.in_list = false,
>  	};
>  
>  	if (unlikely(page->index >= MAIN_BLKADDR(sbi)))
> @@ -2978,7 +2985,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>  		array[i].bio_info.sbi = sbi;
>  		array[i].bio_info.bio = NULL;
>  
> -		mutex_init(&array[i].wio_mutex);
> +		spin_lock_init(&array[i].io_lock);
> +		INIT_LIST_HEAD(&array[i].io_list);
>  	}
>  	return restore_curseg_summaries(sbi);
>  }
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 701944b462cd..b6f5dffeaa61 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -283,7 +283,8 @@ struct curseg_info {
>  	struct rw_semaphore journal_rwsem;	/* protect journal area */
>  	struct f2fs_journal *journal;		/* cached journal info */
>  	struct f2fs_bio_info bio_info;		/* for log bios */
> -	struct mutex wio_mutex;			/* serialize DATA/NODE IOs */
> +	spinlock_t io_lock;			/* serialize DATA/NODE IOs */
> +	struct list_head io_list;		/* tracking fios */
>  	unsigned char alloc_type;		/* current allocation type */
>  	unsigned int segno;			/* current segment number */
>  	unsigned short next_blkoff;		/* next block offset to write */
> -- 
> 2.12.2.575.gb14f27f

Re: [PATCH 1/3] f2fs: split wio_mutex

[Jaegeuk Kim]

On 05/09, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> Split wio_mutex to adjust different temperature bio cache.

This can be rephrased like:

Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h    | 3 ++-
 fs/f2fs/segment.c | 4 ++--
 fs/f2fs/super.c   | 7 ++++---
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c6643783adff..fb710bfbe215 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -888,7 +888,8 @@ struct f2fs_sb_info {
 
 	/* for bio operations */
 	struct f2fs_bio_info *write_io[NR_PAGE_TYPE];	/* for write bios */
-	struct mutex wio_mutex[NODE + 1];	/* bio ordering for NODE/DATA */
+	struct mutex wio_mutex[NR_PAGE_TYPE - 1][NR_TEMP_TYPE];
+						/* bio ordering for NODE/DATA */
 	int write_io_size_bits;			/* Write IO size bits */
 	mempool_t *write_io_dummy;		/* Dummy pages */
 
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index da4fd2c29e86..cc2edaf36e55 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2156,7 +2156,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 	int err;
 
 	if (fio->type == NODE || fio->type == DATA)
-		mutex_lock(&fio->sbi->wio_mutex[fio->type]);
+		mutex_lock(&fio->sbi->wio_mutex[fio->type][fio->temp]);
 reallocate:
 	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
 					&fio->new_blkaddr, sum, type);
@@ -2169,7 +2169,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 	}
 
 	if (fio->type == NODE || fio->type == DATA)
-		mutex_unlock(&fio->sbi->wio_mutex[fio->type]);
+		mutex_unlock(&fio->sbi->wio_mutex[fio->type][fio->temp]);
 }
 
 void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fea563bff0c1..df19902282f2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1555,7 +1555,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
 static void init_sb_info(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_super_block *raw_super = sbi->raw_super;
-	int i;
+	int i, j;
 
 	sbi->log_sectors_per_block =
 		le32_to_cpu(raw_super->log_sectors_per_block);
@@ -1587,8 +1587,9 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 
 	INIT_LIST_HEAD(&sbi->s_list);
 	mutex_init(&sbi->umount_mutex);
-	mutex_init(&sbi->wio_mutex[NODE]);
-	mutex_init(&sbi->wio_mutex[DATA]);
+	for (i = 0; i < NR_PAGE_TYPE - 1; i++)
+		for (j = HOT; j < NR_TEMP_TYPE; j++)
+			mutex_init(&sbi->wio_mutex[i][j]);
 	spin_lock_init(&sbi->cp_lock);
 }
 
-- 
2.11.0

Re: [PATCH 1/3] f2fs: split wio_mutex

[Chao Yu]

On 2017/5/12 2:37, Jaegeuk Kim wrote:
> On 05/09, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> Split wio_mutex to adjust different temperature bio cache.
> 
> This can be rephrased like:

Yup, let me resend this patch.

Thanks,

> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/f2fs.h    | 3 ++-
>  fs/f2fs/segment.c | 4 ++--
>  fs/f2fs/super.c   | 7 ++++---
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c6643783adff..fb710bfbe215 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -888,7 +888,8 @@ struct f2fs_sb_info {
>  
>  	/* for bio operations */
>  	struct f2fs_bio_info *write_io[NR_PAGE_TYPE];	/* for write bios */
> -	struct mutex wio_mutex[NODE + 1];	/* bio ordering for NODE/DATA */
> +	struct mutex wio_mutex[NR_PAGE_TYPE - 1][NR_TEMP_TYPE];
> +						/* bio ordering for NODE/DATA */
>  	int write_io_size_bits;			/* Write IO size bits */
>  	mempool_t *write_io_dummy;		/* Dummy pages */
>  
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index da4fd2c29e86..cc2edaf36e55 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2156,7 +2156,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>  	int err;
>  
>  	if (fio->type == NODE || fio->type == DATA)
> -		mutex_lock(&fio->sbi->wio_mutex[fio->type]);
> +		mutex_lock(&fio->sbi->wio_mutex[fio->type][fio->temp]);
>  reallocate:
>  	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
>  					&fio->new_blkaddr, sum, type);
> @@ -2169,7 +2169,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>  	}
>  
>  	if (fio->type == NODE || fio->type == DATA)
> -		mutex_unlock(&fio->sbi->wio_mutex[fio->type]);
> +		mutex_unlock(&fio->sbi->wio_mutex[fio->type][fio->temp]);
>  }
>  
>  void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index fea563bff0c1..df19902282f2 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1555,7 +1555,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>  static void init_sb_info(struct f2fs_sb_info *sbi)
>  {
>  	struct f2fs_super_block *raw_super = sbi->raw_super;
> -	int i;
> +	int i, j;
>  
>  	sbi->log_sectors_per_block =
>  		le32_to_cpu(raw_super->log_sectors_per_block);
> @@ -1587,8 +1587,9 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>  
>  	INIT_LIST_HEAD(&sbi->s_list);
>  	mutex_init(&sbi->umount_mutex);
> -	mutex_init(&sbi->wio_mutex[NODE]);
> -	mutex_init(&sbi->wio_mutex[DATA]);
> +	for (i = 0; i < NR_PAGE_TYPE - 1; i++)
> +		for (j = HOT; j < NR_TEMP_TYPE; j++)
> +			mutex_init(&sbi->wio_mutex[i][j]);
>  	spin_lock_init(&sbi->cp_lock);
>  }
>  
> 

Re: [PATCH 3/3] f2fs: introduce io_list for serialize data/node IOs

[Chao Yu]

Hi Jaegeuk,

On 2017/5/12 2:36, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On 05/09, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> Serialize data/node IOs by using fifo list instead of mutex lock,
>> it will help to enhance concurrency of f2fs, meanwhile keeping LFS
>> IO semantics.
> 
> I'm not against to give it a try, but not sure how much we can get a benefit
> from this approach frankly. Have you got a trouble on any lock contention from
> the below io_rwsem or mutex?

Yes, because submitting IOs can be blocked in block layer since there may be:
- limitation of some resources, e.g. request number.
- IO throttle
Holding a global mutex lock in the path is not good idea, as it may cause
potential hungtask or concurrency performance regression.

So I add this patch to relief impacting of global mutex.

Thanks,

> 
> Thanks,
> 
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/checkpoint.c |  1 +
>>  fs/f2fs/data.c       | 28 ++++++++++++++++++++++++----
>>  fs/f2fs/f2fs.h       |  5 ++++-
>>  fs/f2fs/gc.c         |  3 ++-
>>  fs/f2fs/segment.c    | 20 ++++++++++++++------
>>  fs/f2fs/segment.h    |  3 ++-
>>  6 files changed, 47 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 2a475e83a092..7b3393474f6b 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -162,6 +162,7 @@ int ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
>>  		.op = REQ_OP_READ,
>>  		.op_flags = sync ? (REQ_META | REQ_PRIO) : REQ_RAHEAD,
>>  		.encrypted_page = NULL,
>> +		.in_list = false,
>>  	};
>>  	struct blk_plug plug;
>>  
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 5f001b471252..89eaa8aaa97b 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -393,10 +393,28 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>>  	struct f2fs_bio_info *io;
>>  	bool is_read = is_read_io(fio->op);
>>  	struct page *bio_page;
>> +	struct curseg_info *curseg;
>>  	int err = 0;
>>  
>> +	if (fio->in_list)
>> +		curseg = CURSEG_I(sbi, fio->seg_type);
>> +
>>  	io = __get_bio_info(sbi, fio->op, fio->type, fio->seg_type);
>>  
>> +	down_write(&io->io_rwsem);
>> +next:
>> +	if (fio->in_list) {
>> +		spin_lock(&curseg->io_lock);
>> +		if (list_empty(&curseg->io_list)) {
>> +			spin_unlock(&curseg->io_lock);
>> +			goto out_fail;
>> +		}
>> +		fio = list_first_entry(&curseg->io_list,
>> +						struct f2fs_io_info, list);
>> +		list_del(&fio->list);
>> +		spin_unlock(&curseg->io_lock);
>> +	}
>> +
>>  	if (fio->old_blkaddr != NEW_ADDR)
>>  		verify_block_addr(sbi, fio->old_blkaddr);
>>  	verify_block_addr(sbi, fio->new_blkaddr);
>> @@ -409,8 +427,6 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>>  	if (!is_read)
>>  		inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>>  
>> -	down_write(&io->io_rwsem);
>> -
>>  	if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
>>  	    (io->fio.op != fio->op || io->fio.op_flags != fio->op_flags) ||
>>  			!__same_bdev(sbi, fio->new_blkaddr, io->bio)))
>> @@ -437,9 +453,13 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>>  
>>  	io->last_block_in_bio = fio->new_blkaddr;
>>  	f2fs_trace_ios(fio, 0);
>> +
>> +	trace_f2fs_submit_page_mbio(fio->page, fio);
>> +
>> +	if (fio->in_list)
>> +		goto next;
>>  out_fail:
>>  	up_write(&io->io_rwsem);
>> -	trace_f2fs_submit_page_mbio(fio->page, fio);
>>  	return err;
>>  }
>>  
>> @@ -752,7 +772,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
>>  	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
>>  
>>  	allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr,
>> -						&sum, CURSEG_WARM_DATA);
>> +					&sum, CURSEG_WARM_DATA, NULL, false);
>>  	set_data_blkaddr(dn);
>>  
>>  	/* update i_size */
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 9129a6229bc8..6b8e9f051aa2 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -802,8 +802,10 @@ struct f2fs_io_info {
>>  	block_t old_blkaddr;	/* old block address before Cow */
>>  	struct page *page;	/* page to be written */
>>  	struct page *encrypted_page;	/* encrypted page */
>> +	struct list_head list;		/* serialize IOs */
>>  	bool submitted;		/* indicate IO submission */
>>  	bool need_lock;		/* indicate we need to lock cp_rwsem */
>> +	bool in_list;		/* indicate fio is in io_list */
>>  };
>>  
>>  #define is_read_io(rw) ((rw) == READ)
>> @@ -2274,7 +2276,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
>>  			bool recover_newaddr);
>>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>  			block_t old_blkaddr, block_t *new_blkaddr,
>> -			struct f2fs_summary *sum, int type);
>> +			struct f2fs_summary *sum, int type,
>> +			struct f2fs_io_info *fio, bool add_list);
>>  void f2fs_wait_on_page_writeback(struct page *page,
>>  			enum page_type type, bool ordered);
>>  void f2fs_wait_on_encrypted_page_writeback(struct f2fs_sb_info *sbi,
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 8b267ca30926..ac2f74e40eea 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -590,6 +590,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>>  		.op = REQ_OP_READ,
>>  		.op_flags = 0,
>>  		.encrypted_page = NULL,
>> +		.in_list = false,
>>  	};
>>  	struct dnode_of_data dn;
>>  	struct f2fs_summary sum;
>> @@ -633,7 +634,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>>  	fio.new_blkaddr = fio.old_blkaddr = dn.data_blkaddr;
>>  
>>  	allocate_data_block(fio.sbi, NULL, fio.old_blkaddr, &newaddr,
>> -							&sum, fio.seg_type);
>> +					&sum, fio.seg_type, NULL, false);
>>  
>>  	fio.encrypted_page = pagecache_get_page(META_MAPPING(fio.sbi), newaddr,
>>  					FGP_LOCK | FGP_CREAT, GFP_NOFS);
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index c047b5d8b9d3..d4975b8f4620 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2100,7 +2100,8 @@ static int __get_segment_type(struct page *page, enum page_type p_type)
>>  
>>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>  		block_t old_blkaddr, block_t *new_blkaddr,
>> -		struct f2fs_summary *sum, int type)
>> +		struct f2fs_summary *sum, int type,
>> +		struct f2fs_io_info *fio, bool add_list)
>>  {
>>  	struct sit_info *sit_i = SIT_I(sbi);
>>  	struct curseg_info *curseg = CURSEG_I(sbi, type);
>> @@ -2136,6 +2137,14 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>  	if (page && IS_NODESEG(type))
>>  		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>>  
>> +	if (add_list) {
>> +		INIT_LIST_HEAD(&fio->list);
>> +		fio->in_list = true;
>> +		spin_lock(&curseg->io_lock);
>> +		list_add_tail(&fio->list, &curseg->io_list);
>> +		spin_unlock(&curseg->io_lock);
>> +	}
>> +
>>  	mutex_unlock(&curseg->curseg_mutex);
>>  }
>>  
>> @@ -2145,10 +2154,9 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>>  
>>  	fio->seg_type = __get_segment_type(fio->page, fio->type);
>>  
>> -	mutex_lock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
>>  reallocate:
>>  	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
>> -					&fio->new_blkaddr, sum, fio->seg_type);
>> +			&fio->new_blkaddr, sum, fio->seg_type, fio, true);
>>  
>>  	/* writeout dirty page into bdev */
>>  	err = f2fs_submit_page_mbio(fio);
>> @@ -2156,8 +2164,6 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
>>  		fio->old_blkaddr = fio->new_blkaddr;
>>  		goto reallocate;
>>  	}
>> -
>> -	mutex_unlock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
>>  }
>>  
>>  void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
>> @@ -2171,6 +2177,7 @@ void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
>>  		.new_blkaddr = page->index,
>>  		.page = page,
>>  		.encrypted_page = NULL,
>> +		.in_list = false,
>>  	};
>>  
>>  	if (unlikely(page->index >= MAIN_BLKADDR(sbi)))
>> @@ -2978,7 +2985,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>>  		array[i].bio_info.sbi = sbi;
>>  		array[i].bio_info.bio = NULL;
>>  
>> -		mutex_init(&array[i].wio_mutex);
>> +		spin_lock_init(&array[i].io_lock);
>> +		INIT_LIST_HEAD(&array[i].io_list);
>>  	}
>>  	return restore_curseg_summaries(sbi);
>>  }
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 701944b462cd..b6f5dffeaa61 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -283,7 +283,8 @@ struct curseg_info {
>>  	struct rw_semaphore journal_rwsem;	/* protect journal area */
>>  	struct f2fs_journal *journal;		/* cached journal info */
>>  	struct f2fs_bio_info bio_info;		/* for log bios */
>> -	struct mutex wio_mutex;			/* serialize DATA/NODE IOs */
>> +	spinlock_t io_lock;			/* serialize DATA/NODE IOs */
>> +	struct list_head io_list;		/* tracking fios */
>>  	unsigned char alloc_type;		/* current allocation type */
>>  	unsigned int segno;			/* current segment number */
>>  	unsigned short next_blkoff;		/* next block offset to write */
>> -- 
>> 2.12.2.575.gb14f27f
> 
> .
> 

Re: [PATCH 3/3] f2fs: introduce io_list for serialize data/node IOs

[Jaegeuk Kim]

On 05/12, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2017/5/12 2:36, Jaegeuk Kim wrote:
> > Hi Chao,
> > 
> > On 05/09, Chao Yu wrote:
> >> From: Chao Yu <yuchao0@huawei.com>
> >>
> >> Serialize data/node IOs by using fifo list instead of mutex lock,
> >> it will help to enhance concurrency of f2fs, meanwhile keeping LFS
> >> IO semantics.
> > 
> > I'm not against to give it a try, but not sure how much we can get a benefit
> > from this approach frankly. Have you got a trouble on any lock contention from
> > the below io_rwsem or mutex?
> 
> Yes, because submitting IOs can be blocked in block layer since there may be:
> - limitation of some resources, e.g. request number.
> - IO throttle
> Holding a global mutex lock in the path is not good idea, as it may cause
> potential hungtask or concurrency performance regression.
> 
> So I add this patch to relief impacting of global mutex.

Okay, could you send a modified patch? Then, let me evaluate it.

Thanks,

> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  fs/f2fs/checkpoint.c |  1 +
> >>  fs/f2fs/data.c       | 28 ++++++++++++++++++++++++----
> >>  fs/f2fs/f2fs.h       |  5 ++++-
> >>  fs/f2fs/gc.c         |  3 ++-
> >>  fs/f2fs/segment.c    | 20 ++++++++++++++------
> >>  fs/f2fs/segment.h    |  3 ++-
> >>  6 files changed, 47 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >> index 2a475e83a092..7b3393474f6b 100644
> >> --- a/fs/f2fs/checkpoint.c
> >> +++ b/fs/f2fs/checkpoint.c
> >> @@ -162,6 +162,7 @@ int ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages,
> >>  		.op = REQ_OP_READ,
> >>  		.op_flags = sync ? (REQ_META | REQ_PRIO) : REQ_RAHEAD,
> >>  		.encrypted_page = NULL,
> >> +		.in_list = false,
> >>  	};
> >>  	struct blk_plug plug;
> >>  
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index 5f001b471252..89eaa8aaa97b 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -393,10 +393,28 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> >>  	struct f2fs_bio_info *io;
> >>  	bool is_read = is_read_io(fio->op);
> >>  	struct page *bio_page;
> >> +	struct curseg_info *curseg;
> >>  	int err = 0;
> >>  
> >> +	if (fio->in_list)
> >> +		curseg = CURSEG_I(sbi, fio->seg_type);
> >> +
> >>  	io = __get_bio_info(sbi, fio->op, fio->type, fio->seg_type);
> >>  
> >> +	down_write(&io->io_rwsem);
> >> +next:
> >> +	if (fio->in_list) {
> >> +		spin_lock(&curseg->io_lock);
> >> +		if (list_empty(&curseg->io_list)) {
> >> +			spin_unlock(&curseg->io_lock);
> >> +			goto out_fail;
> >> +		}
> >> +		fio = list_first_entry(&curseg->io_list,
> >> +						struct f2fs_io_info, list);
> >> +		list_del(&fio->list);
> >> +		spin_unlock(&curseg->io_lock);
> >> +	}
> >> +
> >>  	if (fio->old_blkaddr != NEW_ADDR)
> >>  		verify_block_addr(sbi, fio->old_blkaddr);
> >>  	verify_block_addr(sbi, fio->new_blkaddr);
> >> @@ -409,8 +427,6 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> >>  	if (!is_read)
> >>  		inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> >>  
> >> -	down_write(&io->io_rwsem);
> >> -
> >>  	if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
> >>  	    (io->fio.op != fio->op || io->fio.op_flags != fio->op_flags) ||
> >>  			!__same_bdev(sbi, fio->new_blkaddr, io->bio)))
> >> @@ -437,9 +453,13 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> >>  
> >>  	io->last_block_in_bio = fio->new_blkaddr;
> >>  	f2fs_trace_ios(fio, 0);
> >> +
> >> +	trace_f2fs_submit_page_mbio(fio->page, fio);
> >> +
> >> +	if (fio->in_list)
> >> +		goto next;
> >>  out_fail:
> >>  	up_write(&io->io_rwsem);
> >> -	trace_f2fs_submit_page_mbio(fio->page, fio);
> >>  	return err;
> >>  }
> >>  
> >> @@ -752,7 +772,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
> >>  	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
> >>  
> >>  	allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr,
> >> -						&sum, CURSEG_WARM_DATA);
> >> +					&sum, CURSEG_WARM_DATA, NULL, false);
> >>  	set_data_blkaddr(dn);
> >>  
> >>  	/* update i_size */
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index 9129a6229bc8..6b8e9f051aa2 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -802,8 +802,10 @@ struct f2fs_io_info {
> >>  	block_t old_blkaddr;	/* old block address before Cow */
> >>  	struct page *page;	/* page to be written */
> >>  	struct page *encrypted_page;	/* encrypted page */
> >> +	struct list_head list;		/* serialize IOs */
> >>  	bool submitted;		/* indicate IO submission */
> >>  	bool need_lock;		/* indicate we need to lock cp_rwsem */
> >> +	bool in_list;		/* indicate fio is in io_list */
> >>  };
> >>  
> >>  #define is_read_io(rw) ((rw) == READ)
> >> @@ -2274,7 +2276,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
> >>  			bool recover_newaddr);
> >>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>  			block_t old_blkaddr, block_t *new_blkaddr,
> >> -			struct f2fs_summary *sum, int type);
> >> +			struct f2fs_summary *sum, int type,
> >> +			struct f2fs_io_info *fio, bool add_list);
> >>  void f2fs_wait_on_page_writeback(struct page *page,
> >>  			enum page_type type, bool ordered);
> >>  void f2fs_wait_on_encrypted_page_writeback(struct f2fs_sb_info *sbi,
> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index 8b267ca30926..ac2f74e40eea 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -590,6 +590,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
> >>  		.op = REQ_OP_READ,
> >>  		.op_flags = 0,
> >>  		.encrypted_page = NULL,
> >> +		.in_list = false,
> >>  	};
> >>  	struct dnode_of_data dn;
> >>  	struct f2fs_summary sum;
> >> @@ -633,7 +634,7 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
> >>  	fio.new_blkaddr = fio.old_blkaddr = dn.data_blkaddr;
> >>  
> >>  	allocate_data_block(fio.sbi, NULL, fio.old_blkaddr, &newaddr,
> >> -							&sum, fio.seg_type);
> >> +					&sum, fio.seg_type, NULL, false);
> >>  
> >>  	fio.encrypted_page = pagecache_get_page(META_MAPPING(fio.sbi), newaddr,
> >>  					FGP_LOCK | FGP_CREAT, GFP_NOFS);
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index c047b5d8b9d3..d4975b8f4620 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -2100,7 +2100,8 @@ static int __get_segment_type(struct page *page, enum page_type p_type)
> >>  
> >>  void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>  		block_t old_blkaddr, block_t *new_blkaddr,
> >> -		struct f2fs_summary *sum, int type)
> >> +		struct f2fs_summary *sum, int type,
> >> +		struct f2fs_io_info *fio, bool add_list)
> >>  {
> >>  	struct sit_info *sit_i = SIT_I(sbi);
> >>  	struct curseg_info *curseg = CURSEG_I(sbi, type);
> >> @@ -2136,6 +2137,14 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>  	if (page && IS_NODESEG(type))
> >>  		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
> >>  
> >> +	if (add_list) {
> >> +		INIT_LIST_HEAD(&fio->list);
> >> +		fio->in_list = true;
> >> +		spin_lock(&curseg->io_lock);
> >> +		list_add_tail(&fio->list, &curseg->io_list);
> >> +		spin_unlock(&curseg->io_lock);
> >> +	}
> >> +
> >>  	mutex_unlock(&curseg->curseg_mutex);
> >>  }
> >>  
> >> @@ -2145,10 +2154,9 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
> >>  
> >>  	fio->seg_type = __get_segment_type(fio->page, fio->type);
> >>  
> >> -	mutex_lock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
> >>  reallocate:
> >>  	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
> >> -					&fio->new_blkaddr, sum, fio->seg_type);
> >> +			&fio->new_blkaddr, sum, fio->seg_type, fio, true);
> >>  
> >>  	/* writeout dirty page into bdev */
> >>  	err = f2fs_submit_page_mbio(fio);
> >> @@ -2156,8 +2164,6 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
> >>  		fio->old_blkaddr = fio->new_blkaddr;
> >>  		goto reallocate;
> >>  	}
> >> -
> >> -	mutex_unlock(&CURSEG_I(fio->sbi, fio->seg_type)->wio_mutex);
> >>  }
> >>  
> >>  void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
> >> @@ -2171,6 +2177,7 @@ void write_meta_page(struct f2fs_sb_info *sbi, struct page *page)
> >>  		.new_blkaddr = page->index,
> >>  		.page = page,
> >>  		.encrypted_page = NULL,
> >> +		.in_list = false,
> >>  	};
> >>  
> >>  	if (unlikely(page->index >= MAIN_BLKADDR(sbi)))
> >> @@ -2978,7 +2985,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
> >>  		array[i].bio_info.sbi = sbi;
> >>  		array[i].bio_info.bio = NULL;
> >>  
> >> -		mutex_init(&array[i].wio_mutex);
> >> +		spin_lock_init(&array[i].io_lock);
> >> +		INIT_LIST_HEAD(&array[i].io_list);
> >>  	}
> >>  	return restore_curseg_summaries(sbi);
> >>  }
> >> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> >> index 701944b462cd..b6f5dffeaa61 100644
> >> --- a/fs/f2fs/segment.h
> >> +++ b/fs/f2fs/segment.h
> >> @@ -283,7 +283,8 @@ struct curseg_info {
> >>  	struct rw_semaphore journal_rwsem;	/* protect journal area */
> >>  	struct f2fs_journal *journal;		/* cached journal info */
> >>  	struct f2fs_bio_info bio_info;		/* for log bios */
> >> -	struct mutex wio_mutex;			/* serialize DATA/NODE IOs */
> >> +	spinlock_t io_lock;			/* serialize DATA/NODE IOs */
> >> +	struct list_head io_list;		/* tracking fios */
> >>  	unsigned char alloc_type;		/* current allocation type */
> >>  	unsigned int segno;			/* current segment number */
> >>  	unsigned short next_blkoff;		/* next block offset to write */
> >> -- 
> >> 2.12.2.575.gb14f27f
> > 
> > .
> >