On 02/28, Chao Yu wrote:
> On 2017/2/28 7:49, Jaegeuk Kim wrote:
> > On 02/27, Jaegeuk Kim wrote:
> >> On 02/27, Chao Yu wrote:
> >>> On 2017/2/26 3:56, Jaegeuk Kim wrote:
> >>>> On 02/25, guoweichao wrote:
> >>>>> Hi Jaegeuk,
> >>>>>
> >>>>> I regard no enough free sections as a precondition when talking about
> >>>>> BG_GC -> FG_GC. I mean that for both case a) and b) I mentioned has no 
> >>>>> enough
> >>>>> free sections implicitly. 
> >>>>>
> >>>>> On 2017/2/25 2:49, Jaegeuk Kim wrote:
> >>>>>> Hi Weichao,
> >>>>>>
> >>>>>> On 02/25, Weichao Guo wrote:
> >>>>>>> When turning to FG_GC from BG_GC, we need to write checkpoint in 2 
> >>>>>>> cases:
> >>>>>>> * a) BG_GC have made some progress, e.g.: some prefree segments.
> >>>>>>> * b) There is no victim and no prefree segment.
> >>>>>>
> >>>>>> You missed
> >>>>>>   * c) has_not_enough_free_secs() introduced by
> >>>>>>       6e17bfbc75a5cb ("f2fs: fix to overcome inline_data floods")
> >>>>> As we have enabled SSR for warm node(5b6c6be2d8 ("f2fs: use SSR for 
> >>>>> warm node as well")),
> >>>>> I think inline data floods should not be a problem in most cases.
> >>>>>>
> >>>>>> And, Yunlong pointed that we can't find a case to avoid 
> >>>>>> write_checkpoint()
> >>>>>> mostly due to c) condition.
> >>>>> As inline data floods is an extreme case, and there is little 
> >>>>> possibility caused panic
> >>>>> for inline data floods now, there should be lots of chance to skip 
> >>>>> checkpoint. I mean
> >>>>> that we can make some accurate inline data floods checking before 
> >>>>> writing checkpoint.
> >>>>
> >>>> For now, the safest way is our first option. So, I decided to start with 
> >>>> doing
> >>>> checkpoint due to previous inline_data flooding issue even though it's an
> >>>> extreme case under SSR.
> >>>>
> >>>> Anyway, I agree that we need to find a way to detect when to avoid 
> >>>> checkpoint.
> >>>
> >>> Hi all,
> >>>
> >>> I proposed a approach before, can you please check that one?
> >>>
> >>> https://www.mail-archive.com/linux-f2fs-devel@lists.sourceforge.net/msg03632.html
> >>
> >> Oh, right, let's take a look at this. ;)
> > 
> > Hmm, I just read this patch again, and realized it doesn't quite address the
> > current issue. This patch flushes inline_data inodes in background, which 
> > does
> > not guarantee this worst case. The key idea would be how to measure the 
> > space
> 
> Hmm.. Maybe we can cover worst case by moving judgment condition and flushing
> operation into f2fs_balance_fs.
> 
> > we can do SSR and use it in has_not_enough_free_secs().
> 
> We need to stat usage of slack free space accurately both for data/node, 
> right?

Yup.
Thanks,

> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> >>
> >> Thanks,
> >>
> >>>
> >>> Thanks,
> >>>
> >>>>
> >>>> Thanks,
> >>>>
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>>>
> >>>>>>> For case a), previously, we also check if there is a dirty segment for
> >>>>>>> infering blocks moving in last BG_GC. But dirty segments do not always
> >>>>>>> indicate that, BG_GC may just start and do not move any blocks at all.
> >>>>>>> Futhermore, skipping checkpoint if there are some dirty segments but 
> >>>>>>> no
> >>>>>>> prefree segments is OK.
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Signed-off-by: Weichao Guo <guoweic...@huawei.com>
> >>>>>>> ---
> >>>>>>>  fs/f2fs/gc.c | 7 ++++++-
> >>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>>>>>> index 6c996e3..30d206a 100644
> >>>>>>> --- a/fs/f2fs/gc.c
> >>>>>>> +++ b/fs/f2fs/gc.c
> >>>>>>> @@ -958,7 +958,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, 
> >>>>>>> bool background)
> >>>>>>>                * enough free sections, we should flush dent/node 
> >>>>>>> blocks and do
> >>>>>>>                * garbage collections.
> >>>>>>>                */
> >>>>>>> -             ret = write_checkpoint(sbi, &cpc);
> >>>>>>> +             if (prefree_segments(sbi))
> >>>>>>> +                     ret = write_checkpoint(sbi, &cpc);
> >>>>>>> +             else if (!__get_victim(sbi, &segno, gc_type) {
> >>>>>>> +                     segno = NULL_SEGNO;
> >>>>>>> +                     ret = write_checkpoint(sbi, &cpc);
> >>>>>>> +             }
> >>>>>>>               if (ret)
> >>>>>>>                       goto stop;
> >>>>>>>       } else if (gc_type == BG_GC && !background) {
> >>>>>>> -- 
> >>>>>>> 2.10.1
> >>>>>>
> >>>>>> .
> >>>>>>
> >>>>>
> >>>>> Thanks,
> >>>>> Weichao
> >>>>
> >>>> .
> >>>>
> >>
> >> ------------------------------------------------------------------------------
> >> 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
> > 
> > .
> > 

------------------------------------------------------------------------------
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