On Sun, Jul 24, 2016 at 06:38:46AM +0800, Chao Yu wrote:
...

> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index de6c41c..ec17096 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -908,10 +908,14 @@ gc_more:
> >>             * enough free sections, we should flush dent/node blocks and do
> >>             * garbage collections.
> >>             */
> >> -          if (__get_victim(sbi, &segno, gc_type) || prefree_segments(sbi))
> >> +          if (__get_victim(sbi, &segno, gc_type) || 
> >> prefree_segments(sbi)) {
> >>                    write_checkpoint(sbi, &cpc);
> >> -          else if (has_not_enough_free_secs(sbi, 0))
> > 
> > If segno is NULL_SEGNO, we get a panic when checking the below conditions.
> > Anyway, I don't think we need this condition at all.
> 
> I thinks the condition which trigger our f2fs_bug_on is really a corner case,
> we'd better not waste the victim we found everytime, especially for these
> victims from last background gc's victims.
> 
> What about checking segno first?
> 
> if(segno != NULL_SEGNO && (!get_valid_blocks(sbi, segno, sbi->segs_per_sec) ||
>       IS_CURSEC(sbi, GET_SECNO(sbi, segno)))
>       segno = NULL_SEGNO;

My viewpoint is write_checkpoint() flushes node and dentry pages, which provides
another better victims. Another concern is get_valid_blocks and IS_CURSEG are
not covered by any lock.

Thanks,

> 
> Thanks,
> 
> > Let me remove them and just set NULL_SEGNO only after checkpoint.
> > Please check the dev-test repo.
> > 
> > Thanks,
> > 
> >> +                  if(!get_valid_blocks(sbi, segno, sbi->segs_per_sec)
> >> +                                  || IS_CURSEC(sbi, segno / 
> >> sbi->segs_per_sec))
> >> +                          segno = NULL_SEGNO;
> >> +          } else if (has_not_enough_free_secs(sbi, 0)) {
> >>                    write_checkpoint(sbi, &cpc);
> >> +          }
> >>    }
> >>  
> >>    if (segno == NULL_SEGNO && !__get_victim(sbi, &segno, gc_type))
> >> -- 
> >> 1.9.1
> > 
> > ------------------------------------------------------------------------------
> > What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> > patterns at an interface-level. Reveals which users, apps, and protocols 
> > are 
> > consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> > J-Flow, sFlow and other flows. Make informed decisions using capacity 
> > planning
> > reports.http://sdm.link/zohodev2dev
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to