* [PATCH 1/3] f2fs: remove sleep_time under gc_urgent
@ 2019-05-14 6:36 Park Ju Hyung
2019-05-14 6:36 ` [PATCH 2/3] f2fs: don't wait with each discards " Park Ju Hyung
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Park Ju Hyung @ 2019-05-14 6:36 UTC (permalink / raw)
To: linux-f2fs-devel
gc_urgent is meant to be a hint from the user to force f2fs to run GC
aggressively, which means they are willing to take the hit on increased
latency during gc_urgent. It's meaningless to sleep between each GC under
gc_urgent, Not to mention that the default value of 500 ms makes gc_urgent
super ineffective.
Remove urgent_sleep_time entirely and allow GC to be finished much faster.
Use 1 for wait_ms instead of 0 to prevent possible CPU hoggings.
Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
---
fs/f2fs/gc.c | 3 +--
fs/f2fs/gc.h | 2 --
fs/f2fs/sysfs.c | 3 ---
3 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 963fb4571fd9..9c3ed89c8c5b 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -77,7 +77,7 @@ static int gc_thread_func(void *data)
* So, I'd like to wait some time to collect dirty segments.
*/
if (sbi->gc_mode == GC_URGENT) {
- wait_ms = gc_th->urgent_sleep_time;
+ wait_ms = 1;
mutex_lock(&sbi->gc_mutex);
goto do_gc;
}
@@ -129,7 +129,6 @@ int f2fs_start_gc_thread(struct f2fs_sb_info *sbi)
goto out;
}
- gc_th->urgent_sleep_time = DEF_GC_THREAD_URGENT_SLEEP_TIME;
gc_th->min_sleep_time = DEF_GC_THREAD_MIN_SLEEP_TIME;
gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
index bbac9d3787bd..de79a867837e 100644
--- a/fs/f2fs/gc.h
+++ b/fs/f2fs/gc.h
@@ -10,7 +10,6 @@
* whether IO subsystem is idle
* or not
*/
-#define DEF_GC_THREAD_URGENT_SLEEP_TIME 500 /* 500 ms */
#define DEF_GC_THREAD_MIN_SLEEP_TIME 30000 /* milliseconds */
#define DEF_GC_THREAD_MAX_SLEEP_TIME 60000
#define DEF_GC_THREAD_NOGC_SLEEP_TIME 300000 /* wait 5 min */
@@ -27,7 +26,6 @@ struct f2fs_gc_kthread {
wait_queue_head_t gc_wait_queue_head;
/* for gc sleep time */
- unsigned int urgent_sleep_time;
unsigned int min_sleep_time;
unsigned int max_sleep_time;
unsigned int no_gc_sleep_time;
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 729f46a3c9ee..0165431e83e5 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -397,8 +397,6 @@ static struct f2fs_attr f2fs_attr_##_name = { \
.id = _id, \
}
-F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_urgent_sleep_time,
- urgent_sleep_time);
F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_min_sleep_time, min_sleep_time);
F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_max_sleep_time, max_sleep_time);
F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
@@ -459,7 +457,6 @@ F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
#define ATTR_LIST(name) (&f2fs_attr_##name.attr)
static struct attribute *f2fs_attrs[] = {
- ATTR_LIST(gc_urgent_sleep_time),
ATTR_LIST(gc_min_sleep_time),
ATTR_LIST(gc_max_sleep_time),
ATTR_LIST(gc_no_gc_sleep_time),
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] f2fs: don't wait with each discards under gc_urgent
2019-05-14 6:36 [PATCH 1/3] f2fs: remove sleep_time under gc_urgent Park Ju Hyung
@ 2019-05-14 6:36 ` Park Ju Hyung
2019-05-14 6:36 ` [PATCH 3/3] f2fs: always assume that the device is idle " Park Ju Hyung
2019-05-14 9:20 ` [PATCH 1/3] f2fs: remove sleep_time " Chao Yu
2 siblings, 0 replies; 11+ messages in thread
From: Park Ju Hyung @ 2019-05-14 6:36 UTC (permalink / raw)
To: linux-f2fs-devel
f2fs already addresses gc_urgent under discard, but still uses the
default wait_ms.
To gain as many free segments as fast as possible,
use 1 for wait_ms under gc_urgent for discards.
Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
---
fs/f2fs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 8dee063c833f..dece5ebfb8f0 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1685,7 +1685,8 @@ static int issue_discard_thread(void *data)
wait_event_interruptible_timeout(*q,
kthread_should_stop() || freezing(current) ||
dcc->discard_wake,
- msecs_to_jiffies(wait_ms));
+ msecs_to_jiffies((sbi->gc_mode == GC_URGENT) ?
+ 1 : wait_ms));
if (dcc->discard_wake)
dcc->discard_wake = 0;
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] f2fs: always assume that the device is idle under gc_urgent
2019-05-14 6:36 [PATCH 1/3] f2fs: remove sleep_time under gc_urgent Park Ju Hyung
2019-05-14 6:36 ` [PATCH 2/3] f2fs: don't wait with each discards " Park Ju Hyung
@ 2019-05-14 6:36 ` Park Ju Hyung
2019-05-21 0:11 ` Jaegeuk Kim
2019-05-14 9:20 ` [PATCH 1/3] f2fs: remove sleep_time " Chao Yu
2 siblings, 1 reply; 11+ messages in thread
From: Park Ju Hyung @ 2019-05-14 6:36 UTC (permalink / raw)
To: linux-f2fs-devel
This allows more aggressive discards and balancing job to be done
under gc_urgent.
Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
---
fs/f2fs/f2fs.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 533fafca68f4..14c95116cd3d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2192,6 +2192,9 @@ static inline struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi,
static inline bool is_idle(struct f2fs_sb_info *sbi, int type)
{
+ if (sbi->gc_mode == GC_URGENT)
+ return true;
+
if (get_pages(sbi, F2FS_RD_DATA) || get_pages(sbi, F2FS_RD_NODE) ||
get_pages(sbi, F2FS_RD_META) || get_pages(sbi, F2FS_WB_DATA) ||
get_pages(sbi, F2FS_WB_CP_DATA) ||
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] f2fs: always assume that the device is idle under gc_urgent
2019-05-14 6:36 ` [PATCH 3/3] f2fs: always assume that the device is idle " Park Ju Hyung
@ 2019-05-21 0:11 ` Jaegeuk Kim
2019-05-22 12:10 ` Chao Yu
0 siblings, 1 reply; 11+ messages in thread
From: Jaegeuk Kim @ 2019-05-21 0:11 UTC (permalink / raw)
To: Park Ju Hyung; +Cc: linux-f2fs-devel
On 05/14, Park Ju Hyung wrote:
> This allows more aggressive discards and balancing job to be done
> under gc_urgent.
Let me merge this first. :)
>
> Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
> ---
> fs/f2fs/f2fs.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 533fafca68f4..14c95116cd3d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2192,6 +2192,9 @@ static inline struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi,
>
> static inline bool is_idle(struct f2fs_sb_info *sbi, int type)
> {
> + if (sbi->gc_mode == GC_URGENT)
> + return true;
> +
> if (get_pages(sbi, F2FS_RD_DATA) || get_pages(sbi, F2FS_RD_NODE) ||
> get_pages(sbi, F2FS_RD_META) || get_pages(sbi, F2FS_WB_DATA) ||
> get_pages(sbi, F2FS_WB_CP_DATA) ||
> --
> 2.21.0
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] f2fs: always assume that the device is idle under gc_urgent
2019-05-21 0:11 ` Jaegeuk Kim
@ 2019-05-22 12:10 ` Chao Yu
0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu @ 2019-05-22 12:10 UTC (permalink / raw)
To: Jaegeuk Kim, Park Ju Hyung; +Cc: linux-f2fs-devel
On 2019-5-21 8:11, Jaegeuk Kim wrote:
> On 05/14, Park Ju Hyung wrote:
>> This allows more aggressive discards and balancing job to be done
>> under gc_urgent.
>
> Let me merge this first. :)
>
>>
>> Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] f2fs: remove sleep_time under gc_urgent
2019-05-14 6:36 [PATCH 1/3] f2fs: remove sleep_time under gc_urgent Park Ju Hyung
2019-05-14 6:36 ` [PATCH 2/3] f2fs: don't wait with each discards " Park Ju Hyung
2019-05-14 6:36 ` [PATCH 3/3] f2fs: always assume that the device is idle " Park Ju Hyung
@ 2019-05-14 9:20 ` Chao Yu
[not found] ` <CAD14+f0Zia3oAi+QO+wCBrbV_=csp1SWB4BE7yN0h+=paZpg=w@mail.gmail.com>
2 siblings, 1 reply; 11+ messages in thread
From: Chao Yu @ 2019-05-14 9:20 UTC (permalink / raw)
To: Park Ju Hyung, linux-f2fs-devel
Hi Ju Hyung,
On 2019/5/14 14:36, Park Ju Hyung wrote:
> gc_urgent is meant to be a hint from the user to force f2fs to run GC
> aggressively, which means they are willing to take the hit on increased
> latency during gc_urgent. It's meaningless to sleep between each GC under
> gc_urgent, Not to mention that the default value of 500 ms makes gc_urgent
> super ineffective.
>
> Remove urgent_sleep_time entirely and allow GC to be finished much faster.
>
> Use 1 for wait_ms instead of 0 to prevent possible CPU hoggings.
IIRC, related interfaces (gc_urgent/urgent_sleep_time) were introduced for
Android Go, if some conditions (in userspace) are satisfied, GC/discard threads
are waked up via sysfs to clean filesystem/device space. Considering the system
runs in low-performance device, we can't expect that device can handle IOs very
quickly, so that once one of condition (e.g. screen is lighted up) breaks, apps
may be stuck due to racing with IO from GC. Anyway I think we need to set proper
interval for background GC, IMO, 1 ms is too short...
And it needs to wait for Jaegeuk's opinion.
Thanks,
>
> Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com>
> ---
> fs/f2fs/gc.c | 3 +--
> fs/f2fs/gc.h | 2 --
> fs/f2fs/sysfs.c | 3 ---
> 3 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 963fb4571fd9..9c3ed89c8c5b 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -77,7 +77,7 @@ static int gc_thread_func(void *data)
> * So, I'd like to wait some time to collect dirty segments.
> */
> if (sbi->gc_mode == GC_URGENT) {
> - wait_ms = gc_th->urgent_sleep_time;
> + wait_ms = 1;
> mutex_lock(&sbi->gc_mutex);
> goto do_gc;
> }
> @@ -129,7 +129,6 @@ int f2fs_start_gc_thread(struct f2fs_sb_info *sbi)
> goto out;
> }
>
> - gc_th->urgent_sleep_time = DEF_GC_THREAD_URGENT_SLEEP_TIME;
> gc_th->min_sleep_time = DEF_GC_THREAD_MIN_SLEEP_TIME;
> gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
> gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
> diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
> index bbac9d3787bd..de79a867837e 100644
> --- a/fs/f2fs/gc.h
> +++ b/fs/f2fs/gc.h
> @@ -10,7 +10,6 @@
> * whether IO subsystem is idle
> * or not
> */
> -#define DEF_GC_THREAD_URGENT_SLEEP_TIME 500 /* 500 ms */
> #define DEF_GC_THREAD_MIN_SLEEP_TIME 30000 /* milliseconds */
> #define DEF_GC_THREAD_MAX_SLEEP_TIME 60000
> #define DEF_GC_THREAD_NOGC_SLEEP_TIME 300000 /* wait 5 min */
> @@ -27,7 +26,6 @@ struct f2fs_gc_kthread {
> wait_queue_head_t gc_wait_queue_head;
>
> /* for gc sleep time */
> - unsigned int urgent_sleep_time;
> unsigned int min_sleep_time;
> unsigned int max_sleep_time;
> unsigned int no_gc_sleep_time;
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 729f46a3c9ee..0165431e83e5 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -397,8 +397,6 @@ static struct f2fs_attr f2fs_attr_##_name = { \
> .id = _id, \
> }
>
> -F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_urgent_sleep_time,
> - urgent_sleep_time);
> F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_min_sleep_time, min_sleep_time);
> F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_max_sleep_time, max_sleep_time);
> F2FS_RW_ATTR(GC_THREAD, f2fs_gc_kthread, gc_no_gc_sleep_time, no_gc_sleep_time);
> @@ -459,7 +457,6 @@ F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
>
> #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
> static struct attribute *f2fs_attrs[] = {
> - ATTR_LIST(gc_urgent_sleep_time),
> ATTR_LIST(gc_min_sleep_time),
> ATTR_LIST(gc_max_sleep_time),
> ATTR_LIST(gc_no_gc_sleep_time),
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-22 12:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14 6:36 [PATCH 1/3] f2fs: remove sleep_time under gc_urgent Park Ju Hyung
2019-05-14 6:36 ` [PATCH 2/3] f2fs: don't wait with each discards " Park Ju Hyung
2019-05-14 6:36 ` [PATCH 3/3] f2fs: always assume that the device is idle " Park Ju Hyung
2019-05-21 0:11 ` Jaegeuk Kim
2019-05-22 12:10 ` Chao Yu
2019-05-14 9:20 ` [PATCH 1/3] f2fs: remove sleep_time " Chao Yu
[not found] ` <CAD14+f0Zia3oAi+QO+wCBrbV_=csp1SWB4BE7yN0h+=paZpg=w@mail.gmail.com>
2019-05-14 11:19 ` Chao Yu
2019-05-14 11:30 ` Ju Hyung Park
2019-05-15 2:08 ` Chao Yu
2019-05-21 0:10 ` Jaegeuk Kim
2019-05-22 12:10 ` Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.