Re: [PATCH 1/3] f2fs: remove sleep_time under gc_urgent

[Chao Yu <chao@kernel.org>]

On 2019-5-21 8:10, Jaegeuk Kim wrote:
> On 05/15, Chao Yu wrote:
>> Hi Ju Hyung,
>>
>> On 2019/5/14 19:30, Ju Hyung Park wrote:
>>> Hi Chao,
>>>
>>> On Tue, May 14, 2019 at 8:19 PM Chao Yu <yuchao0@huawei.com> wrote:
>>>>> I've been using this(with a slightly different code) for years and yet to notice
>>>>> any spikes in lags/slowdowns. Worst scenario, I'd just have to deal with an
>>>>> added split second(100ms max?) delay in screen wake-up.
>>>>
>>>> I'm not sure about why this happened... maybe you need to do some test to
>>>> analyse the root cause of it, filesystem/device fragment? too many undiscard
>>>> space? or non-storage issue?
>>>
>>> Um, I'm not sure you understood what I said.
>>> What I meant is that I haven't found any issues with using an approach
>>> like this(gc_urgent with 1ms sleep intervals) for years on various
>>> Android devices.
>>
>> Ah, sorry, I misread what you said.
>>
>>>
>>>> I agreed that it should done as soon as possible, but it needs to consider IO
>>>> race in between Apps after screen wake-up and BGGC to avoid potential ANR.
>>>
>>> I actually need to check whether vold turns off gc_urgent immediately
>>> after screen turns itself back on.
>>> I don't think we need to take potential ANR in to account *if* vold
>>> stops gc_urgent right after screen-on. What do you think?
>>
>> What do you mean, I didn't catch it...
>>
>>>
>>>> It's userspace strategy, we can change both of them
>>>> (vold_wait_time/gc_urgent_sleep_time) in userspace if current value doesn't make
>>>> any sense.
>>>
>>> Even the user can set the tunables themselves, the default should be
>>> sensical imo.
>>
>> Agreed, how about adjusting this default value according device type, for fast
>> device, like SSD, we can set default interval to very small value.
>>
>> Maybe we can implement this base on below commit if you agreed.
>>
>> f2fs: support tunning for multiple kind of storage device
>>
>> Thanks,
>>
>>> An "urgent" GC that only GCs up-to 2 segments per second doesn't sound
>>> that "urgent" :p
> 
> Yes, it seems I set it too conservatively at that time. It'd be fine to decrease
> the default time, but I'd prefer to remain the sysfs entry in order for user to
> configure it just in case. It'd be also great to have the above Chao's patch to
> have some default values regarding to discard/GC policies.

Cool, let me rework on that patch. :)

Thanks,

> 
> Thanks,
> 
>>>
>>> Thanks.
>>> .
>>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>