2015-02-02 23:29:33

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 5/5 v2] f2fs: introduce a batched trim

Change long from v1:
o add description
o change the # of batched segments suggested by Chao
o make consistent for # of batched segments

This patch introduces a batched trimming feature, which submits split discard
commands.

This patch introduces a batched trimming feature, which submits split discard
commands.

This is to avoid long latency due to huge trim commands.
If fstrim was triggered ranging from 0 to the end of device, we should lock
all the checkpoint-related mutexes, resulting in very long latency.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/segment.c | 16 +++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8231a59..ec5e66f 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -105,6 +105,8 @@ enum {
CP_DISCARD,
};

+#define BATCHED_TRIM_SEGMENTS(sbi) (((sbi)->segs_per_sec) << 5)
+
struct cp_control {
int reason;
__u64 trim_start;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5ea57ec..b85bb97 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1066,14 +1066,20 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
GET_SEGNO(sbi, end);
cpc.reason = CP_DISCARD;
- cpc.trim_start = start_segno;
- cpc.trim_end = end_segno;
cpc.trim_minlen = range->minlen >> sbi->log_blocksize;

/* do checkpoint to issue discard commands safely */
- mutex_lock(&sbi->gc_mutex);
- write_checkpoint(sbi, &cpc);
- mutex_unlock(&sbi->gc_mutex);
+ for (; start_segno <= end_segno;
+ start_segno += BATCHED_TRIM_SEGMENTS(sbi)) {
+ cpc.trim_start = start_segno;
+ cpc.trim_end = min_t(unsigned int,
+ start_segno + BATCHED_TRIM_SEGMENTS (sbi) - 1,
+ end_segno);
+
+ mutex_lock(&sbi->gc_mutex);
+ write_checkpoint(sbi, &cpc);
+ mutex_unlock(&sbi->gc_mutex);
+ }
out:
range->len = cpc.trimmed << sbi->log_blocksize;
return 0;
--
2.1.1


2015-02-03 02:49:41

by Changman Lee

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 5/5 v2] f2fs: introduce a batched trim

Hi Jaegeuk,

IMHO, it looks better user could decide the size of trim considering latency of trim.
Otherwise, additional checkpoints user doesn't want will occur.

Regards,
Changman

On Mon, Feb 02, 2015 at 03:29:25PM -0800, Jaegeuk Kim wrote:
> Change long from v1:
> o add description
> o change the # of batched segments suggested by Chao
> o make consistent for # of batched segments
>
> This patch introduces a batched trimming feature, which submits split discard
> commands.
>
> This patch introduces a batched trimming feature, which submits split discard
> commands.
>
> This is to avoid long latency due to huge trim commands.
> If fstrim was triggered ranging from 0 to the end of device, we should lock
> all the checkpoint-related mutexes, resulting in very long latency.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 16 +++++++++++-----
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 8231a59..ec5e66f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -105,6 +105,8 @@ enum {
> CP_DISCARD,
> };
>
> +#define BATCHED_TRIM_SEGMENTS(sbi) (((sbi)->segs_per_sec) << 5)
> +
> struct cp_control {
> int reason;
> __u64 trim_start;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 5ea57ec..b85bb97 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1066,14 +1066,20 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
> end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
> GET_SEGNO(sbi, end);
> cpc.reason = CP_DISCARD;
> - cpc.trim_start = start_segno;
> - cpc.trim_end = end_segno;
> cpc.trim_minlen = range->minlen >> sbi->log_blocksize;
>
> /* do checkpoint to issue discard commands safely */
> - mutex_lock(&sbi->gc_mutex);
> - write_checkpoint(sbi, &cpc);
> - mutex_unlock(&sbi->gc_mutex);
> + for (; start_segno <= end_segno;
> + start_segno += BATCHED_TRIM_SEGMENTS(sbi)) {
> + cpc.trim_start = start_segno;
> + cpc.trim_end = min_t(unsigned int,
> + start_segno + BATCHED_TRIM_SEGMENTS (sbi) - 1,
> + end_segno);
> +
> + mutex_lock(&sbi->gc_mutex);
> + write_checkpoint(sbi, &cpc);
> + mutex_unlock(&sbi->gc_mutex);
> + }
> out:
> range->len = cpc.trimmed << sbi->log_blocksize;
> return 0;
> --
> 2.1.1
>
>
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming. The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2015-02-03 20:10:40

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 5/5 v3] f2fs: introduce a batched trim

Hi Changman,

Good idea!

Change log from v2:
o add sysfs to change the # of sections for trimming.

Change log from v1:
o add description
o change the # of batched segments suggested by Chao
o make consistent for # of batched segments

This patch introduces a batched trimming feature, which submits split discard
commands.

This is to avoid long latency due to huge trim commands.
If fstrim was triggered ranging from 0 to the end of device, we should lock
all the checkpoint-related mutexes, resulting in very long latency.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 6 ++++++
Documentation/filesystems/f2fs.txt | 4 ++++
fs/f2fs/f2fs.h | 7 +++++++
fs/f2fs/segment.c | 18 +++++++++++++-----
fs/f2fs/super.c | 2 ++
5 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 6f9157f..2c4cc42 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -74,3 +74,9 @@ Date: March 2014
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.
diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index 6758aa3..dac11d7 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -199,6 +199,10 @@ Files in /sys/fs/f2fs/<devname>
checkpoint is triggered, and issued during the
checkpoint. By default, it is disabled with 0.

+ trim_sections This parameter controls the number of sections
+ to be trimmed out in batch mode when FITRIM
+ conducts. 32 sections is set by default.
+
ipu_policy This parameter controls the policy of in-place
updates in f2fs. There are five policies:
0x01: F2FS_IPU_FORCE, 0x02: F2FS_IPU_SSR,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 964c240..6f57da1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -105,6 +105,10 @@ enum {
CP_DISCARD,
};

+#define DEF_BATCHED_TRIM_SECTIONS 32
+#define BATCHED_TRIM_SEGMENTS(sbi) \
+ (SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
+
struct cp_control {
int reason;
__u64 trim_start;
@@ -448,6 +452,9 @@ struct f2fs_sm_info {
int nr_discards; /* # of discards in the list */
int max_discards; /* max. discards to be issued */

+ /* for batched trimming */
+ 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 */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5ea57ec..c542f63 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1066,14 +1066,20 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
GET_SEGNO(sbi, end);
cpc.reason = CP_DISCARD;
- cpc.trim_start = start_segno;
- cpc.trim_end = end_segno;
cpc.trim_minlen = range->minlen >> sbi->log_blocksize;

/* do checkpoint to issue discard commands safely */
- mutex_lock(&sbi->gc_mutex);
- write_checkpoint(sbi, &cpc);
- mutex_unlock(&sbi->gc_mutex);
+ for (; start_segno <= end_segno;
+ start_segno += BATCHED_TRIM_SEGMENTS(sbi)) {
+ cpc.trim_start = start_segno;
+ cpc.trim_end = min_t(unsigned int,
+ start_segno + BATCHED_TRIM_SEGMENTS (sbi) - 1,
+ end_segno);
+
+ mutex_lock(&sbi->gc_mutex);
+ write_checkpoint(sbi, &cpc);
+ mutex_unlock(&sbi->gc_mutex);
+ }
out:
range->len = cpc.trimmed << sbi->log_blocksize;
return 0;
@@ -2127,6 +2133,8 @@ 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)) {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1e92c2e..f2fe666 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -195,6 +195,7 @@ 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);
@@ -210,6 +211,7 @@ 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),
--
2.1.1

2015-02-05 09:31:42

by Chao Yu

[permalink] [raw]
Subject: RE: [f2fs-dev] [PATCH 5/5 v3] f2fs: introduce a batched trim

Hi Jaegeuk, Changman,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Wednesday, February 04, 2015 4:11 AM
> To: Changman Lee
> Cc: Chao Yu; [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev] [PATCH 5/5 v3] f2fs: introduce a batched trim
>
> Hi Changman,
>
> Good idea!
>
> Change log from v2:
> o add sysfs to change the # of sections for trimming.

Good idea!

>
> Change log from v1:
> o add description
> o change the # of batched segments suggested by Chao
> o make consistent for # of batched segments
>
> This patch introduces a batched trimming feature, which submits split discard
> commands.
>
> This is to avoid long latency due to huge trim commands.
> If fstrim was triggered ranging from 0 to the end of device, we should lock
> all the checkpoint-related mutexes, resulting in very long latency.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-fs-f2fs | 6 ++++++
> Documentation/filesystems/f2fs.txt | 4 ++++
> fs/f2fs/f2fs.h | 7 +++++++
> fs/f2fs/segment.c | 18 +++++++++++++-----
> fs/f2fs/super.c | 2 ++
> 5 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs
> b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 6f9157f..2c4cc42 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -74,3 +74,9 @@ Date: March 2014
> 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.
> diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
> index 6758aa3..dac11d7 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -199,6 +199,10 @@ Files in /sys/fs/f2fs/<devname>
> checkpoint is triggered, and issued during the
> checkpoint. By default, it is disabled with 0.
>
> + trim_sections This parameter controls the number of sections
> + to be trimmed out in batch mode when FITRIM
> + conducts. 32 sections is set by default.
> +
> ipu_policy This parameter controls the policy of in-place
> updates in f2fs. There are five policies:
> 0x01: F2FS_IPU_FORCE, 0x02: F2FS_IPU_SSR,
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 964c240..6f57da1 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -105,6 +105,10 @@ enum {
> CP_DISCARD,
> };
>
> +#define DEF_BATCHED_TRIM_SECTIONS 32
> +#define BATCHED_TRIM_SEGMENTS(sbi) \
> + (SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
> +
> struct cp_control {
> int reason;
> __u64 trim_start;
> @@ -448,6 +452,9 @@ struct f2fs_sm_info {
> int nr_discards; /* # of discards in the list */
> int max_discards; /* max. discards to be issued */
>
> + /* for batched trimming */
> + 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 */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 5ea57ec..c542f63 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1066,14 +1066,20 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
> end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
> GET_SEGNO(sbi, end);
> cpc.reason = CP_DISCARD;
> - cpc.trim_start = start_segno;
> - cpc.trim_end = end_segno;
> cpc.trim_minlen = range->minlen >> sbi->log_blocksize;
>
> /* do checkpoint to issue discard commands safely */
> - mutex_lock(&sbi->gc_mutex);
> - write_checkpoint(sbi, &cpc);
> - mutex_unlock(&sbi->gc_mutex);
> + for (; start_segno <= end_segno;
> + start_segno += BATCHED_TRIM_SEGMENTS(sbi)) {
> + cpc.trim_start = start_segno;
> + cpc.trim_end = min_t(unsigned int,
> + start_segno + BATCHED_TRIM_SEGMENTS (sbi) - 1,

Actually what I mean is that in each trim we try to align start segment
to first segment of section as much as possible.

How about using:
cpc.trim_end = min_t(unsigned int, rounddown(start_segno +
BATCHED_TRIM_SEGMENTS(sbi),
(sbi)->segs_per_sec) - 1, end_segno);
Thanks,

> + end_segno);
> +
> + mutex_lock(&sbi->gc_mutex);
> + write_checkpoint(sbi, &cpc);
> + mutex_unlock(&sbi->gc_mutex);
> + }
> out:
> range->len = cpc.trimmed << sbi->log_blocksize;
> return 0;
> @@ -2127,6 +2133,8 @@ 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)) {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 1e92c2e..f2fe666 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -195,6 +195,7 @@ 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);
> @@ -210,6 +211,7 @@ 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),
> --
> 2.1.1

2015-02-06 06:18:42

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 5/5 v4] f2fs: introduce a batched trim

Hi Chao,

Something like this?

Change log from v3:
o align to section size

This patch introduces a batched trimming feature, which submits split discard
commands.

This is to avoid long latency due to huge trim commands.
If fstrim was triggered ranging from 0 to the end of device, we should lock
all the checkpoint-related mutexes, resulting in very long latency.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 6 ++++++
Documentation/filesystems/f2fs.txt | 4 ++++
fs/f2fs/f2fs.h | 7 +++++++
fs/f2fs/segment.c | 17 ++++++++++++-----
fs/f2fs/super.c | 2 ++
5 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 6f9157f..2c4cc42 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -74,3 +74,9 @@ Date: March 2014
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.
diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index 6758aa3..dac11d7 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -199,6 +199,10 @@ Files in /sys/fs/f2fs/<devname>
checkpoint is triggered, and issued during the
checkpoint. By default, it is disabled with 0.

+ trim_sections This parameter controls the number of sections
+ to be trimmed out in batch mode when FITRIM
+ conducts. 32 sections is set by default.
+
ipu_policy This parameter controls the policy of in-place
updates in f2fs. There are five policies:
0x01: F2FS_IPU_FORCE, 0x02: F2FS_IPU_SSR,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index db1ff55..d82a10a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -105,6 +105,10 @@ enum {
CP_DISCARD,
};

+#define DEF_BATCHED_TRIM_SECTIONS 32
+#define BATCHED_TRIM_SEGMENTS(sbi) \
+ (SM_I(sbi)->trim_sections * (sbi)->segs_per_sec)
+
struct cp_control {
int reason;
__u64 trim_start;
@@ -448,6 +452,9 @@ struct f2fs_sm_info {
int nr_discards; /* # of discards in the list */
int max_discards; /* max. discards to be issued */

+ /* for batched trimming */
+ 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 */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5ea57ec..9f278d1 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1066,14 +1066,19 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
GET_SEGNO(sbi, end);
cpc.reason = CP_DISCARD;
- cpc.trim_start = start_segno;
- cpc.trim_end = end_segno;
cpc.trim_minlen = range->minlen >> sbi->log_blocksize;

/* do checkpoint to issue discard commands safely */
- mutex_lock(&sbi->gc_mutex);
- write_checkpoint(sbi, &cpc);
- mutex_unlock(&sbi->gc_mutex);
+ for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) {
+ cpc.trim_start = start_segno;
+ 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);
+ write_checkpoint(sbi, &cpc);
+ mutex_unlock(&sbi->gc_mutex);
+ }
out:
range->len = cpc.trimmed << sbi->log_blocksize;
return 0;
@@ -2127,6 +2132,8 @@ 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)) {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1e92c2e..f2fe666 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -195,6 +195,7 @@ 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);
@@ -210,6 +211,7 @@ 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),
--
2.1.1

2015-02-06 08:21:34

by Chao Yu

[permalink] [raw]
Subject: RE: [f2fs-dev] [PATCH 5/5 v4] f2fs: introduce a batched trim

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Friday, February 06, 2015 2:19 PM
> To: Chao Yu
> Cc: 'Changman Lee'; [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev] [PATCH 5/5 v4] f2fs: introduce a batched trim
>
> Hi Chao,
>
> Something like this?
>
> Change log from v3:
> o align to section size
>
> This patch introduces a batched trimming feature, which submits split discard
> commands.
>
> This is to avoid long latency due to huge trim commands.
> If fstrim was triggered ranging from 0 to the end of device, we should lock
> all the checkpoint-related mutexes, resulting in very long latency.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>

Looks good to me.

Reviewed-by: Chao Yu <[email protected]>

2015-02-07 15:58:09

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 5/5 v4] f2fs: introduce a batched trim

Hi Jaegeuk,

> + /* for batched trimming */
> + int trim_sections; /* # of sections to trim */
I would like to suggest to declare trim_sections variable as an
unsigned int and not int, since it can't be negative by definition.
What do you think about it?

2015-02-09 07:04:13

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 5/5 v4] f2fs: introduce a batched trim

On Sat, Feb 07, 2015 at 05:57:46PM +0200, Leon Romanovsky wrote:
> Hi Jaegeuk,
>
> > + /* for batched trimming */
> > + int trim_sections; /* # of sections to trim */
> I would like to suggest to declare trim_sections variable as an
> unsigned int and not int, since it can't be negative by definition.
> What do you think about it?

Hi Leon,

No problem. :)

I'll change that and merge the patch.

Thanks,