Re: [PATCH v4] f2fs: introduce discard_granularity sysfs entry

From: Chao Yu
Date: Tue Aug 15 2017 - 08:05:21 EST


On 2017/8/15 11:45, Jaegeuk Kim wrote:
> On 08/07, Chao Yu wrote:
>> From: Chao Yu <yuchao0@xxxxxxxxxx>
>>
>> Commit d618ebaf0aa8 ("f2fs: enable small discard by default") enables
>> f2fs to issue 4K size discard in real-time discard mode. However, issuing
>> smaller discard may cost more lifetime but releasing less free space in
>> flash device. Since f2fs has ability of separating hot/cold data and
>> garbage collection, we can expect that small-sized invalid region would
>> expand soon with OPU, deletion or garbage collection on valid datas, so
>> it's better to delay or skip issuing smaller size discards, it could help
>> to reduce overmuch consumption of IO bandwidth and lifetime of flash
>> storage.
>>
>> This patch makes f2fs selectng 64K size as its default minimal
>> granularity, and issue discard with the size which is not smaller than
>> minimal granularity. Also it exposes discard granularity as sysfs entry
>> for configuration in different scenario.
>
> Hi Chao,
>
> I'd like to change the default value to 1 in order to keep the original
> behavior, since we must avoid performance fluctuation after this single
> patch. Instead, you probably can change the value through sysfs.

As I know, in fragmented filesystem space, there are may dozens of thousand
discard, in scenario of cellphone user are using, 30% is above 64K size, but
occupy 75% space of all undiscard space, so I changed discard_granularity to 64K
just to release bulk space in device. For other small-sized discards, I expect
that they may extend and cross the granularity threshold soon, and fstrim of
android could cover them in the night.

Do you encounter performance degression due to holding 64k-below size discard?

thank,

>
> Thanks,
>
>>
>> Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
>> ---
>> v4: minor change in commit log.
>> Documentation/ABI/testing/sysfs-fs-f2fs | 9 +++++++++
>> fs/f2fs/f2fs.h | 4 ++++
>> fs/f2fs/segment.c | 23 ++++++++++++++++++++++-
>> fs/f2fs/sysfs.c | 7 +++++++
>> 4 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 84c606fb3ca4..c579ce5e0ef5 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -57,6 +57,15 @@ Contact: "Jaegeuk Kim" <jaegeuk.kim@xxxxxxxxxxx>
>> Description:
>> Controls the issue rate of small discard commands.
>>
>> +What: /sys/fs/f2fs/<disk>/discard_granularity
>> +Date: July 2017
>> +Contact: "Chao Yu" <yuchao0@xxxxxxxxxx>
>> +Description:
>> + Controls discard granularity of inner discard thread, inner thread
>> + will not issue discards with size that is smaller than granularity.
>> + The unit size is one block, now only support configuring in range
>> + of [1, 512].
>> +
>> What: /sys/fs/f2fs/<disk>/max_victim_search
>> Date: January 2014
>> Contact: "Jaegeuk Kim" <jaegeuk.kim@xxxxxxxxxxx>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 0ccccbfdeeab..1bcaa93bfed7 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -195,6 +195,9 @@ struct discard_entry {
>> unsigned char discard_map[SIT_VBLOCK_MAP_SIZE]; /* segment discard bitmap */
>> };
>>
>> +/* default discard granularity of inner discard thread, unit: block count */
>> +#define DEFAULT_DISCARD_GRANULARITY 16
>> +
>> /* max discard pend list number */
>> #define MAX_PLIST_NUM 512
>> #define plist_idx(blk_num) ((blk_num) >= MAX_PLIST_NUM ? \
>> @@ -240,6 +243,7 @@ struct discard_cmd_control {
>> struct mutex cmd_lock;
>> unsigned int nr_discards; /* # of discards in the list */
>> unsigned int max_discards; /* max. discards to be issued */
>> + unsigned int discard_granularity; /* discard granularity */
>> unsigned int undiscard_blks; /* # of undiscard blocks */
>> atomic_t issued_discard; /* # of issued discard */
>> atomic_t issing_discard; /* # of issing discard */
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 71c2e0ac13d7..f336f8c541f0 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1019,7 +1019,8 @@ static void __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>> f2fs_bug_on(sbi,
>> !__check_rb_tree_consistence(sbi, &dcc->root));
>> blk_start_plug(&plug);
>> - for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
>> + for (i = MAX_PLIST_NUM - 1; i >= 0 &&
>> + i >= dcc->discard_granularity - 1; i--) {
>> pend_list = &dcc->pend_list[i];
>> list_for_each_entry_safe(dc, tmp, pend_list, list) {
>> f2fs_bug_on(sbi, dc->state != D_PREP);
>> @@ -1035,6 +1036,24 @@ static void __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>> mutex_unlock(&dcc->cmd_lock);
>> }
>>
>> +static void __drop_discard_cmd(struct f2fs_sb_info *sbi)
>> +{
>> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>> + struct list_head *pend_list;
>> + struct discard_cmd *dc, *tmp;
>> + int i;
>> +
>> + mutex_lock(&dcc->cmd_lock);
>> + for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
>> + pend_list = &dcc->pend_list[i];
>> + list_for_each_entry_safe(dc, tmp, pend_list, list) {
>> + f2fs_bug_on(sbi, dc->state != D_PREP);
>> + __remove_discard_cmd(sbi, dc);
>> + }
>> + }
>> + mutex_unlock(&dcc->cmd_lock);
>> +}
>> +
>> static void __wait_one_discard_bio(struct f2fs_sb_info *sbi,
>> struct discard_cmd *dc)
>> {
>> @@ -1117,6 +1136,7 @@ void stop_discard_thread(struct f2fs_sb_info *sbi)
>> void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
>> {
>> __issue_discard_cmd(sbi, false);
>> + __drop_discard_cmd(sbi);
>> __wait_discard_cmd(sbi, false);
>> }
>>
>> @@ -1449,6 +1469,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>> atomic_set(&dcc->discard_cmd_cnt, 0);
>> dcc->nr_discards = 0;
>> dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
>> + dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY;
>> dcc->undiscard_blks = 0;
>> dcc->root = RB_ROOT;
>>
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index 045b948c7466..4e5a95e9e666 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -152,6 +152,11 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>> spin_unlock(&sbi->stat_lock);
>> return count;
>> }
>> +
>> + if (!strcmp(a->attr.name, "discard_granularity") &&
>> + (t == 0 || t > MAX_PLIST_NUM))
>> + return -EINVAL;
>> +
>> *ui = t;
>>
>> if (!strcmp(a->attr.name, "iostat_enable") && *ui == 0)
>> @@ -241,6 +246,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(DCC_INFO, discard_cmd_control, max_small_discards, max_discards);
>> +F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_granularity, discard_granularity);
>> F2FS_RW_ATTR(RESERVED_BLOCKS, f2fs_sb_info, reserved_blocks, reserved_blocks);
>> 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);
>> @@ -281,6 +287,7 @@ static struct attribute *f2fs_attrs[] = {
>> ATTR_LIST(gc_idle),
>> ATTR_LIST(reclaim_segments),
>> ATTR_LIST(max_small_discards),
>> + ATTR_LIST(discard_granularity),
>> ATTR_LIST(batched_trim_sections),
>> ATTR_LIST(ipu_policy),
>> ATTR_LIST(min_ipu_util),
>> --
>> 2.14.0
>
> .
>