On 2018/7/7 9:10, Jaegeuk Kim wrote:
> On 07/07, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2018/7/7 6:49, Jaegeuk Kim wrote:
>>> On 07/05, Chao Yu wrote:
>>>> If discard IOs are blocked by user IO, do not skip to select and issue
>>>> discard with lower granularity, retry with current granularity.
>>>
>>> We need to stop as soon as possible since user activity comes. Later, 
>>> discard
>>> thread will try it again in another idle time. What's your concern?
>>
>> Currently, our implementation is that we will try to issue
>> DEF_MAX_DISCARD_REQUEST discards as many as possible in batch, for example:
>>
>> If there is 4 discard entry in rb-tree, we will try to issue them by below 
>> order
>> in batch:
>>
>> No.          size            is_idle         op
>> #1 discard   2MB             false           skip
>> #2 discard   2MB             false           skip
>> #3 discard   256KB           true            issue
>> #4 discard   16KB            true            issue
>>
>> So if is_idle is false temporarily, we can still have chance to issue #3 & #4
>> discard in this round once is_idle is flipped?
>>
>> But the problem is we skip issue discard with big granularity, so I add this
>> patch to do retry with current granularity.
>>
>> Do you mean that we need to stop issuing discard immediately once we detect 
>> IO
>> is busy?
> 
> Yes. If we retry to issue discard continuously, it hurts normal IO latencies.

OK, I will send another patch to change the logic to terminate issuing discard
immediately if there is queued IO.

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Chao Yu <yuch...@huawei.com>
>>>> ---
>>>> v2:
>>>> - fix deadloop.
>>>>  fs/f2fs/segment.c | 16 +++++++++++++---
>>>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 1f9ae8270f86..6e2b2e717a40 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -1191,7 +1191,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info 
>>>> *sbi,
>>>>    struct discard_cmd *dc, *tmp;
>>>>    struct blk_plug plug;
>>>>    int i, iter = 0, issued = 0;
>>>> -  bool io_interrupted = false;
>>>> +  bool io_interrupted = false, end_up = false;
>>>>  
>>>>    for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
>>>>            if (i + 1 < dpolicy->granularity)
>>>> @@ -1199,6 +1199,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info 
>>>> *sbi,
>>>>            pend_list = &dcc->pend_list[i];
>>>>  
>>>>            mutex_lock(&dcc->cmd_lock);
>>>> +retry:
>>>>            if (list_empty(pend_list))
>>>>                    goto next;
>>>>            if (unlikely(dcc->rbtree_check))
>>>> @@ -1217,14 +1218,23 @@ static int __issue_discard_cmd(struct f2fs_sb_info 
>>>> *sbi,
>>>>                    __submit_discard_cmd(sbi, dpolicy, dc);
>>>>                    issued++;
>>>>  skip:
>>>> -                  if (++iter >= dpolicy->max_requests)
>>>> +                  if (++iter >= dpolicy->max_requests) {
>>>> +                          end_up = true;
>>>>                            break;
>>>> +                  }
>>>>            }
>>>>            blk_finish_plug(&plug);
>>>> +
>>>> +          /*
>>>> +           * if discard IO was interrupted by user IOs, give another
>>>> +           * chance to issue discard with current granularity.
>>>> +           */
>>>> +          if (io_interrupted && !end_up)
>>>> +                  goto retry;
>>>>  next:
>>>>            mutex_unlock(&dcc->cmd_lock);
>>>>  
>>>> -          if (iter >= dpolicy->max_requests)
>>>> +          if (end_up)
>>>>                    break;
>>>>    }
>>>>  
>>>> -- 
>>>> 2.18.0.rc1

------------------------------------------------------------------------------
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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to