[PATCH 1/7] f2fs: set mapping error for EIO

[PATCH 1/7] f2fs: set mapping error for EIO

[Jaegeuk Kim]

If EIO occurred, we need to set all the mapping to avoid any further IOs.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index c9d6fe2..30dc448 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1243,7 +1243,7 @@ write:
 
 	/* we should bypass data pages to proceed the kworkder jobs */
 	if (unlikely(f2fs_cp_error(sbi))) {
-		SetPageError(page);
+		mapping_set_error(page->mapping, -EIO);
 		goto out;
 	}
 
-- 
2.8.3

[PATCH 2/7] f2fs: avoid reverse IO order for NODE and DATA

[Jaegeuk Kim]

There is a data race between allocate_data_block() and f2fs_sbumit_page_mbio(),
which incur unnecessary reversed bio submission.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h    | 1 +
 fs/f2fs/segment.c | 6 ++++++
 fs/f2fs/super.c   | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d1702ee..b35b68b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -786,6 +786,7 @@ struct f2fs_sb_info {
 	/* for bio operations */
 	struct f2fs_bio_info read_io;			/* for read bios */
 	struct f2fs_bio_info write_io[NR_PAGE_TYPE];	/* for write bios */
+	struct mutex wio_mutex[NODE + 1];	/* bio ordering for NODE/DATA */
 
 	/* for checkpoint */
 	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9011bffd..7b58bfb 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1399,11 +1399,17 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 {
 	int type = __get_segment_type(fio->page, fio->type);
 
+	if (fio->type == NODE || fio->type == DATA)
+		mutex_lock(&fio->sbi->wio_mutex[fio->type]);
+
 	allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
 					&fio->new_blkaddr, sum, type);
 
 	/* writeout dirty page into bdev */
 	f2fs_submit_page_mbio(fio);
+
+	if (fio->type == NODE || fio->type == DATA)
+		mutex_unlock(&fio->sbi->wio_mutex[fio->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 94bb87d..dc66f16 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1436,6 +1436,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]);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
-- 
2.8.3

[PATCH 3/7] f2fs: drop any block plugging

[Jaegeuk Kim]

In f2fs, we don't need to keep block plugging for NODE and DATA writes, since
we already merged bios as much as possible.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c |  4 ----
 fs/f2fs/data.c       | 17 ++++++++++-------
 fs/f2fs/gc.c         |  5 -----
 fs/f2fs/segment.c    |  7 +------
 4 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 5ddd15c..4179c7b 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
 		.nr_to_write = LONG_MAX,
 		.for_reclaim = 0,
 	};
-	struct blk_plug plug;
 	int err = 0;
 
-	blk_start_plug(&plug);
-
 retry_flush_dents:
 	f2fs_lock_all(sbi);
 	/* write all the dirty dentry pages */
@@ -938,7 +935,6 @@ retry_flush_nodes:
 		goto retry_flush_nodes;
 	}
 out:
-	blk_finish_plug(&plug);
 	return err;
 }
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 30dc448..5f655d0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
 }
 
 static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
-						struct bio *bio)
+			struct bio *bio, enum page_type type)
 {
-	if (!is_read_io(rw))
+	if (!is_read_io(rw)) {
 		atomic_inc(&sbi->nr_wb_bios);
+		if (current->plug && (type == DATA || type == NODE))
+			blk_finish_plug(current->plug);
+	}
 	submit_bio(rw, bio);
 }
 
@@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
 	else
 		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
 
-	__submit_bio(io->sbi, fio->rw, io->bio);
+	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
 	io->bio = NULL;
 }
 
@@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
 		return -EFAULT;
 	}
 
-	__submit_bio(fio->sbi, fio->rw, bio);
+	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
 	return 0;
 }
 
@@ -1040,7 +1043,7 @@ got_it:
 		 */
 		if (bio && (last_block_in_bio != block_nr - 1)) {
 submit_and_realloc:
-			__submit_bio(F2FS_I_SB(inode), READ, bio);
+			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
 			bio = NULL;
 		}
 		if (bio == NULL) {
@@ -1083,7 +1086,7 @@ set_error_page:
 		goto next_page;
 confused:
 		if (bio) {
-			__submit_bio(F2FS_I_SB(inode), READ, bio);
+			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
 			bio = NULL;
 		}
 		unlock_page(page);
@@ -1093,7 +1096,7 @@ next_page:
 	}
 	BUG_ON(pages && !list_empty(pages));
 	if (bio)
-		__submit_bio(F2FS_I_SB(inode), READ, bio);
+		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
 	return 0;
 }
 
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 4a03076..67fd285 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -777,7 +777,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
 {
 	struct page *sum_page;
 	struct f2fs_summary_block *sum;
-	struct blk_plug plug;
 	unsigned int segno = start_segno;
 	unsigned int end_segno = start_segno + sbi->segs_per_sec;
 	int seg_freed = 0;
@@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
 		unlock_page(sum_page);
 	}
 
-	blk_start_plug(&plug);
-
 	for (segno = start_segno; segno < end_segno; segno++) {
 		/* find segment summary of victim */
 		sum_page = find_get_page(META_MAPPING(sbi),
@@ -830,8 +827,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
 		f2fs_submit_merged_bio(sbi,
 				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
 
-	blk_finish_plug(&plug);
-
 	if (gc_type == FG_GC) {
 		while (start_segno < end_segno)
 			if (get_valid_blocks(sbi, start_segno++, 1) == 0)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 7b58bfb..eff046a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
 			excess_prefree_segs(sbi) ||
 			excess_dirty_nats(sbi) ||
 			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
-		if (test_opt(sbi, DATA_FLUSH)) {
-			struct blk_plug plug;
-
-			blk_start_plug(&plug);
+		if (test_opt(sbi, DATA_FLUSH))
 			sync_dirty_inodes(sbi, FILE_INODE);
-			blk_finish_plug(&plug);
-		}
 		f2fs_sync_fs(sbi->sb, true);
 		stat_inc_bg_cp_count(sbi->stat_info);
 	}
-- 
2.8.3

Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[Chao Yu]

Hi Jaegeuk,

On 2016/6/9 1:24, Jaegeuk Kim wrote:
> In f2fs, we don't need to keep block plugging for NODE and DATA writes, since
> we already merged bios as much as possible.

IMO, we can not remove block plug, this is because there are still many
conditions which stops us merging r/w IOs into one bio as we expect,
theoretically, block plug can hold bios as much as possible, then submitting
them into queue in batch, it will reduce racing of grabbing queue->lock during
bio submitting, if we drop them, when syncing nodes or flushing datas, we will
suffer more lock racing.

Or there are something I am missing, do you suffer any performance issue on
block plug?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/checkpoint.c |  4 ----
>  fs/f2fs/data.c       | 17 ++++++++++-------
>  fs/f2fs/gc.c         |  5 -----
>  fs/f2fs/segment.c    |  7 +------
>  4 files changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 5ddd15c..4179c7b 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
>  		.nr_to_write = LONG_MAX,
>  		.for_reclaim = 0,
>  	};
> -	struct blk_plug plug;
>  	int err = 0;
>  
> -	blk_start_plug(&plug);
> -
>  retry_flush_dents:
>  	f2fs_lock_all(sbi);
>  	/* write all the dirty dentry pages */
> @@ -938,7 +935,6 @@ retry_flush_nodes:
>  		goto retry_flush_nodes;
>  	}
>  out:
> -	blk_finish_plug(&plug);
>  	return err;
>  }
>  
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 30dc448..5f655d0 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
>  }
>  
>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> -						struct bio *bio)
> +			struct bio *bio, enum page_type type)
>  {
> -	if (!is_read_io(rw))
> +	if (!is_read_io(rw)) {
>  		atomic_inc(&sbi->nr_wb_bios);
> +		if (current->plug && (type == DATA || type == NODE))
> +			blk_finish_plug(current->plug);
> +	}
>  	submit_bio(rw, bio);
>  }
>  
> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
>  	else
>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
>  
> -	__submit_bio(io->sbi, fio->rw, io->bio);
> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
>  	io->bio = NULL;
>  }
>  
> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
>  		return -EFAULT;
>  	}
>  
> -	__submit_bio(fio->sbi, fio->rw, bio);
> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
>  	return 0;
>  }
>  
> @@ -1040,7 +1043,7 @@ got_it:
>  		 */
>  		if (bio && (last_block_in_bio != block_nr - 1)) {
>  submit_and_realloc:
> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>  			bio = NULL;
>  		}
>  		if (bio == NULL) {
> @@ -1083,7 +1086,7 @@ set_error_page:
>  		goto next_page;
>  confused:
>  		if (bio) {
> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>  			bio = NULL;
>  		}
>  		unlock_page(page);
> @@ -1093,7 +1096,7 @@ next_page:
>  	}
>  	BUG_ON(pages && !list_empty(pages));
>  	if (bio)
> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>  	return 0;
>  }
>  
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 4a03076..67fd285 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>  {
>  	struct page *sum_page;
>  	struct f2fs_summary_block *sum;
> -	struct blk_plug plug;
>  	unsigned int segno = start_segno;
>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
>  	int seg_freed = 0;
> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>  		unlock_page(sum_page);
>  	}
>  
> -	blk_start_plug(&plug);
> -
>  	for (segno = start_segno; segno < end_segno; segno++) {
>  		/* find segment summary of victim */
>  		sum_page = find_get_page(META_MAPPING(sbi),
> @@ -830,8 +827,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>  		f2fs_submit_merged_bio(sbi,
>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
>  
> -	blk_finish_plug(&plug);
> -
>  	if (gc_type == FG_GC) {
>  		while (start_segno < end_segno)
>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0)
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 7b58bfb..eff046a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
>  			excess_prefree_segs(sbi) ||
>  			excess_dirty_nats(sbi) ||
>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> -		if (test_opt(sbi, DATA_FLUSH)) {
> -			struct blk_plug plug;
> -
> -			blk_start_plug(&plug);
> +		if (test_opt(sbi, DATA_FLUSH))
>  			sync_dirty_inodes(sbi, FILE_INODE);
> -			blk_finish_plug(&plug);
> -		}
>  		f2fs_sync_fs(sbi->sb, true);
>  		stat_inc_bg_cp_count(sbi->stat_info);
>  	}
> 

Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[Jaegeuk Kim]

On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2016/6/9 1:24, Jaegeuk Kim wrote:
> > In f2fs, we don't need to keep block plugging for NODE and DATA writes, since
> > we already merged bios as much as possible.
> 
> IMO, we can not remove block plug, this is because there are still many
> conditions which stops us merging r/w IOs into one bio as we expect,
> theoretically, block plug can hold bios as much as possible, then submitting
> them into queue in batch, it will reduce racing of grabbing queue->lock during
> bio submitting, if we drop them, when syncing nodes or flushing datas, we will
> suffer more lock racing.
> 
> Or there are something I am missing, do you suffer any performance issue on
> block plug?

In the latest patch, I've turned off plugging forcefully, only if the underlying
device is SMR drive.
And, still I removed other block plugging, since I couldn't see any performance
regression. Even in some workloads, I could have seen some inverted IOs due to
race condition between plugged and unplugged IOs.

Thanks,

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/checkpoint.c |  4 ----
> >  fs/f2fs/data.c       | 17 ++++++++++-------
> >  fs/f2fs/gc.c         |  5 -----
> >  fs/f2fs/segment.c    |  7 +------
> >  4 files changed, 11 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index 5ddd15c..4179c7b 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
> >  		.nr_to_write = LONG_MAX,
> >  		.for_reclaim = 0,
> >  	};
> > -	struct blk_plug plug;
> >  	int err = 0;
> >  
> > -	blk_start_plug(&plug);
> > -
> >  retry_flush_dents:
> >  	f2fs_lock_all(sbi);
> >  	/* write all the dirty dentry pages */
> > @@ -938,7 +935,6 @@ retry_flush_nodes:
> >  		goto retry_flush_nodes;
> >  	}
> >  out:
> > -	blk_finish_plug(&plug);
> >  	return err;
> >  }
> >  
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 30dc448..5f655d0 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
> >  }
> >  
> >  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> > -						struct bio *bio)
> > +			struct bio *bio, enum page_type type)
> >  {
> > -	if (!is_read_io(rw))
> > +	if (!is_read_io(rw)) {
> >  		atomic_inc(&sbi->nr_wb_bios);
> > +		if (current->plug && (type == DATA || type == NODE))
> > +			blk_finish_plug(current->plug);
> > +	}
> >  	submit_bio(rw, bio);
> >  }
> >  
> > @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> >  	else
> >  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
> >  
> > -	__submit_bio(io->sbi, fio->rw, io->bio);
> > +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
> >  	io->bio = NULL;
> >  }
> >  
> > @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> >  		return -EFAULT;
> >  	}
> >  
> > -	__submit_bio(fio->sbi, fio->rw, bio);
> > +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
> >  	return 0;
> >  }
> >  
> > @@ -1040,7 +1043,7 @@ got_it:
> >  		 */
> >  		if (bio && (last_block_in_bio != block_nr - 1)) {
> >  submit_and_realloc:
> > -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >  			bio = NULL;
> >  		}
> >  		if (bio == NULL) {
> > @@ -1083,7 +1086,7 @@ set_error_page:
> >  		goto next_page;
> >  confused:
> >  		if (bio) {
> > -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >  			bio = NULL;
> >  		}
> >  		unlock_page(page);
> > @@ -1093,7 +1096,7 @@ next_page:
> >  	}
> >  	BUG_ON(pages && !list_empty(pages));
> >  	if (bio)
> > -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> > +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >  	return 0;
> >  }
> >  
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 4a03076..67fd285 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -777,7 +777,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >  {
> >  	struct page *sum_page;
> >  	struct f2fs_summary_block *sum;
> > -	struct blk_plug plug;
> >  	unsigned int segno = start_segno;
> >  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
> >  	int seg_freed = 0;
> > @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >  		unlock_page(sum_page);
> >  	}
> >  
> > -	blk_start_plug(&plug);
> > -
> >  	for (segno = start_segno; segno < end_segno; segno++) {
> >  		/* find segment summary of victim */
> >  		sum_page = find_get_page(META_MAPPING(sbi),
> > @@ -830,8 +827,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >  		f2fs_submit_merged_bio(sbi,
> >  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
> >  
> > -	blk_finish_plug(&plug);
> > -
> >  	if (gc_type == FG_GC) {
> >  		while (start_segno < end_segno)
> >  			if (get_valid_blocks(sbi, start_segno++, 1) == 0)
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 7b58bfb..eff046a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
> >  			excess_prefree_segs(sbi) ||
> >  			excess_dirty_nats(sbi) ||
> >  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> > -		if (test_opt(sbi, DATA_FLUSH)) {
> > -			struct blk_plug plug;
> > -
> > -			blk_start_plug(&plug);
> > +		if (test_opt(sbi, DATA_FLUSH))
> >  			sync_dirty_inodes(sbi, FILE_INODE);
> > -			blk_finish_plug(&plug);
> > -		}
> >  		f2fs_sync_fs(sbi->sb, true);
> >  		stat_inc_bg_cp_count(sbi->stat_info);
> >  	}
> > 

Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[Chao Yu]

On 2016/7/10 0:32, Jaegeuk Kim wrote:
> On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2016/6/9 1:24, Jaegeuk Kim wrote:
>>> In f2fs, we don't need to keep block plugging for NODE and DATA writes, since
>>> we already merged bios as much as possible.
>>
>> IMO, we can not remove block plug, this is because there are still many
>> conditions which stops us merging r/w IOs into one bio as we expect,
>> theoretically, block plug can hold bios as much as possible, then submitting
>> them into queue in batch, it will reduce racing of grabbing queue->lock during
>> bio submitting, if we drop them, when syncing nodes or flushing datas, we will
>> suffer more lock racing.
>>
>> Or there are something I am missing, do you suffer any performance issue on
>> block plug?
> 
> In the latest patch, I've turned off plugging forcefully, only if the underlying
> device is SMR drive.

Got it.

> And, still I removed other block plugging, since I couldn't see any performance
> regression.

I suspect that in low-end machine with single-queue which is used in block
layer, we will suffer regression.

> Even in some workloads, I could have seen some inverted IOs due to
> race condition between plugged and unplugged IOs.

For data path, what about enabling block plug for IPU/SSR ?

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/checkpoint.c |  4 ----
>>>  fs/f2fs/data.c       | 17 ++++++++++-------
>>>  fs/f2fs/gc.c         |  5 -----
>>>  fs/f2fs/segment.c    |  7 +------
>>>  4 files changed, 11 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 5ddd15c..4179c7b 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
>>>  		.nr_to_write = LONG_MAX,
>>>  		.for_reclaim = 0,
>>>  	};
>>> -	struct blk_plug plug;
>>>  	int err = 0;
>>>  
>>> -	blk_start_plug(&plug);
>>> -
>>>  retry_flush_dents:
>>>  	f2fs_lock_all(sbi);
>>>  	/* write all the dirty dentry pages */
>>> @@ -938,7 +935,6 @@ retry_flush_nodes:
>>>  		goto retry_flush_nodes;
>>>  	}
>>>  out:
>>> -	blk_finish_plug(&plug);
>>>  	return err;
>>>  }
>>>  
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 30dc448..5f655d0 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
>>>  }
>>>  
>>>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
>>> -						struct bio *bio)
>>> +			struct bio *bio, enum page_type type)
>>>  {
>>> -	if (!is_read_io(rw))
>>> +	if (!is_read_io(rw)) {
>>>  		atomic_inc(&sbi->nr_wb_bios);
>>> +		if (current->plug && (type == DATA || type == NODE))
>>> +			blk_finish_plug(current->plug);
>>> +	}
>>>  	submit_bio(rw, bio);
>>>  }
>>>  
>>> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
>>>  	else
>>>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
>>>  
>>> -	__submit_bio(io->sbi, fio->rw, io->bio);
>>> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
>>>  	io->bio = NULL;
>>>  }
>>>  
>>> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
>>>  		return -EFAULT;
>>>  	}
>>>  
>>> -	__submit_bio(fio->sbi, fio->rw, bio);
>>> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1040,7 +1043,7 @@ got_it:
>>>  		 */
>>>  		if (bio && (last_block_in_bio != block_nr - 1)) {
>>>  submit_and_realloc:
>>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
>>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>>>  			bio = NULL;
>>>  		}
>>>  		if (bio == NULL) {
>>> @@ -1083,7 +1086,7 @@ set_error_page:
>>>  		goto next_page;
>>>  confused:
>>>  		if (bio) {
>>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
>>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>>>  			bio = NULL;
>>>  		}
>>>  		unlock_page(page);
>>> @@ -1093,7 +1096,7 @@ next_page:
>>>  	}
>>>  	BUG_ON(pages && !list_empty(pages));
>>>  	if (bio)
>>> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
>>> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 4a03076..67fd285 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>>  {
>>>  	struct page *sum_page;
>>>  	struct f2fs_summary_block *sum;
>>> -	struct blk_plug plug;
>>>  	unsigned int segno = start_segno;
>>>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
>>>  	int seg_freed = 0;
>>> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>>  		unlock_page(sum_page);
>>>  	}
>>>  
>>> -	blk_start_plug(&plug);
>>> -
>>>  	for (segno = start_segno; segno < end_segno; segno++) {
>>>  		/* find segment summary of victim */
>>>  		sum_page = find_get_page(META_MAPPING(sbi),
>>> @@ -830,8 +827,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>>  		f2fs_submit_merged_bio(sbi,
>>>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
>>>  
>>> -	blk_finish_plug(&plug);
>>> -
>>>  	if (gc_type == FG_GC) {
>>>  		while (start_segno < end_segno)
>>>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0)
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 7b58bfb..eff046a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
>>>  			excess_prefree_segs(sbi) ||
>>>  			excess_dirty_nats(sbi) ||
>>>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
>>> -		if (test_opt(sbi, DATA_FLUSH)) {
>>> -			struct blk_plug plug;
>>> -
>>> -			blk_start_plug(&plug);
>>> +		if (test_opt(sbi, DATA_FLUSH))
>>>  			sync_dirty_inodes(sbi, FILE_INODE);
>>> -			blk_finish_plug(&plug);
>>> -		}
>>>  		f2fs_sync_fs(sbi->sb, true);
>>>  		stat_inc_bg_cp_count(sbi->stat_info);
>>>  	}
>>>
> 
> .
> 

Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[Jaegeuk Kim]

Hi Chao,

On Tue, Jul 12, 2016 at 09:38:11AM +0800, Chao Yu wrote:
> On 2016/7/10 0:32, Jaegeuk Kim wrote:
> > On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
> >> Hi Jaegeuk,
> >>
> >> On 2016/6/9 1:24, Jaegeuk Kim wrote:
> >>> In f2fs, we don't need to keep block plugging for NODE and DATA writes, since
> >>> we already merged bios as much as possible.
> >>
> >> IMO, we can not remove block plug, this is because there are still many
> >> conditions which stops us merging r/w IOs into one bio as we expect,
> >> theoretically, block plug can hold bios as much as possible, then submitting
> >> them into queue in batch, it will reduce racing of grabbing queue->lock during
> >> bio submitting, if we drop them, when syncing nodes or flushing datas, we will
> >> suffer more lock racing.
> >>
> >> Or there are something I am missing, do you suffer any performance issue on
> >> block plug?
> > 
> > In the latest patch, I've turned off plugging forcefully, only if the underlying
> > device is SMR drive.
> 
> Got it.
> 
> > And, still I removed other block plugging, since I couldn't see any performance
> > regression.
> 
> I suspect that in low-end machine with single-queue which is used in block
> layer, we will suffer regression.
> 
> > Even in some workloads, I could have seen some inverted IOs due to
> > race condition between plugged and unplugged IOs.
> 
> For data path, what about enabling block plug for IPU/SSR ?

Not sure. IPU and SSR will produce small (likely random) writes.
What I'm seeing here is that we already try to merge bios as much as possible.
Thus, I'm in doubt why we need to wait for merging them by block layer.
If possible, could you check this in android?

Thanks,

> >>>
> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>> ---
> >>>  fs/f2fs/checkpoint.c |  4 ----
> >>>  fs/f2fs/data.c       | 17 ++++++++++-------
> >>>  fs/f2fs/gc.c         |  5 -----
> >>>  fs/f2fs/segment.c    |  7 +------
> >>>  4 files changed, 11 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >>> index 5ddd15c..4179c7b 100644
> >>> --- a/fs/f2fs/checkpoint.c
> >>> +++ b/fs/f2fs/checkpoint.c
> >>> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
> >>>  		.nr_to_write = LONG_MAX,
> >>>  		.for_reclaim = 0,
> >>>  	};
> >>> -	struct blk_plug plug;
> >>>  	int err = 0;
> >>>  
> >>> -	blk_start_plug(&plug);
> >>> -
> >>>  retry_flush_dents:
> >>>  	f2fs_lock_all(sbi);
> >>>  	/* write all the dirty dentry pages */
> >>> @@ -938,7 +935,6 @@ retry_flush_nodes:
> >>>  		goto retry_flush_nodes;
> >>>  	}
> >>>  out:
> >>> -	blk_finish_plug(&plug);
> >>>  	return err;
> >>>  }
> >>>  
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >>> index 30dc448..5f655d0 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
> >>>  }
> >>>  
> >>>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> >>> -						struct bio *bio)
> >>> +			struct bio *bio, enum page_type type)
> >>>  {
> >>> -	if (!is_read_io(rw))
> >>> +	if (!is_read_io(rw)) {
> >>>  		atomic_inc(&sbi->nr_wb_bios);
> >>> +		if (current->plug && (type == DATA || type == NODE))
> >>> +			blk_finish_plug(current->plug);
> >>> +	}
> >>>  	submit_bio(rw, bio);
> >>>  }
> >>>  
> >>> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> >>>  	else
> >>>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
> >>>  
> >>> -	__submit_bio(io->sbi, fio->rw, io->bio);
> >>> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
> >>>  	io->bio = NULL;
> >>>  }
> >>>  
> >>> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> >>>  		return -EFAULT;
> >>>  	}
> >>>  
> >>> -	__submit_bio(fio->sbi, fio->rw, bio);
> >>> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> @@ -1040,7 +1043,7 @@ got_it:
> >>>  		 */
> >>>  		if (bio && (last_block_in_bio != block_nr - 1)) {
> >>>  submit_and_realloc:
> >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  			bio = NULL;
> >>>  		}
> >>>  		if (bio == NULL) {
> >>> @@ -1083,7 +1086,7 @@ set_error_page:
> >>>  		goto next_page;
> >>>  confused:
> >>>  		if (bio) {
> >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  			bio = NULL;
> >>>  		}
> >>>  		unlock_page(page);
> >>> @@ -1093,7 +1096,7 @@ next_page:
> >>>  	}
> >>>  	BUG_ON(pages && !list_empty(pages));
> >>>  	if (bio)
> >>> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>> index 4a03076..67fd285 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>>  {
> >>>  	struct page *sum_page;
> >>>  	struct f2fs_summary_block *sum;
> >>> -	struct blk_plug plug;
> >>>  	unsigned int segno = start_segno;
> >>>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
> >>>  	int seg_freed = 0;
> >>> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>>  		unlock_page(sum_page);
> >>>  	}
> >>>  
> >>> -	blk_start_plug(&plug);
> >>> -
> >>>  	for (segno = start_segno; segno < end_segno; segno++) {
> >>>  		/* find segment summary of victim */
> >>>  		sum_page = find_get_page(META_MAPPING(sbi),
> >>> @@ -830,8 +827,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>>  		f2fs_submit_merged_bio(sbi,
> >>>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
> >>>  
> >>> -	blk_finish_plug(&plug);
> >>> -
> >>>  	if (gc_type == FG_GC) {
> >>>  		while (start_segno < end_segno)
> >>>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0)
> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>> index 7b58bfb..eff046a 100644
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
> >>>  			excess_prefree_segs(sbi) ||
> >>>  			excess_dirty_nats(sbi) ||
> >>>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> >>> -		if (test_opt(sbi, DATA_FLUSH)) {
> >>> -			struct blk_plug plug;
> >>> -
> >>> -			blk_start_plug(&plug);
> >>> +		if (test_opt(sbi, DATA_FLUSH))
> >>>  			sync_dirty_inodes(sbi, FILE_INODE);
> >>> -			blk_finish_plug(&plug);
> >>> -		}
> >>>  		f2fs_sync_fs(sbi->sb, true);
> >>>  		stat_inc_bg_cp_count(sbi->stat_info);
> >>>  	}
> >>>
> > 
> > .
> > 

RE: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[hebiao (G)]

Hi, Kim,

	You are right. If file system can merge bios as much as possible. It's very helpful to block layer. But plugging mechanism has a precognition of IO stream except merging bios. For example, we can out the low-power mode in advance when you start a plug and we can in the low-power mode only when you end a plug to avoid in-out low-power mode frequently. So I want to know if there is any side effect in F2FS to reserve the plug mechanism ?

-----Original Message-----
From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] 
Sent: 2016年7月13日 1:08
To: Yuchao (T) <yuchao0@huawei.com>
Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; CHEN CHUN YEN (IAN) <chen.chun.yen@huawei.com>; hebiao (G) <hebiao6@huawei.com>
Subject: Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

Hi Chao,

On Tue, Jul 12, 2016 at 09:38:11AM +0800, Chao Yu wrote:
> On 2016/7/10 0:32, Jaegeuk Kim wrote:
> > On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
> >> Hi Jaegeuk,
> >>
> >> On 2016/6/9 1:24, Jaegeuk Kim wrote:
> >>> In f2fs, we don't need to keep block plugging for NODE and DATA 
> >>> writes, since we already merged bios as much as possible.
> >>
> >> IMO, we can not remove block plug, this is because there are still 
> >> many conditions which stops us merging r/w IOs into one bio as we 
> >> expect, theoretically, block plug can hold bios as much as 
> >> possible, then submitting them into queue in batch, it will reduce 
> >> racing of grabbing queue->lock during bio submitting, if we drop 
> >> them, when syncing nodes or flushing datas, we will suffer more lock racing.
> >>
> >> Or there are something I am missing, do you suffer any performance 
> >> issue on block plug?
> > 
> > In the latest patch, I've turned off plugging forcefully, only if 
> > the underlying device is SMR drive.
> 
> Got it.
> 
> > And, still I removed other block plugging, since I couldn't see any 
> > performance regression.
> 
> I suspect that in low-end machine with single-queue which is used in 
> block layer, we will suffer regression.
> 
> > Even in some workloads, I could have seen some inverted IOs due to 
> > race condition between plugged and unplugged IOs.
> 
> For data path, what about enabling block plug for IPU/SSR ?

Not sure. IPU and SSR will produce small (likely random) writes.
What I'm seeing here is that we already try to merge bios as much as possible.
Thus, I'm in doubt why we need to wait for merging them by block layer.
If possible, could you check this in android?

Thanks,

> >>>
> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>> ---
> >>>  fs/f2fs/checkpoint.c |  4 ----
> >>>  fs/f2fs/data.c       | 17 ++++++++++-------
> >>>  fs/f2fs/gc.c         |  5 -----
> >>>  fs/f2fs/segment.c    |  7 +------
> >>>  4 files changed, 11 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 
> >>> 5ddd15c..4179c7b 100644
> >>> --- a/fs/f2fs/checkpoint.c
> >>> +++ b/fs/f2fs/checkpoint.c
> >>> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
> >>>  		.nr_to_write = LONG_MAX,
> >>>  		.for_reclaim = 0,
> >>>  	};
> >>> -	struct blk_plug plug;
> >>>  	int err = 0;
> >>>  
> >>> -	blk_start_plug(&plug);
> >>> -
> >>>  retry_flush_dents:
> >>>  	f2fs_lock_all(sbi);
> >>>  	/* write all the dirty dentry pages */ @@ -938,7 +935,6 @@ 
> >>> retry_flush_nodes:
> >>>  		goto retry_flush_nodes;
> >>>  	}
> >>>  out:
> >>> -	blk_finish_plug(&plug);
> >>>  	return err;
> >>>  }
> >>>  
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 
> >>> 30dc448..5f655d0 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct 
> >>> f2fs_sb_info *sbi, block_t blk_addr,  }
> >>>  
> >>>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> >>> -						struct bio *bio)
> >>> +			struct bio *bio, enum page_type type)
> >>>  {
> >>> -	if (!is_read_io(rw))
> >>> +	if (!is_read_io(rw)) {
> >>>  		atomic_inc(&sbi->nr_wb_bios);
> >>> +		if (current->plug && (type == DATA || type == NODE))
> >>> +			blk_finish_plug(current->plug);
> >>> +	}
> >>>  	submit_bio(rw, bio);
> >>>  }
> >>>  
> >>> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> >>>  	else
> >>>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
> >>>  
> >>> -	__submit_bio(io->sbi, fio->rw, io->bio);
> >>> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
> >>>  	io->bio = NULL;
> >>>  }
> >>>  
> >>> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> >>>  		return -EFAULT;
> >>>  	}
> >>>  
> >>> -	__submit_bio(fio->sbi, fio->rw, bio);
> >>> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> @@ -1040,7 +1043,7 @@ got_it:
> >>>  		 */
> >>>  		if (bio && (last_block_in_bio != block_nr - 1)) {
> >>>  submit_and_realloc:
> >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  			bio = NULL;
> >>>  		}
> >>>  		if (bio == NULL) {
> >>> @@ -1083,7 +1086,7 @@ set_error_page:
> >>>  		goto next_page;
> >>>  confused:
> >>>  		if (bio) {
> >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  			bio = NULL;
> >>>  		}
> >>>  		unlock_page(page);
> >>> @@ -1093,7 +1096,7 @@ next_page:
> >>>  	}
> >>>  	BUG_ON(pages && !list_empty(pages));
> >>>  	if (bio)
> >>> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> >>> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> >>>  	return 0;
> >>>  }
> >>>  
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 4a03076..67fd285 
> >>> 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct 
> >>> f2fs_sb_info *sbi,  {
> >>>  	struct page *sum_page;
> >>>  	struct f2fs_summary_block *sum;
> >>> -	struct blk_plug plug;
> >>>  	unsigned int segno = start_segno;
> >>>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
> >>>  	int seg_freed = 0;
> >>> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>>  		unlock_page(sum_page);
> >>>  	}
> >>>  
> >>> -	blk_start_plug(&plug);
> >>> -
> >>>  	for (segno = start_segno; segno < end_segno; segno++) {
> >>>  		/* find segment summary of victim */
> >>>  		sum_page = find_get_page(META_MAPPING(sbi), @@ -830,8 +827,6 @@ 
> >>> static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>>  		f2fs_submit_merged_bio(sbi,
> >>>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
> >>>  
> >>> -	blk_finish_plug(&plug);
> >>> -
> >>>  	if (gc_type == FG_GC) {
> >>>  		while (start_segno < end_segno)
> >>>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0) diff --git 
> >>> a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 7b58bfb..eff046a 
> >>> 100644
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
> >>>  			excess_prefree_segs(sbi) ||
> >>>  			excess_dirty_nats(sbi) ||
> >>>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> >>> -		if (test_opt(sbi, DATA_FLUSH)) {
> >>> -			struct blk_plug plug;
> >>> -
> >>> -			blk_start_plug(&plug);
> >>> +		if (test_opt(sbi, DATA_FLUSH))
> >>>  			sync_dirty_inodes(sbi, FILE_INODE);
> >>> -			blk_finish_plug(&plug);
> >>> -		}
> >>>  		f2fs_sync_fs(sbi->sb, true);
> >>>  		stat_inc_bg_cp_count(sbi->stat_info);
> >>>  	}
> >>>
> > 
> > .
> > 

Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[Jaegeuk Kim]

Hi hebiao,

On Wed, Jul 13, 2016 at 01:21:15AM +0000, hebiao (G) wrote:
> Hi, Kim,
> 
> 	You are right. If file system can merge bios as much as possible. It's very helpful to block layer. But plugging mechanism has a precognition of IO stream except merging bios. For example, we can out the low-power mode in advance when you start a plug and we can in the low-power mode only when you end a plug to avoid in-out low-power mode frequently. So I want to know if there is any side effect in F2FS to reserve the plug mechanism ?

Oh, does android kernel act like that?
Nevertheless, I'll take a look at any performance regression if I add plugs in
all the IO submission again.

Thanks,

> 
> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] 
> Sent: 2016年7月13日 1:08
> To: Yuchao (T) <yuchao0@huawei.com>
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; CHEN CHUN YEN (IAN) <chen.chun.yen@huawei.com>; hebiao (G) <hebiao6@huawei.com>
> Subject: Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging
> 
> Hi Chao,
> 
> On Tue, Jul 12, 2016 at 09:38:11AM +0800, Chao Yu wrote:
> > On 2016/7/10 0:32, Jaegeuk Kim wrote:
> > > On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
> > >> Hi Jaegeuk,
> > >>
> > >> On 2016/6/9 1:24, Jaegeuk Kim wrote:
> > >>> In f2fs, we don't need to keep block plugging for NODE and DATA 
> > >>> writes, since we already merged bios as much as possible.
> > >>
> > >> IMO, we can not remove block plug, this is because there are still 
> > >> many conditions which stops us merging r/w IOs into one bio as we 
> > >> expect, theoretically, block plug can hold bios as much as 
> > >> possible, then submitting them into queue in batch, it will reduce 
> > >> racing of grabbing queue->lock during bio submitting, if we drop 
> > >> them, when syncing nodes or flushing datas, we will suffer more lock racing.
> > >>
> > >> Or there are something I am missing, do you suffer any performance 
> > >> issue on block plug?
> > > 
> > > In the latest patch, I've turned off plugging forcefully, only if 
> > > the underlying device is SMR drive.
> > 
> > Got it.
> > 
> > > And, still I removed other block plugging, since I couldn't see any 
> > > performance regression.
> > 
> > I suspect that in low-end machine with single-queue which is used in 
> > block layer, we will suffer regression.
> > 
> > > Even in some workloads, I could have seen some inverted IOs due to 
> > > race condition between plugged and unplugged IOs.
> > 
> > For data path, what about enabling block plug for IPU/SSR ?
> 
> Not sure. IPU and SSR will produce small (likely random) writes.
> What I'm seeing here is that we already try to merge bios as much as possible.
> Thus, I'm in doubt why we need to wait for merging them by block layer.
> If possible, could you check this in android?
> 
> Thanks,
> 
> > >>>
> > >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > >>> ---
> > >>>  fs/f2fs/checkpoint.c |  4 ----
> > >>>  fs/f2fs/data.c       | 17 ++++++++++-------
> > >>>  fs/f2fs/gc.c         |  5 -----
> > >>>  fs/f2fs/segment.c    |  7 +------
> > >>>  4 files changed, 11 insertions(+), 22 deletions(-)
> > >>>
> > >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 
> > >>> 5ddd15c..4179c7b 100644
> > >>> --- a/fs/f2fs/checkpoint.c
> > >>> +++ b/fs/f2fs/checkpoint.c
> > >>> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
> > >>>  		.nr_to_write = LONG_MAX,
> > >>>  		.for_reclaim = 0,
> > >>>  	};
> > >>> -	struct blk_plug plug;
> > >>>  	int err = 0;
> > >>>  
> > >>> -	blk_start_plug(&plug);
> > >>> -
> > >>>  retry_flush_dents:
> > >>>  	f2fs_lock_all(sbi);
> > >>>  	/* write all the dirty dentry pages */ @@ -938,7 +935,6 @@ 
> > >>> retry_flush_nodes:
> > >>>  		goto retry_flush_nodes;
> > >>>  	}
> > >>>  out:
> > >>> -	blk_finish_plug(&plug);
> > >>>  	return err;
> > >>>  }
> > >>>  
> > >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 
> > >>> 30dc448..5f655d0 100644
> > >>> --- a/fs/f2fs/data.c
> > >>> +++ b/fs/f2fs/data.c
> > >>> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct 
> > >>> f2fs_sb_info *sbi, block_t blk_addr,  }
> > >>>  
> > >>>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> > >>> -						struct bio *bio)
> > >>> +			struct bio *bio, enum page_type type)
> > >>>  {
> > >>> -	if (!is_read_io(rw))
> > >>> +	if (!is_read_io(rw)) {
> > >>>  		atomic_inc(&sbi->nr_wb_bios);
> > >>> +		if (current->plug && (type == DATA || type == NODE))
> > >>> +			blk_finish_plug(current->plug);
> > >>> +	}
> > >>>  	submit_bio(rw, bio);
> > >>>  }
> > >>>  
> > >>> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> > >>>  	else
> > >>>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
> > >>>  
> > >>> -	__submit_bio(io->sbi, fio->rw, io->bio);
> > >>> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
> > >>>  	io->bio = NULL;
> > >>>  }
> > >>>  
> > >>> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > >>>  		return -EFAULT;
> > >>>  	}
> > >>>  
> > >>> -	__submit_bio(fio->sbi, fio->rw, bio);
> > >>> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
> > >>>  	return 0;
> > >>>  }
> > >>>  
> > >>> @@ -1040,7 +1043,7 @@ got_it:
> > >>>  		 */
> > >>>  		if (bio && (last_block_in_bio != block_nr - 1)) {
> > >>>  submit_and_realloc:
> > >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  			bio = NULL;
> > >>>  		}
> > >>>  		if (bio == NULL) {
> > >>> @@ -1083,7 +1086,7 @@ set_error_page:
> > >>>  		goto next_page;
> > >>>  confused:
> > >>>  		if (bio) {
> > >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  			bio = NULL;
> > >>>  		}
> > >>>  		unlock_page(page);
> > >>> @@ -1093,7 +1096,7 @@ next_page:
> > >>>  	}
> > >>>  	BUG_ON(pages && !list_empty(pages));
> > >>>  	if (bio)
> > >>> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  	return 0;
> > >>>  }
> > >>>  
> > >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 4a03076..67fd285 
> > >>> 100644
> > >>> --- a/fs/f2fs/gc.c
> > >>> +++ b/fs/f2fs/gc.c
> > >>> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct 
> > >>> f2fs_sb_info *sbi,  {
> > >>>  	struct page *sum_page;
> > >>>  	struct f2fs_summary_block *sum;
> > >>> -	struct blk_plug plug;
> > >>>  	unsigned int segno = start_segno;
> > >>>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
> > >>>  	int seg_freed = 0;
> > >>> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> > >>>  		unlock_page(sum_page);
> > >>>  	}
> > >>>  
> > >>> -	blk_start_plug(&plug);
> > >>> -
> > >>>  	for (segno = start_segno; segno < end_segno; segno++) {
> > >>>  		/* find segment summary of victim */
> > >>>  		sum_page = find_get_page(META_MAPPING(sbi), @@ -830,8 +827,6 @@ 
> > >>> static int do_garbage_collect(struct f2fs_sb_info *sbi,
> > >>>  		f2fs_submit_merged_bio(sbi,
> > >>>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
> > >>>  
> > >>> -	blk_finish_plug(&plug);
> > >>> -
> > >>>  	if (gc_type == FG_GC) {
> > >>>  		while (start_segno < end_segno)
> > >>>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0) diff --git 
> > >>> a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 7b58bfb..eff046a 
> > >>> 100644
> > >>> --- a/fs/f2fs/segment.c
> > >>> +++ b/fs/f2fs/segment.c
> > >>> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
> > >>>  			excess_prefree_segs(sbi) ||
> > >>>  			excess_dirty_nats(sbi) ||
> > >>>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> > >>> -		if (test_opt(sbi, DATA_FLUSH)) {
> > >>> -			struct blk_plug plug;
> > >>> -
> > >>> -			blk_start_plug(&plug);
> > >>> +		if (test_opt(sbi, DATA_FLUSH))
> > >>>  			sync_dirty_inodes(sbi, FILE_INODE);
> > >>> -			blk_finish_plug(&plug);
> > >>> -		}
> > >>>  		f2fs_sync_fs(sbi->sb, true);
> > >>>  		stat_inc_bg_cp_count(sbi->stat_info);
> > >>>  	}
> > >>>
> > > 
> > > .
> > > 

RE: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

[hebiao (G)]

Thanks Kim, it'll be very meaningful in block layer :-)

-----Original Message-----
From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] 
Sent: 2016年7月14日 10:40
To: hebiao (G) <hebiao6@huawei.com>
Cc: Yuchao (T) <yuchao0@huawei.com>; linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; CHEN CHUN YEN (IAN) <chen.chun.yen@huawei.com>; Kongfei <kongfei@hisilicon.com>
Subject: Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging

Hi hebiao,

On Wed, Jul 13, 2016 at 01:21:15AM +0000, hebiao (G) wrote:
> Hi, Kim,
> 
> 	You are right. If file system can merge bios as much as possible. It's very helpful to block layer. But plugging mechanism has a precognition of IO stream except merging bios. For example, we can out the low-power mode in advance when you start a plug and we can in the low-power mode only when you end a plug to avoid in-out low-power mode frequently. So I want to know if there is any side effect in F2FS to reserve the plug mechanism ?

Oh, does android kernel act like that?
Nevertheless, I'll take a look at any performance regression if I add plugs in all the IO submission again.

Thanks,

> 
> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: 2016年7月13日 1:08
> To: Yuchao (T) <yuchao0@huawei.com>
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; 
> linux-f2fs-devel@lists.sourceforge.net; CHEN CHUN YEN (IAN) 
> <chen.chun.yen@huawei.com>; hebiao (G) <hebiao6@huawei.com>
> Subject: Re: [f2fs-dev] [PATCH 3/7] f2fs: drop any block plugging
> 
> Hi Chao,
> 
> On Tue, Jul 12, 2016 at 09:38:11AM +0800, Chao Yu wrote:
> > On 2016/7/10 0:32, Jaegeuk Kim wrote:
> > > On Sat, Jul 09, 2016 at 10:28:49AM +0800, Chao Yu wrote:
> > >> Hi Jaegeuk,
> > >>
> > >> On 2016/6/9 1:24, Jaegeuk Kim wrote:
> > >>> In f2fs, we don't need to keep block plugging for NODE and DATA 
> > >>> writes, since we already merged bios as much as possible.
> > >>
> > >> IMO, we can not remove block plug, this is because there are 
> > >> still many conditions which stops us merging r/w IOs into one bio 
> > >> as we expect, theoretically, block plug can hold bios as much as 
> > >> possible, then submitting them into queue in batch, it will 
> > >> reduce racing of grabbing queue->lock during bio submitting, if 
> > >> we drop them, when syncing nodes or flushing datas, we will suffer more lock racing.
> > >>
> > >> Or there are something I am missing, do you suffer any 
> > >> performance issue on block plug?
> > > 
> > > In the latest patch, I've turned off plugging forcefully, only if 
> > > the underlying device is SMR drive.
> > 
> > Got it.
> > 
> > > And, still I removed other block plugging, since I couldn't see 
> > > any performance regression.
> > 
> > I suspect that in low-end machine with single-queue which is used in 
> > block layer, we will suffer regression.
> > 
> > > Even in some workloads, I could have seen some inverted IOs due to 
> > > race condition between plugged and unplugged IOs.
> > 
> > For data path, what about enabling block plug for IPU/SSR ?
> 
> Not sure. IPU and SSR will produce small (likely random) writes.
> What I'm seeing here is that we already try to merge bios as much as possible.
> Thus, I'm in doubt why we need to wait for merging them by block layer.
> If possible, could you check this in android?
> 
> Thanks,
> 
> > >>>
> > >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > >>> ---
> > >>>  fs/f2fs/checkpoint.c |  4 ----
> > >>>  fs/f2fs/data.c       | 17 ++++++++++-------
> > >>>  fs/f2fs/gc.c         |  5 -----
> > >>>  fs/f2fs/segment.c    |  7 +------
> > >>>  4 files changed, 11 insertions(+), 22 deletions(-)
> > >>>
> > >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index 
> > >>> 5ddd15c..4179c7b 100644
> > >>> --- a/fs/f2fs/checkpoint.c
> > >>> +++ b/fs/f2fs/checkpoint.c
> > >>> @@ -897,11 +897,8 @@ static int block_operations(struct f2fs_sb_info *sbi)
> > >>>  		.nr_to_write = LONG_MAX,
> > >>>  		.for_reclaim = 0,
> > >>>  	};
> > >>> -	struct blk_plug plug;
> > >>>  	int err = 0;
> > >>>  
> > >>> -	blk_start_plug(&plug);
> > >>> -
> > >>>  retry_flush_dents:
> > >>>  	f2fs_lock_all(sbi);
> > >>>  	/* write all the dirty dentry pages */ @@ -938,7 +935,6 @@
> > >>> retry_flush_nodes:
> > >>>  		goto retry_flush_nodes;
> > >>>  	}
> > >>>  out:
> > >>> -	blk_finish_plug(&plug);
> > >>>  	return err;
> > >>>  }
> > >>>  
> > >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
> > >>> 30dc448..5f655d0 100644
> > >>> --- a/fs/f2fs/data.c
> > >>> +++ b/fs/f2fs/data.c
> > >>> @@ -98,10 +98,13 @@ static struct bio *__bio_alloc(struct 
> > >>> f2fs_sb_info *sbi, block_t blk_addr,  }
> > >>>  
> > >>>  static inline void __submit_bio(struct f2fs_sb_info *sbi, int rw,
> > >>> -						struct bio *bio)
> > >>> +			struct bio *bio, enum page_type type)
> > >>>  {
> > >>> -	if (!is_read_io(rw))
> > >>> +	if (!is_read_io(rw)) {
> > >>>  		atomic_inc(&sbi->nr_wb_bios);
> > >>> +		if (current->plug && (type == DATA || type == NODE))
> > >>> +			blk_finish_plug(current->plug);
> > >>> +	}
> > >>>  	submit_bio(rw, bio);
> > >>>  }
> > >>>  
> > >>> @@ -117,7 +120,7 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> > >>>  	else
> > >>>  		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
> > >>>  
> > >>> -	__submit_bio(io->sbi, fio->rw, io->bio);
> > >>> +	__submit_bio(io->sbi, fio->rw, io->bio, fio->type);
> > >>>  	io->bio = NULL;
> > >>>  }
> > >>>  
> > >>> @@ -235,7 +238,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > >>>  		return -EFAULT;
> > >>>  	}
> > >>>  
> > >>> -	__submit_bio(fio->sbi, fio->rw, bio);
> > >>> +	__submit_bio(fio->sbi, fio->rw, bio, fio->type);
> > >>>  	return 0;
> > >>>  }
> > >>>  
> > >>> @@ -1040,7 +1043,7 @@ got_it:
> > >>>  		 */
> > >>>  		if (bio && (last_block_in_bio != block_nr - 1)) {
> > >>>  submit_and_realloc:
> > >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  			bio = NULL;
> > >>>  		}
> > >>>  		if (bio == NULL) {
> > >>> @@ -1083,7 +1086,7 @@ set_error_page:
> > >>>  		goto next_page;
> > >>>  confused:
> > >>>  		if (bio) {
> > >>> -			__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +			__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  			bio = NULL;
> > >>>  		}
> > >>>  		unlock_page(page);
> > >>> @@ -1093,7 +1096,7 @@ next_page:
> > >>>  	}
> > >>>  	BUG_ON(pages && !list_empty(pages));
> > >>>  	if (bio)
> > >>> -		__submit_bio(F2FS_I_SB(inode), READ, bio);
> > >>> +		__submit_bio(F2FS_I_SB(inode), READ, bio, DATA);
> > >>>  	return 0;
> > >>>  }
> > >>>  
> > >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 4a03076..67fd285
> > >>> 100644
> > >>> --- a/fs/f2fs/gc.c
> > >>> +++ b/fs/f2fs/gc.c
> > >>> @@ -777,7 +777,6 @@ static int do_garbage_collect(struct 
> > >>> f2fs_sb_info *sbi,  {
> > >>>  	struct page *sum_page;
> > >>>  	struct f2fs_summary_block *sum;
> > >>> -	struct blk_plug plug;
> > >>>  	unsigned int segno = start_segno;
> > >>>  	unsigned int end_segno = start_segno + sbi->segs_per_sec;
> > >>>  	int seg_freed = 0;
> > >>> @@ -795,8 +794,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> > >>>  		unlock_page(sum_page);
> > >>>  	}
> > >>>  
> > >>> -	blk_start_plug(&plug);
> > >>> -
> > >>>  	for (segno = start_segno; segno < end_segno; segno++) {
> > >>>  		/* find segment summary of victim */
> > >>>  		sum_page = find_get_page(META_MAPPING(sbi), @@ -830,8 +827,6 
> > >>> @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> > >>>  		f2fs_submit_merged_bio(sbi,
> > >>>  				(type == SUM_TYPE_NODE) ? NODE : DATA, WRITE);
> > >>>  
> > >>> -	blk_finish_plug(&plug);
> > >>> -
> > >>>  	if (gc_type == FG_GC) {
> > >>>  		while (start_segno < end_segno)
> > >>>  			if (get_valid_blocks(sbi, start_segno++, 1) == 0) diff --git 
> > >>> a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 7b58bfb..eff046a
> > >>> 100644
> > >>> --- a/fs/f2fs/segment.c
> > >>> +++ b/fs/f2fs/segment.c
> > >>> @@ -379,13 +379,8 @@ void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi)
> > >>>  			excess_prefree_segs(sbi) ||
> > >>>  			excess_dirty_nats(sbi) ||
> > >>>  			(is_idle(sbi) && f2fs_time_over(sbi, CP_TIME))) {
> > >>> -		if (test_opt(sbi, DATA_FLUSH)) {
> > >>> -			struct blk_plug plug;
> > >>> -
> > >>> -			blk_start_plug(&plug);
> > >>> +		if (test_opt(sbi, DATA_FLUSH))
> > >>>  			sync_dirty_inodes(sbi, FILE_INODE);
> > >>> -			blk_finish_plug(&plug);
> > >>> -		}
> > >>>  		f2fs_sync_fs(sbi->sb, true);
> > >>>  		stat_inc_bg_cp_count(sbi->stat_info);
> > >>>  	}
> > >>>
> > > 
> > > .
> > > 

[PATCH 4/7] f2fs: skip clean segment for gc

[Jaegeuk Kim]

If a segment in a section is clean or prefreed, we don't need to get its summary
and do gc.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/gc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 67fd285..e1d274c 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -795,6 +795,10 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
 	}
 
 	for (segno = start_segno; segno < end_segno; segno++) {
+
+		if (get_valid_blocks(sbi, segno, 1) == 0)
+			continue;
+
 		/* find segment summary of victim */
 		sum_page = find_get_page(META_MAPPING(sbi),
 					GET_SUM_BLOCK(sbi, segno));
-- 
2.8.3

[PATCH 5/7] f2fs: introduce force_lfs mount option

[Jaegeuk Kim]

This mount option is to enable original log-structured filesystem forcefully.
So, there should be no random writes for main area.

Especially, this supports host-managed SMR device.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 Documentation/filesystems/f2fs.txt |  3 +++
 fs/f2fs/data.c                     |  2 ++
 fs/f2fs/f2fs.h                     |  2 ++
 fs/f2fs/file.c                     |  8 +++++++-
 fs/f2fs/segment.c                  | 20 +++++++++++++++++++-
 fs/f2fs/segment.h                  |  7 +++++++
 fs/f2fs/super.c                    | 28 ++++++++++++++++++++++++++++
 7 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index e1c9f08..3a5ce24 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -151,6 +151,9 @@ noinline_data          Disable the inline data feature, inline data feature is
                        enabled by default.
 data_flush             Enable data flushing before checkpoint in order to
                        persist data of regular and symlink.
+mode=%s                Control block allocation mode which supports "adaptive"
+                       and "lfs". In "lfs" mode, there should be no random
+                       writes towards main area.
 
 ================================================================================
 DEBUGFS ENTRIES
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5f655d0..607ef43 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1710,6 +1710,8 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 
 	if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode))
 		return 0;
+	if (test_opt(F2FS_I_SB(inode), LFS))
+		return 0;
 
 	trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter));
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b35b68b..d919294 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -111,6 +111,8 @@ static inline bool time_to_inject(int type)
 #define F2FS_MOUNT_FORCE_FG_GC		0x00004000
 #define F2FS_MOUNT_DATA_FLUSH		0x00008000
 #define F2FS_MOUNT_FAULT_INJECTION	0x00010000
+#define F2FS_MOUNT_ADAPTIVE		0x00020000
+#define F2FS_MOUNT_LFS			0x00040000
 
 #define clear_opt(sbi, option)	(sbi->mount_opt.opt &= ~F2FS_MOUNT_##option)
 #define set_opt(sbi, option)	(sbi->mount_opt.opt |= F2FS_MOUNT_##option)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e66d223..f1f78fb 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -917,9 +917,15 @@ static int __exchange_data_block(struct inode *inode, pgoff_t src,
 		return full ? truncate_hole(inode, dst, dst + 1) : 0;
 
 	if (do_replace) {
-		struct page *ipage = get_node_page(sbi, inode->i_ino);
+		struct page *ipage;
 		struct node_info ni;
 
+		if (test_opt(sbi, LFS)) {
+			ret = -ENOTSUPP;
+			goto err_out;
+		}
+
+		ipage = get_node_page(sbi, inode->i_ino);
 		if (IS_ERR(ipage)) {
 			ret = PTR_ERR(ipage);
 			goto err_out;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index eff046a..9ff4e30 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -594,6 +594,9 @@ bool discard_next_dnode(struct f2fs_sb_info *sbi, block_t blkaddr)
 {
 	int err = -EOPNOTSUPP;
 
+	if (test_opt(sbi, LFS))
+		return false;
+
 	if (test_opt(sbi, DISCARD)) {
 		struct seg_entry *se = get_seg_entry(sbi,
 				GET_SEGNO(sbi, blkaddr));
@@ -707,6 +710,7 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
 	unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
 	unsigned int start = 0, end = -1;
+	unsigned int secno, start_segno;
 
 	mutex_lock(&dirty_i->seglist_lock);
 
@@ -726,8 +730,22 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		if (!test_opt(sbi, DISCARD))
 			continue;
 
-		f2fs_issue_discard(sbi, START_BLOCK(sbi, start),
+		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
+			f2fs_issue_discard(sbi, START_BLOCK(sbi, start),
 				(end - start) << sbi->log_blocks_per_seg);
+			continue;
+		}
+next:
+		secno = GET_SECNO(sbi, start);
+		start_segno = secno * sbi->segs_per_sec;
+		if (!IS_CURSEC(sbi, secno) &&
+			!get_valid_blocks(sbi, start, sbi->segs_per_sec))
+			f2fs_issue_discard(sbi, START_BLOCK(sbi, start_segno),
+				sbi->segs_per_sec << sbi->log_blocks_per_seg);
+
+		start = start_segno + sbi->segs_per_sec;
+		if (start < end)
+			goto next;
 	}
 	mutex_unlock(&dirty_i->seglist_lock);
 
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 890bb28d..d74cc33 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -470,6 +470,10 @@ static inline bool need_SSR(struct f2fs_sb_info *sbi)
 {
 	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
 	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
+
+	if (test_opt(sbi, LFS))
+		return false;
+
 	return free_sections(sbi) <= (node_secs + 2 * dent_secs +
 						reserved_sections(sbi) + 1);
 }
@@ -533,6 +537,9 @@ static inline bool need_inplace_update(struct inode *inode)
 	if (S_ISDIR(inode->i_mode) || f2fs_is_atomic_file(inode))
 		return false;
 
+	if (test_opt(sbi, LFS))
+		return false;
+
 	if (policy & (0x1 << F2FS_IPU_FORCE))
 		return true;
 	if (policy & (0x1 << F2FS_IPU_SSR) && need_SSR(sbi))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index dc66f16..edc736d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -94,6 +94,7 @@ enum {
 	Opt_noextent_cache,
 	Opt_noinline_data,
 	Opt_data_flush,
+	Opt_mode,
 	Opt_fault_injection,
 	Opt_lazytime,
 	Opt_nolazytime,
@@ -123,6 +124,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_noextent_cache, "noextent_cache"},
 	{Opt_noinline_data, "noinline_data"},
 	{Opt_data_flush, "data_flush"},
+	{Opt_mode, "mode=%s"},
 	{Opt_fault_injection, "fault_injection=%u"},
 	{Opt_lazytime, "lazytime"},
 	{Opt_nolazytime, "nolazytime"},
@@ -506,6 +508,25 @@ static int parse_options(struct super_block *sb, char *options)
 		case Opt_data_flush:
 			set_opt(sbi, DATA_FLUSH);
 			break;
+		case Opt_mode:
+			name = match_strdup(&args[0]);
+
+			if (!name)
+				return -ENOMEM;
+			if (strlen(name) == 8 &&
+					!strncmp(name, "adaptive", 8)) {
+				set_opt(sbi, ADAPTIVE);
+				clear_opt(sbi, LFS);
+			} else if (strlen(name) == 3 &&
+					!strncmp(name, "lfs", 3)) {
+				clear_opt(sbi, ADAPTIVE);
+				set_opt(sbi, LFS);
+			} else {
+				kfree(name);
+				return -EINVAL;
+			}
+			kfree(name);
+			break;
 		case Opt_fault_injection:
 			if (args->from && match_int(args, &arg))
 				return -EINVAL;
@@ -870,6 +891,12 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_puts(seq, ",noextent_cache");
 	if (test_opt(sbi, DATA_FLUSH))
 		seq_puts(seq, ",data_flush");
+
+	seq_puts(seq, ",mode=");
+	if (test_opt(sbi, ADAPTIVE))
+		seq_puts(seq, "adaptive");
+	else if (test_opt(sbi, LFS))
+		seq_puts(seq, "lfs");
 	seq_printf(seq, ",active_logs=%u", sbi->active_logs);
 
 	return 0;
@@ -953,6 +980,7 @@ static void default_options(struct f2fs_sb_info *sbi)
 	set_opt(sbi, EXTENT_CACHE);
 	sbi->sb->s_flags |= MS_LAZYTIME;
 	set_opt(sbi, FLUSH_MERGE);
+	set_opt(sbi, ADAPTIVE);
 
 #ifdef CONFIG_F2FS_FS_XATTR
 	set_opt(sbi, XATTR_USER);
-- 
2.8.3

[PATCH 6/7] f2fs: fix deadlock in add_link failure

[Jaegeuk Kim]

mkdir                        sync_dirty_inode
 - init_inode_metadata
   - lock_page(node)
   - make_empty_dir
                             - filemap_fdatawrite()
                              - do_writepages
                              - lock_page(data)
                              - write_page(data)
                               - lock_page(node)
   - f2fs_init_acl
    - error
   - truncate_inode_pages
    - lock_page(data)

So, we don't need to truncate data pages in this error case, which will
be done by f2fs_evict_inode.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/dir.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index f6ab3c2..4864824 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -450,9 +450,6 @@ struct page *init_inode_metadata(struct inode *inode, struct inode *dir,
 	return page;
 
 put_error:
-	/* truncate empty dir pages */
-	truncate_inode_pages(&inode->i_data, 0);
-
 	clear_nlink(inode);
 	update_inode(inode, page);
 	f2fs_put_page(page, 1);
-- 
2.8.3

[PATCH 7/7] f2fs: don't need to flush unlinked dentry pages

[Jaegeuk Kim]

We don't need to flush any dentry pages used by unlinked directory.
They will be truncated by f2fs_evict_inode.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 4179c7b..e6714cb 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -846,7 +846,8 @@ retry:
 	inode = igrab(&fi->vfs_inode);
 	spin_unlock(&sbi->inode_lock[type]);
 	if (inode) {
-		filemap_fdatawrite(inode->i_mapping);
+		if (inode->i_nlink)
+			filemap_fdatawrite(inode->i_mapping);
 		iput(inode);
 	} else {
 		/*
-- 
2.8.3