f2fs: clean up flush/discard command namings

This patch modifies stat information more clearly.

Signed-off-by Jaegeuk Kim <[email protected]>

We don't need to do multiple checkpoints, since we don't actually wait for completion of discard commands during checkpoint.
Instead, we still need to avoid very big discard commands, since that large discard can interfere block allocation.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ------
 fs/f2fs/f2fs.h                          |  9 +-------
 fs/f2fs/segment.c                       | 38 +++++++++++----------------------
 fs/f2fs/super.c                         |  2 --
 4 files changed, 14 insertions(+), 41 deletions(-)

Documentation/ABI/testing/sysfs-fs-f2fs +0 -6

index a809f6005f14..df6b3f6e164a 100644
@@ -75,12 +75,6 @@ Contact:	"Jaegeuk Kim" <[email protected]>
 Description:
 		 Controls the memory footprint used by f2fs.
 
-What:		/sys/fs/f2fs/<disk>/trim_sections
-Date:		February 2015
-Contact:	"Jaegeuk Kim" <[email protected]>
-Description:
-		 Controls the trimming rate in batch mode.
-
 What:		/sys/fs/f2fs/<disk>/cp_interval
 Date:		October 2015
 Contact:	"Jaegeuk Kim" <[email protected]>

fs/f2fs/f2fs.h +1 -8

index 548e75d18ec1..a2850bf2a487 100644
@@ -128,11 +128,7 @@ enum {
 	CP_DISCARD,
 };
 
-#define DEF_BATCHED_TRIM_SECTIONS	2
-#define BATCHED_TRIM_SEGMENTS(sbi)	\
-		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
-#define BATCHED_TRIM_BLOCKS(sbi)	\
-		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
+#define MAX_DISCARD_BLOCKS(sbi) (1 << (sbi)->log_blocks_per_seg)
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 

@@ -638,9 +634,6 @@ struct f2fs_sm_info {
 	int nr_discards;			/* # of discards in the list */
 	int max_discards;			/* max. discards to be issued */
 
-	/* for batched trimming */
-	unsigned int trim_sections;		/* # of sections to trim */
-
 	struct list_head sit_entry_set;	/* sit entry set list */
 
 	unsigned int ipu_policy;	/* in-place-update policy */

fs/f2fs/segment.c +13 -25

index 73282108fa33..e6f3c6db7616 100644
@@ -823,7 +823,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
 	if (!list_empty(head)) {
 		last = list_last_entry(head, struct discard_entry, list);
 		if (START_BLOCK(sbi, cpc->trim_start) + start ==
-						last->blkaddr + last->len) {
+				last->blkaddr + last->len &&
+				last->len <= MAX_DISCARD_BLOCKS(sbi)) {
 			last->len += end - start;
 			goto done;
 		}

@@ -1513,36 +1514,25 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 			"Found FS corruption, run fsck to fix.");
 		goto out;
 	}
+	if (sbi->discard_blks == 0)
+		goto out;
 
 	/* start/end segment number in main_area */
 	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
 	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
 						GET_SEGNO(sbi, end);
+	/*
+	 * do checkpoint to issue discard commands safely since we now can
+	 * use async discard.
+	 */
 	cpc.reason = CP_DISCARD;
 	cpc.trim_minlen = max_t(__u64, 1, F2FS_BYTES_TO_BLK(range->minlen));
+	cpc.trim_start = start_segno;
+	cpc.trim_end = end_segno;
 
-	/* do checkpoint to issue discard commands safely */
-	for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) {
-		cpc.trim_start = start_segno;
-
-		if (sbi->discard_blks == 0)
-			break;
-		else if (sbi->discard_blks < BATCHED_TRIM_BLOCKS(sbi))
-			cpc.trim_end = end_segno;
-		else
-			cpc.trim_end = min_t(unsigned int,
-				rounddown(start_segno +
-				BATCHED_TRIM_SEGMENTS(sbi),
-				sbi->segs_per_sec) - 1, end_segno);
-
-		mutex_lock(&sbi->gc_mutex);
-		err = write_checkpoint(sbi, &cpc);
-		mutex_unlock(&sbi->gc_mutex);
-		if (err)
-			break;
-
-		schedule();
-	}
+	mutex_lock(&sbi->gc_mutex);
+	err = write_checkpoint(sbi, &cpc);
+	mutex_unlock(&sbi->gc_mutex);
 out:
 	range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
 	return err;

@@ -2708,8 +2698,6 @@ int build_segment_manager(struct f2fs_sb_info *sbi)
 	sm_info->nr_discards = 0;
 	sm_info->max_discards = 0;
 
-	sm_info->trim_sections = DEF_BATCHED_TRIM_SECTIONS;
-
 	INIT_LIST_HEAD(&sm_info->sit_entry_set);
 
 	if (test_opt(sbi, FLUSH_MERGE) && !f2fs_readonly(sbi->sb)) {

fs/f2fs/super.c +0 -1

index 461c29043aec..37d40d8aa9c4 100644
@@ -284,7 +284,6 @@ F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
 F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_idle, gc_idle);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, max_small_discards, max_discards);
-F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_fsync_blocks, min_fsync_blocks);

Quoted 4 lines · Jaegeuk Kim

We don't need to do multiple checkpoints, since we don't actually wait for completion of discard commands during checkpoint.
Instead, we still need to avoid very big discard commands, since that large discard can interfere block allocation.

I hope we can keep this functionality and related sysfs interface, the main concern is that: for 16T size fragmented partition, at most there will be ~sizeof(u32)/2 4K blocks which could be invalid and needed to be discarded. After removal of in-batch discard, we have to handle billions of discard message with gc_mutex & cp_rwsem held, which will freeze f2fs for very long time.

So I suggest we just set trim_sections to 512 segments (covers one GB) by default. How do you think?

Thanks,

Quoted 138 lines

Signed-off-by Jaegeuk Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ------
 fs/f2fs/f2fs.h                          |  9 +-------
 fs/f2fs/segment.c                       | 38 +++++++++++----------------------
 fs/f2fs/super.c                         |  2 --
 4 files changed, 14 insertions(+), 41 deletions(-)

Documentation/ABI/testing/sysfs-fs-f2fs +0 -6

index a809f6005f14..df6b3f6e164a 100644
@@ -75,12 +75,6 @@ Contact:	"Jaegeuk Kim" <[email protected]>
 Description:
 		 Controls the memory footprint used by f2fs.
 
-What:		/sys/fs/f2fs/<disk>/trim_sections
-Date:		February 2015
-Contact:	"Jaegeuk Kim" <[email protected]>
-Description:
-		 Controls the trimming rate in batch mode.
-
 What:		/sys/fs/f2fs/<disk>/cp_interval
 Date:		October 2015
 Contact:	"Jaegeuk Kim" <[email protected]>

fs/f2fs/f2fs.h +1 -8

index 548e75d18ec1..a2850bf2a487 100644
@@ -128,11 +128,7 @@ enum {
 	CP_DISCARD,
 };
 
-#define DEF_BATCHED_TRIM_SECTIONS	2
-#define BATCHED_TRIM_SEGMENTS(sbi)	\
-		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
-#define BATCHED_TRIM_BLOCKS(sbi)	\
-		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
+#define MAX_DISCARD_BLOCKS(sbi) (1 << (sbi)->log_blocks_per_seg)
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 

@@ -638,9 +634,6 @@ struct f2fs_sm_info {
 	int nr_discards;			/* # of discards in the list */
 	int max_discards;			/* max. discards to be issued */
 
-	/* for batched trimming */
-	unsigned int trim_sections;		/* # of sections to trim */
-
 	struct list_head sit_entry_set;	/* sit entry set list */
 
 	unsigned int ipu_policy;	/* in-place-update policy */

fs/f2fs/segment.c +13 -25

index 73282108fa33..e6f3c6db7616 100644
@@ -823,7 +823,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
 	if (!list_empty(head)) {
 		last = list_last_entry(head, struct discard_entry, list);
 		if (START_BLOCK(sbi, cpc->trim_start) + start ==
-						last->blkaddr + last->len) {
+				last->blkaddr + last->len &&
+				last->len <= MAX_DISCARD_BLOCKS(sbi)) {
 			last->len += end - start;
 			goto done;
 		}

@@ -1513,36 +1514,25 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 			"Found FS corruption, run fsck to fix.");
 		goto out;
 	}
+	if (sbi->discard_blks == 0)
+		goto out;
 
 	/* start/end segment number in main_area */
 	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
 	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
 						GET_SEGNO(sbi, end);
+	/*
+	 * do checkpoint to issue discard commands safely since we now can
+	 * use async discard.
+	 */
 	cpc.reason = CP_DISCARD;
 	cpc.trim_minlen = max_t(__u64, 1, F2FS_BYTES_TO_BLK(range->minlen));
+	cpc.trim_start = start_segno;
+	cpc.trim_end = end_segno;
 
-	/* do checkpoint to issue discard commands safely */
-	for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) {
-		cpc.trim_start = start_segno;
-
-		if (sbi->discard_blks == 0)
-			break;
-		else if (sbi->discard_blks < BATCHED_TRIM_BLOCKS(sbi))
-			cpc.trim_end = end_segno;
-		else
-			cpc.trim_end = min_t(unsigned int,
-				rounddown(start_segno +
-				BATCHED_TRIM_SEGMENTS(sbi),
-				sbi->segs_per_sec) - 1, end_segno);
-
-		mutex_lock(&sbi->gc_mutex);
-		err = write_checkpoint(sbi, &cpc);
-		mutex_unlock(&sbi->gc_mutex);
-		if (err)
-			break;
-
-		schedule();
-	}
+	mutex_lock(&sbi->gc_mutex);
+	err = write_checkpoint(sbi, &cpc);
+	mutex_unlock(&sbi->gc_mutex);
 out:
 	range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
 	return err;

@@ -2708,8 +2698,6 @@ int build_segment_manager(struct f2fs_sb_info *sbi)
 	sm_info->nr_discards = 0;
 	sm_info->max_discards = 0;
 
-	sm_info->trim_sections = DEF_BATCHED_TRIM_SECTIONS;
-
 	INIT_LIST_HEAD(&sm_info->sit_entry_set);
 
 	if (test_opt(sbi, FLUSH_MERGE) && !f2fs_readonly(sbi->sb)) {

fs/f2fs/super.c +0 -1

index 461c29043aec..37d40d8aa9c4 100644
@@ -284,7 +284,6 @@ F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
 F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_idle, gc_idle);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, max_small_discards, max_discards);
-F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_fsync_blocks, min_fsync_blocks);

fs/f2fs/super.c static struct attribute *f2fs_attrs[] = { · line 313

Quoted 7 lines

fs/f2fs/super.c +0 -1

@@ -309,7 +308,6 @@ static struct attribute *f2fs_attrs[] = {
 	ATTR_LIST(gc_idle),
 	ATTR_LIST(reclaim_segments),
 	ATTR_LIST(max_small_discards),
-	ATTR_LIST(batched_trim_sections),
 	ATTR_LIST(ipu_policy),
 	ATTR_LIST(min_ipu_util),
 	ATTR_LIST(min_fsync_blocks),

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Quoted 6 lines · Chao Yu

Quoted 4 lines · Jaegeuk Kim

We don't need to do multiple checkpoints, since we don't actually wait for completion of discard commands during checkpoint.
Instead, we still need to avoid very big discard commands, since that large discard can interfere block allocation.

Quoted 5 lines

I hope we can keep this functionality and related sysfs interface, the main concern is that: for 16T size fragmented partition, at most there will be ~sizeof(u32)/2 4K blocks which could be invalid and needed to be discarded. After removal of in-batch discard, we have to handle billions of discard message with gc_mutex & cp_rwsem held, which will freeze f2fs for very long time.

The baseline of this patch is that we have a kernel thread to issue discard commands asynchronously. So, I don't expect such the huge latency anymore.

Thanks,

Quoted 153 lines

So I suggest we just set trim_sections to 512 segments (covers one GB) by default. How do you think?

Thanks,

Quoted 138 lines

Signed-off-by Jaegeuk Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ------
 fs/f2fs/f2fs.h                          |  9 +-------
 fs/f2fs/segment.c                       | 38 +++++++++++----------------------
 fs/f2fs/super.c                         |  2 --
 4 files changed, 14 insertions(+), 41 deletions(-)

Documentation/ABI/testing/sysfs-fs-f2fs +0 -6

index a809f6005f14..df6b3f6e164a 100644
@@ -75,12 +75,6 @@ Contact:	"Jaegeuk Kim" <[email protected]>
 Description:
 		 Controls the memory footprint used by f2fs.
 
-What:		/sys/fs/f2fs/<disk>/trim_sections
-Date:		February 2015
-Contact:	"Jaegeuk Kim" <[email protected]>
-Description:
-		 Controls the trimming rate in batch mode.
-
 What:		/sys/fs/f2fs/<disk>/cp_interval
 Date:		October 2015
 Contact:	"Jaegeuk Kim" <[email protected]>

fs/f2fs/f2fs.h +1 -8

index 548e75d18ec1..a2850bf2a487 100644
@@ -128,11 +128,7 @@ enum {
 	CP_DISCARD,
 };
 
-#define DEF_BATCHED_TRIM_SECTIONS	2
-#define BATCHED_TRIM_SEGMENTS(sbi)	\
-		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
-#define BATCHED_TRIM_BLOCKS(sbi)	\
-		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
+#define MAX_DISCARD_BLOCKS(sbi) (1 << (sbi)->log_blocks_per_seg)
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 

@@ -638,9 +634,6 @@ struct f2fs_sm_info {
 	int nr_discards;			/* # of discards in the list */
 	int max_discards;			/* max. discards to be issued */
 
-	/* for batched trimming */
-	unsigned int trim_sections;		/* # of sections to trim */
-
 	struct list_head sit_entry_set;	/* sit entry set list */
 
 	unsigned int ipu_policy;	/* in-place-update policy */

fs/f2fs/segment.c +13 -25

index 73282108fa33..e6f3c6db7616 100644
@@ -823,7 +823,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
 	if (!list_empty(head)) {
 		last = list_last_entry(head, struct discard_entry, list);
 		if (START_BLOCK(sbi, cpc->trim_start) + start ==
-						last->blkaddr + last->len) {
+				last->blkaddr + last->len &&
+				last->len <= MAX_DISCARD_BLOCKS(sbi)) {
 			last->len += end - start;
 			goto done;
 		}

@@ -1513,36 +1514,25 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 			"Found FS corruption, run fsck to fix.");
 		goto out;
 	}
+	if (sbi->discard_blks == 0)
+		goto out;
 
 	/* start/end segment number in main_area */
 	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
 	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
 						GET_SEGNO(sbi, end);
+	/*
+	 * do checkpoint to issue discard commands safely since we now can
+	 * use async discard.
+	 */
 	cpc.reason = CP_DISCARD;
 	cpc.trim_minlen = max_t(__u64, 1, F2FS_BYTES_TO_BLK(range->minlen));
+	cpc.trim_start = start_segno;
+	cpc.trim_end = end_segno;
 
-	/* do checkpoint to issue discard commands safely */
-	for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) {
-		cpc.trim_start = start_segno;
-
-		if (sbi->discard_blks == 0)
-			break;
-		else if (sbi->discard_blks < BATCHED_TRIM_BLOCKS(sbi))
-			cpc.trim_end = end_segno;
-		else
-			cpc.trim_end = min_t(unsigned int,
-				rounddown(start_segno +
-				BATCHED_TRIM_SEGMENTS(sbi),
-				sbi->segs_per_sec) - 1, end_segno);
-
-		mutex_lock(&sbi->gc_mutex);
-		err = write_checkpoint(sbi, &cpc);
-		mutex_unlock(&sbi->gc_mutex);
-		if (err)
-			break;
-
-		schedule();
-	}
+	mutex_lock(&sbi->gc_mutex);
+	err = write_checkpoint(sbi, &cpc);
+	mutex_unlock(&sbi->gc_mutex);
 out:
 	range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
 	return err;

@@ -2708,8 +2698,6 @@ int build_segment_manager(struct f2fs_sb_info *sbi)
 	sm_info->nr_discards = 0;
 	sm_info->max_discards = 0;
 
-	sm_info->trim_sections = DEF_BATCHED_TRIM_SECTIONS;
-
 	INIT_LIST_HEAD(&sm_info->sit_entry_set);
 
 	if (test_opt(sbi, FLUSH_MERGE) && !f2fs_readonly(sbi->sb)) {

fs/f2fs/super.c +0 -1

index 461c29043aec..37d40d8aa9c4 100644
@@ -284,7 +284,6 @@ F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
 F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_idle, gc_idle);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, max_small_discards, max_discards);
-F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_fsync_blocks, min_fsync_blocks);

fs/f2fs/super.c static struct attribute *f2fs_attrs[] = { · line 313

Quoted 7 lines

fs/f2fs/super.c +0 -1

@@ -309,7 +308,6 @@ static struct attribute *f2fs_attrs[] = {
 	ATTR_LIST(gc_idle),
 	ATTR_LIST(reclaim_segments),
 	ATTR_LIST(max_small_discards),
-	ATTR_LIST(batched_trim_sections),
 	ATTR_LIST(ipu_policy),
 	ATTR_LIST(min_ipu_util),
 	ATTR_LIST(min_fsync_blocks),

Quoted 7 lines

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

.

Quoted 13 lines · Jaegeuk Kim

Quoted 11 lines · Chao Yu

Quoted 4 lines · Jaegeuk Kim

We don't need to do multiple checkpoints, since we don't actually wait for completion of discard commands during checkpoint.
Instead, we still need to avoid very big discard commands, since that large discard can interfere block allocation.

I hope we can keep this functionality and related sysfs interface, the main concern is that: for 16T size fragmented partition, at most there will be ~sizeof(u32)/2 4K blocks which could be invalid and needed to be discarded. After removal of in-batch discard, we have to handle billions of discard message with gc_mutex & cp_rwsem held, which will freeze f2fs for very long time.

Quoted 2 lines

The baseline of this patch is that we have a kernel thread to issue discard commands asynchronously. So, I don't expect such the huge latency anymore.

Yes, I know async kernel thread was already introduced, and we can expect such implementation can help to reduce huge handling latency. But still in checkpoint we need to check region of invalid block and send the pending discard entries to kernel thread, so what I concern is that in scenario of there is huge number of discard messages, we will still face long latency with cp lock held.

Thanks,

Quoted 164 lines

Thanks,

Quoted 153 lines

So I suggest we just set trim_sections to 512 segments (covers one GB) by default. How do you think?

Thanks,

Quoted 138 lines

Signed-off-by Jaegeuk Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-fs-f2fs |  6 ------
 fs/f2fs/f2fs.h                          |  9 +-------
 fs/f2fs/segment.c                       | 38 +++++++++++----------------------
 fs/f2fs/super.c                         |  2 --
 4 files changed, 14 insertions(+), 41 deletions(-)

Documentation/ABI/testing/sysfs-fs-f2fs +0 -6

index a809f6005f14..df6b3f6e164a 100644
@@ -75,12 +75,6 @@ Contact:	"Jaegeuk Kim" <[email protected]>
 Description:
 		 Controls the memory footprint used by f2fs.
 
-What:		/sys/fs/f2fs/<disk>/trim_sections
-Date:		February 2015
-Contact:	"Jaegeuk Kim" <[email protected]>
-Description:
-		 Controls the trimming rate in batch mode.
-
 What:		/sys/fs/f2fs/<disk>/cp_interval
 Date:		October 2015
 Contact:	"Jaegeuk Kim" <[email protected]>

fs/f2fs/f2fs.h +1 -8

index 548e75d18ec1..a2850bf2a487 100644
@@ -128,11 +128,7 @@ enum {
 	CP_DISCARD,
 };
 
-#define DEF_BATCHED_TRIM_SECTIONS	2
-#define BATCHED_TRIM_SEGMENTS(sbi)	\
-		(SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
-#define BATCHED_TRIM_BLOCKS(sbi)	\
-		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
+#define MAX_DISCARD_BLOCKS(sbi) (1 << (sbi)->log_blocks_per_seg)
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
 

@@ -638,9 +634,6 @@ struct f2fs_sm_info {
 	int nr_discards;			/* # of discards in the list */
 	int max_discards;			/* max. discards to be issued */
 
-	/* for batched trimming */
-	unsigned int trim_sections;		/* # of sections to trim */
-
 	struct list_head sit_entry_set;	/* sit entry set list */
 
 	unsigned int ipu_policy;	/* in-place-update policy */

fs/f2fs/segment.c +13 -25

index 73282108fa33..e6f3c6db7616 100644
@@ -823,7 +823,8 @@ static void __add_discard_entry(struct f2fs_sb_info *sbi,
 	if (!list_empty(head)) {
 		last = list_last_entry(head, struct discard_entry, list);
 		if (START_BLOCK(sbi, cpc->trim_start) + start ==
-						last->blkaddr + last->len) {
+				last->blkaddr + last->len &&
+				last->len <= MAX_DISCARD_BLOCKS(sbi)) {
 			last->len += end - start;
 			goto done;
 		}

@@ -1513,36 +1514,25 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 			"Found FS corruption, run fsck to fix.");
 		goto out;
 	}
+	if (sbi->discard_blks == 0)
+		goto out;
 
 	/* start/end segment number in main_area */
 	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
 	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
 						GET_SEGNO(sbi, end);
+	/*
+	 * do checkpoint to issue discard commands safely since we now can
+	 * use async discard.
+	 */
 	cpc.reason = CP_DISCARD;
 	cpc.trim_minlen = max_t(__u64, 1, F2FS_BYTES_TO_BLK(range->minlen));
+	cpc.trim_start = start_segno;
+	cpc.trim_end = end_segno;
 
-	/* do checkpoint to issue discard commands safely */
-	for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) {
-		cpc.trim_start = start_segno;
-
-		if (sbi->discard_blks == 0)
-			break;
-		else if (sbi->discard_blks < BATCHED_TRIM_BLOCKS(sbi))
-			cpc.trim_end = end_segno;
-		else
-			cpc.trim_end = min_t(unsigned int,
-				rounddown(start_segno +
-				BATCHED_TRIM_SEGMENTS(sbi),
-				sbi->segs_per_sec) - 1, end_segno);
-
-		mutex_lock(&sbi->gc_mutex);
-		err = write_checkpoint(sbi, &cpc);
-		mutex_unlock(&sbi->gc_mutex);
-		if (err)
-			break;
-
-		schedule();
-	}
+	mutex_lock(&sbi->gc_mutex);
+	err = write_checkpoint(sbi, &cpc);
+	mutex_unlock(&sbi->gc_mutex);
 out:
 	range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
 	return err;

@@ -2708,8 +2698,6 @@ int build_segment_manager(struct f2fs_sb_info *sbi)
 	sm_info->nr_discards = 0;
 	sm_info->max_discards = 0;
 
-	sm_info->trim_sections = DEF_BATCHED_TRIM_SECTIONS;
-
 	INIT_LIST_HEAD(&sm_info->sit_entry_set);
 
 	if (test_opt(sbi, FLUSH_MERGE) && !f2fs_readonly(sbi->sb)) {

fs/f2fs/super.c +0 -1

index 461c29043aec..37d40d8aa9c4 100644
@@ -284,7 +284,6 @@ F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
 F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_idle, gc_idle);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, reclaim_segments, rec_prefree_segments);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, max_small_discards, max_discards);
-F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, batched_trim_sections, trim_sections);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, ipu_policy, ipu_policy);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_ipu_util, min_ipu_util);
 F2FS_RW_ATTR(SM_INFO, f2fs_sm_info, min_fsync_blocks, min_fsync_blocks);

fs/f2fs/super.c static struct attribute *f2fs_attrs[] = { · line 313

Quoted 7 lines

fs/f2fs/super.c +0 -1

@@ -309,7 +308,6 @@ static struct attribute *f2fs_attrs[] = {
 	ATTR_LIST(gc_idle),
 	ATTR_LIST(reclaim_segments),
 	ATTR_LIST(max_small_discards),
-	ATTR_LIST(batched_trim_sections),
 	ATTR_LIST(ipu_policy),
 	ATTR_LIST(min_ipu_util),
 	ATTR_LIST(min_fsync_blocks),

Quoted 7 lines

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Quoted 2 lines

.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

This patch adds discard_cmd_control with the existing discarding controls.

Signed-off-by Jaegeuk Kim <[email protected]>

This patch adds a kernel thread to issue discard commands.
It proposes three states, D_PREP, D_SUBMIT, and D_DONE to identify current bio status.

Quoted 3 lines · Jaegeuk Kim

This patch adds a kernel thread to issue discard commands.
It proposes three states, D_PREP, D_SUBMIT, and D_DONE to identify current bio status.

Why? Instead of creating the complexity of a thread you should look into issuing the discard bios asynchronously, which should help to simplify a lot of the discard logic in f2fs.

Previously, I've done to issue discard bios asynchronously. But the problem that I've got is that was not enough. When testing nvme SSD with noop IO scheduler, submit_bio() was blocked at every 8 async discard bios, resulting in very slow checkpoint process which blocks most of other FS operations.

Quoted 4 lines · Jaegeuk Kim

Previously, I've done to issue discard bios asynchronously. But the problem that I've got is that was not enough. When testing nvme SSD with noop IO scheduler, submit_bio() was blocked at every 8 async discard bios, resulting in very slow checkpoint process which blocks most of other FS operations.

Where does it block? Are you running out of request? What driver is this on top of?

Quoted 6 lines · Christoph Hellwig

Quoted 4 lines · Jaegeuk Kim

Previously, I've done to issue discard bios asynchronously. But the problem that I've got is that was not enough. When testing nvme SSD with noop IO scheduler, submit_bio() was blocked at every 8 async discard bios, resulting in very slow checkpoint process which blocks most of other FS operations.

Quoted 2 lines

Where does it block? Are you running out of request? What driver is this on top of?

Ping? I'm currently spending a lot of effort on fs and block dіscard code, and I'd like to make sure we get common infrastructure instead of local hacks.

Sorry for the late response due to the travel.

When doing fstrim with a fresh f2fs image fomatted on Intel NVMe SSD whose model name is SSDPE2MW012T4, I've got the following trace.

So, I investigated why block_rq_complete() happened in more detail.

The root-caused call path looks like:
- submit_bio
- generic_make_request
- q->make_request_fn
- blk_mq_make_request
- blk_mq_map_request
- blk_mq_alloc_request
- blk_mq_get_tag
- __blk_mq_get_tag
- bt_get
- blk_mq_run_hw_queue
- finish_wait
--> this waits for pending 8 discard bios!

It seems the problem comes from the storage processing discard commands too slowly comparing to normal read/write IOs.

Any thoughts?

Quoted 4 lines · Jaegeuk Kim

Sorry for the late response due to the travel.

When doing fstrim with a fresh f2fs image fomatted on Intel NVMe SSD whose model name is SSDPE2MW012T4, I've got the following trace.

<snip>

Quoted 15 lines

So, I investigated why block_rq_complete() happened in more detail.

The root-caused call path looks like:
- submit_bio
- generic_make_request
- q->make_request_fn
- blk_mq_make_request
- blk_mq_map_request
- blk_mq_alloc_request
- blk_mq_get_tag
- __blk_mq_get_tag
- bt_get
- blk_mq_run_hw_queue
- finish_wait
--> this waits for pending 8 discard bios!

You're blocking on tag allocation. How many tags per queue does your device have?, e.g. do a

cat /sys/block/nvme0n1/mq/0/nr_tags

Quoted 4 lines

It seems the problem comes from the storage processing discard commands too slowly comparing to normal read/write IOs.

Any thoughts?

Deallocate is always going to be an exception path compared to normal read/write… but just how much slower is going to be device dependent.

One option would be to reuse the number of discards, for that can you try the series here to support vectored discards:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/vectored-discard-for-axboe

This patch adds a kernel thread to issue discard commands.
It proposes three states, D_PREP, D_SUBMIT, and D_DONE to identify current bio status.

Signed-off-by Jaegeuk Kim <[email protected]>

This patch adds stat information for flush and discard commands.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/debug.c   | 7 +++++--
 fs/f2fs/f2fs.h    | 3 ++-
 fs/f2fs/segment.c | 4 ++++
 3 files changed, 11 insertions(+), 3 deletions(-)

fs/f2fs/debug.c +5 -2

index f9f6b0aeba02..e67cab2fafac 100644
@@ -54,6 +54,8 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 	si->max_aw_cnt = atomic_read(&sbi->max_aw_cnt);
 	si->nr_wb_cp_data = get_pages(sbi, F2FS_WB_CP_DATA);
 	si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA);
+	si->nr_flush = atomic_read(&SM_I(sbi)->fcc_info->submit_flush);
+	si->nr_discard = atomic_read(&SM_I(sbi)->dcc_info->submit_discard);
 	si->total_count = (int)sbi->user_block_count / sbi->blocks_per_seg;
 	si->rsvd_segs = reserved_segments(sbi);
 	si->overp_segs = overprovision_segments(sbi);

@@ -318,8 +320,9 @@ static int stat_show(struct seq_file *s, void *v)
 		seq_printf(s, "  - Inner Struct Count: tree: %d(%d), node: %d\n",
 				si->ext_tree, si->zombie_tree, si->ext_node);
 		seq_puts(s, "\nBalancing F2FS Async:\n");
-		seq_printf(s, "  - IO (CP: %4d, Data: %4d)\n",
-			   si->nr_wb_cp_data, si->nr_wb_data);
+		seq_printf(s, "  - IO (CP: %4d, Data: %4d, Flush: %4d, Discard: %4d)\n",
+			   si->nr_wb_cp_data, si->nr_wb_data,
+			   si->nr_flush, si->nr_discard);
 		seq_printf(s, "  - inmem: %4d, atomic IO: %4d (Max. %4d)\n",
 			   si->inmem_pages, si->aw_cnt, si->max_aw_cnt);
 		seq_printf(s, "  - nodes: %4d in %4d\n",

fs/f2fs/f2fs.h +0 -0

index ef5e3709c161..d51bc18e7292 100644

fs/f2fs/f2fs.h +1 -1

 /* for the list of fsync inodes, used only during recovery */

@@ -2254,7 +2255,7 @@ struct f2fs_stat_info {
 	unsigned int ndirty_dirs, ndirty_files, ndirty_all;
 	int nats, dirty_nats, sits, dirty_sits, free_nids, alloc_nids;
 	int total_count, utilization;
-	int bg_gc, nr_wb_cp_data, nr_wb_data;
+	int bg_gc, nr_wb_cp_data, nr_wb_data, nr_flush, nr_discard;
 	int inline_xattr, inline_inode, inline_dir, orphans;
 	int aw_cnt, max_aw_cnt;
 	unsigned int valid_count, valid_node_count, valid_inode_count, discard_blks;

fs/f2fs/segment.c +3 -0

index 74364006bfa6..5a2d380182d9 100644
@@ -653,6 +653,8 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi, struct discard_cmd *d
 {
 	int err = dc->bio->bi_error;
 
+	atomic_dec(&(SM_I(sbi)->dcc_info->submit_discard));
+
 	if (err == -EOPNOTSUPP)
 		err = 0;
 

@@ -678,6 +680,7 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 			if (dc->state == D_PREP) {
 				dc->state = D_SUBMIT;
 				submit_bio(dc->bio);
+				atomic_inc(&dcc->submit_discard);
 			}
 			wait_for_completion_io(&dc->wait);
 

This patch adds stat information for flush and discard commands.

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/debug.c   | 7 +++++--
 fs/f2fs/f2fs.h    | 3 ++-
 fs/f2fs/segment.c | 4 ++++
 3 files changed, 11 insertions(+), 3 deletions(-)

fs/f2fs/debug.c +5 -2

index f9f6b0aeba02..e67cab2fafac 100644
@@ -54,6 +54,8 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 	si->max_aw_cnt = atomic_read(&sbi->max_aw_cnt);
 	si->nr_wb_cp_data = get_pages(sbi, F2FS_WB_CP_DATA);
 	si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA);
+	si->nr_flush = atomic_read(&SM_I(sbi)->fcc_info->submit_flush);
+	si->nr_discard = atomic_read(&SM_I(sbi)->dcc_info->submit_discard);
 	si->total_count = (int)sbi->user_block_count / sbi->blocks_per_seg;
 	si->rsvd_segs = reserved_segments(sbi);
 	si->overp_segs = overprovision_segments(sbi);

@@ -318,8 +320,9 @@ static int stat_show(struct seq_file *s, void *v)
 		seq_printf(s, "  - Inner Struct Count: tree: %d(%d), node: %d\n",
 				si->ext_tree, si->zombie_tree, si->ext_node);
 		seq_puts(s, "\nBalancing F2FS Async:\n");
-		seq_printf(s, "  - IO (CP: %4d, Data: %4d)\n",
-			   si->nr_wb_cp_data, si->nr_wb_data);
+		seq_printf(s, "  - IO (CP: %4d, Data: %4d, Flush: %4d, Discard: %4d)\n",
+			   si->nr_wb_cp_data, si->nr_wb_data,
+			   si->nr_flush, si->nr_discard);
 		seq_printf(s, "  - inmem: %4d, atomic IO: %4d (Max. %4d)\n",
 			   si->inmem_pages, si->aw_cnt, si->max_aw_cnt);
 		seq_printf(s, "  - nodes: %4d in %4d\n",

fs/f2fs/f2fs.h +2 -1

index ef5e3709c161..d51bc18e7292 100644
@@ -201,6 +201,7 @@ struct discard_cmd_control {
 	wait_queue_head_t discard_wait_queue;	/* waiting queue for wake-up */
 	struct mutex cmd_lock;
 	int max_discards;			/* max. discards to be issued */
+	atomic_t submit_discard;		/* # of issued discard */
 };
 
 /* for the list of fsync inodes, used only during recovery */

@@ -2254,7 +2255,7 @@ struct f2fs_stat_info {
 	unsigned int ndirty_dirs, ndirty_files, ndirty_all;
 	int nats, dirty_nats, sits, dirty_sits, free_nids, alloc_nids;
 	int total_count, utilization;
-	int bg_gc, nr_wb_cp_data, nr_wb_data;
+	int bg_gc, nr_wb_cp_data, nr_wb_data, nr_flush, nr_discard;
 	int inline_xattr, inline_inode, inline_dir, orphans;
 	int aw_cnt, max_aw_cnt;
 	unsigned int valid_count, valid_node_count, valid_inode_count, discard_blks;

fs/f2fs/segment.c +0 -0

index 74364006bfa6..5a2d380182d9 100644

fs/f2fs/segment.c +3 -0

+
 	if (err == -EOPNOTSUPP)
 		err = 0;
 

@@ -678,6 +680,7 @@ void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 			if (dc->state == D_PREP) {
 				dc->state = D_SUBMIT;
 				submit_bio(dc->bio);
+				atomic_inc(&dcc->submit_discard);
 			}
 			wait_for_completion_io(&dc->wait);
 

@@ -723,6 +726,7 @@ static int issue_discard_thread(void *data)
 		if (dc->state == D_PREP) {
 			dc->state = D_SUBMIT;
 			submit_bio(dc->bio);
+			atomic_inc(&dcc->submit_discard);
 			if (iter++ > DISCARD_ISSUE_RATE)
 				break;
 		} else if (dc->state == D_DONE) {

From 5d86527e4c70aeb8482dac24059624da720a0807 Mon Sep 17 00:00:00 2001

This patch adds stat information for flush and discard commands.

Signed-off-by Jaegeuk Kim <[email protected]>

This patch simply cleans up the names for flush/discard commands.

Signed-off-by Jaegeuk Kim <[email protected]>