Re: [PATCH 3/3] f2fs: provide more chance for node and data to get ssr segment

[Yunlong Song]

The get_ssr_segment happens when need_SSR is true, that is to say we must make sure we can
achieve a right available SSR segment at least, so why not select the type with larger probability?
And I think warm and hot are more similar with each other, maybe the update frequency is difference,
but if time goes by, warm segments will finally update their blocks like hot segments, but cold segments
are something like which should and will never update, so I prefer to put hot and warm to one side,
and put cold to the other side.

On 2017/2/25 4:05, Jaegeuk Kim wrote:
> On 02/25, Yunlong Song wrote:
>> HiïJaegeuk and Chao,
>>     SorryïI misunderstood CURSEG_HOT_DATA is greater than CURSEG_WARM_DATA and use "i--" in the codes, so just forget that.
>>     But there is still a issue, when type is CURSEG_WARM_DATA, chao's patch will select cold SSR segment first, but my patch will select hot SSR segment first, since I think the probability of having hot SSR segment is bigger than having cold SSR segment due to the temperature itself.
> One thing that I'm seeing is, theoretically hot segments will get dirty more
> frequently. So if we select dirty cold segments for warm data, we can gain
> fully valid cold segments more, which can mitigate log thrashing problem.
>
> Thanks,
>
>>
>> On 02/25/2017 01:58, Jaegeuk Kim wrote:
>> On 02/24, Yunlong Song wrote:
>>> Hi, Chao,
>>>
>>> Not looks good to me, since there is some case your code does not include:
>>> if type is CURSEG_HOT_DATA, and if get_victim also returns 0 for both CURSEG_HOT_DATA and
>>> CURSEG_WARM_DATA, then i will be -1 and pass to get_victim in your code.  So I still suggest
>>> my original patch attached below.
>> Why does i become -1?
>>
>>> On 2017/2/24 18:47, Chao Yu wrote:
>>>> On 2017/2/24 17:19, Yunlong Song wrote:
>>>>> Hi Jaegeuk and Chao,
>>>>>
>>>>>     How about the question I pointed out in last mail:
>>>>>     Why not take "neighboring temperature" for ssr? For example, if type == CURSEG_COLD_DATA,
>>>>>     the new patch selects CURSEG_HOT_DATA first, why not select CURSEG_WARM_DATA first?
>>>>>     The patch I sent ensure this "neighboring temperature" for ssr. This is to reduce the influence of
>>>>>     mixing different levels of hot/code node types.
>>>> Agreed, I sent one patch for changing the policy of SSR, how do you think of it?
>>>>
>>>> Thanks,
>>>>
>>>>> On 2017/2/24 17:05, Chao Yu wrote:
>>>>>> Hi Jaegeuk,
>>>>>>
>>>>>> Reviewed-by: Chao Yu <yuchao0@xxxxxxxxxx>
>>>>>>
>>>>>> For your attached two patches.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> On 2017/2/23 9:17, Jaegeuk Kim wrote:
>>>>>>> Hi Yunlong,
>>>>>>>
>>>>>>> I've been testing the similar patches as I attached.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> On 02/22, Yunlong Song wrote:
>>>>>>>> Signed-off-by: Yunlong Song <yunlong.song@xxxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  fs/f2fs/segment.c | 9 +++++++++
>>>>>>>>  1 file changed, 9 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>>>>> index 9d13743..5fe71b9 100644
>>>>>>>> --- a/fs/f2fs/segment.c
>>>>>>>> +++ b/fs/f2fs/segment.c
>>>>>>>> @@ -1540,12 +1540,17 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi, int type)
>>>>>>>>  {
>>>>>>>>      struct curseg_info *curseg = CURSEG_I(sbi, type);
>>>>>>>>      const struct victim_selection *v_ops = DIRTY_I(sbi)->v_ops;
>>>>>>>> +    int old_type = type;
>>>>>>>>  
>>>>>>>>      if (IS_NODESEG(type)) {
>>>>>>>>          for (; type >= CURSEG_HOT_NODE; type--)
>>>>>>>>              if (v_ops->get_victim(sbi, &(curseg)->next_segno,
>>>>>>>>                              BG_GC, type, SSR))
>>>>>>>>                  return 1;
>>>>>>>> +        for (type = old_type + 1; type <= CURSEG_COLD_NODE; type++)
>>>>>>>> +            if (v_ops->get_victim(sbi, &(curseg)->next_segno,
>>>>>>>> +                            BG_GC, type, SSR))
>>>>>>>> +                return 1;
>>>>>>>>          return 0;
>>>>>>>>      }
>>>>>>>>  
>>>>>>>> @@ -1554,6 +1559,10 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi, int type)
>>>>>>>>          if (v_ops->get_victim(sbi, &(curseg)->next_segno,
>>>>>>>>                          BG_GC, type, SSR))
>>>>>>>>              return 1;
>>>>>>>> +    for (type = old_type + 1; type <= CURSEG_COLD_DATA; type++)
>>>>>>>> +        if (v_ops->get_victim(sbi, &(curseg)->next_segno,
>>>>>>>> +                        BG_GC, type, SSR))
>>>>>>>> +            return 1;
>>>>>>>>      return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> --
>>>>>>>> 1.8.5.2
>>>>>> .
>>>>>>
>>>> .
>>>>
>>>
>>> --
>>> Thanks,
>>> Yunlong Song
>>>
> .
>


-- 
Thanks,
Yunlong Song