* [f2fs-dev] [PATCH RESEND] f2fs: Modify do_garbage_collect() to collect all the segs in once @ 2013-07-12 5:51 Gu Zheng 2013-07-14 23:56 ` Jaegeuk Kim 0 siblings, 1 reply; 3+ messages in thread From: Gu Zheng @ 2013-07-12 5:51 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-f2fs-devel Current do_garbage_collect() collect per segment per time. If there are more than one segments in section, we need to call do_garbage_collect() many times to collect all the segments(current is a for loop). We can move the loop into the do_garbage_collect(), so that we can collect all the segs of section in one time. Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> --- fs/f2fs/gc.c | 59 ++++++++++++++++++++++++++++++++------------------------- 1 files changed, 33 insertions(+), 26 deletions(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 35f9b1a..9589ffe 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -634,42 +634,50 @@ static int __get_victim(struct f2fs_sb_info *sbi, unsigned int *victim, return ret; } -static void do_garbage_collect(struct f2fs_sb_info *sbi, unsigned int segno, - struct list_head *ilist, int gc_type) +static void do_garbage_collect(struct f2fs_sb_info *sbi, + unsigned int start_segno, struct list_head *ilist, int gc_type) { - struct page *sum_page; - struct f2fs_summary_block *sum; - struct blk_plug plug; + unsigned int segno = start_segno; - /* read segment summary of victim */ - sum_page = get_sum_page(sbi, segno); - if (IS_ERR(sum_page)) - return; + for (; sbi->segs_per_sec--; segno++) { + struct page *sum_page; + struct f2fs_summary_block *sum; + struct blk_plug plug; - blk_start_plug(&plug); + /* read segment summary of victim */ + sum_page = get_sum_page(sbi, segno); + if (IS_ERR(sum_page)) + continue; - sum = page_address(sum_page); + blk_start_plug(&plug); - switch (GET_SUM_TYPE((&sum->footer))) { - case SUM_TYPE_NODE: - gc_node_segment(sbi, sum->entries, segno, gc_type); - break; - case SUM_TYPE_DATA: - gc_data_segment(sbi, sum->entries, ilist, segno, gc_type); - break; - } - blk_finish_plug(&plug); + sum = page_address(sum_page); - stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); - stat_inc_call_count(sbi->stat_info); + switch (GET_SUM_TYPE((&sum->footer))) { + case SUM_TYPE_NODE: + gc_node_segment(sbi, sum->entries, + segno, gc_type); + break; + case SUM_TYPE_DATA: + gc_data_segment(sbi, sum->entries, ilist, + segno, gc_type); + break; + default: + BUG(); + } + blk_finish_plug(&plug); + + stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); + stat_inc_call_count(sbi->stat_info); - f2fs_put_page(sum_page, 1); + f2fs_put_page(sum_page, 1); + } } int f2fs_gc(struct f2fs_sb_info *sbi) { struct list_head ilist; - unsigned int segno, i; + unsigned int segno; int gc_type = BG_GC; int nfree = 0; int ret = -1; @@ -688,8 +696,7 @@ gc_more: goto stop; ret = 0; - for (i = 0; i < sbi->segs_per_sec; i++) - do_garbage_collect(sbi, segno + i, &ilist, gc_type); + do_garbage_collect(sbi, segno, &ilist, gc_type); if (gc_type == FG_GC) { sbi->cur_victim_sec = NULL_SEGNO; -- 1.7.7 ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH RESEND] f2fs: Modify do_garbage_collect() to collect all the segs in once 2013-07-12 5:51 [f2fs-dev] [PATCH RESEND] f2fs: Modify do_garbage_collect() to collect all the segs in once Gu Zheng @ 2013-07-14 23:56 ` Jaegeuk Kim 2013-07-15 1:47 ` Gu Zheng 0 siblings, 1 reply; 3+ messages in thread From: Jaegeuk Kim @ 2013-07-14 23:56 UTC (permalink / raw) To: Gu Zheng; +Cc: linux-f2fs-devel Hi Gu, Sorry for the delay. Could you explain why this should be applied? IMO, nothing was different: no removal of loops, no better code readability, no performance improvement. Rather than that, it seems that it'd be better that do_garbage_collect conducts GC just for one segment. Any opinion? Thanks, 2013-07-12 (금), 13:51 +0800, Gu Zheng: > Current do_garbage_collect() collect per segment per time. If there are more > than one segments in section, we need to call do_garbage_collect() many times to > collect all the segments(current is a for loop). We can move the loop into the > do_garbage_collect(), so that we can collect all the segs of section in one time. > > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > --- > fs/f2fs/gc.c | 59 ++++++++++++++++++++++++++++++++------------------------- > 1 files changed, 33 insertions(+), 26 deletions(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 35f9b1a..9589ffe 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -634,42 +634,50 @@ static int __get_victim(struct f2fs_sb_info *sbi, unsigned > int *victim, > return ret; > } > > -static void do_garbage_collect(struct f2fs_sb_info *sbi, unsigned int segno, > - struct list_head *ilist, int gc_type) > +static void do_garbage_collect(struct f2fs_sb_info *sbi, > + unsigned int start_segno, struct list_head *ilist, int gc_type) > { > - struct page *sum_page; > - struct f2fs_summary_block *sum; > - struct blk_plug plug; > + unsigned int segno = start_segno; > > - /* read segment summary of victim */ > - sum_page = get_sum_page(sbi, segno); > - if (IS_ERR(sum_page)) > - return; > + for (; sbi->segs_per_sec--; segno++) { > + struct page *sum_page; > + struct f2fs_summary_block *sum; > + struct blk_plug plug; > > - blk_start_plug(&plug); > + /* read segment summary of victim */ > + sum_page = get_sum_page(sbi, segno); > + if (IS_ERR(sum_page)) > + continue; > > - sum = page_address(sum_page); > + blk_start_plug(&plug); > > - switch (GET_SUM_TYPE((&sum->footer))) { > - case SUM_TYPE_NODE: > - gc_node_segment(sbi, sum->entries, segno, gc_type); > - break; > - case SUM_TYPE_DATA: > - gc_data_segment(sbi, sum->entries, ilist, segno, gc_type); > - break; > - } > - blk_finish_plug(&plug); > + sum = page_address(sum_page); > > - stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); > - stat_inc_call_count(sbi->stat_info); > + switch (GET_SUM_TYPE((&sum->footer))) { > + case SUM_TYPE_NODE: > + gc_node_segment(sbi, sum->entries, > + segno, gc_type); > + break; > + case SUM_TYPE_DATA: > + gc_data_segment(sbi, sum->entries, ilist, > + segno, gc_type); > + break; > + default: > + BUG(); > + } > + blk_finish_plug(&plug); > + > + stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); > + stat_inc_call_count(sbi->stat_info); > > - f2fs_put_page(sum_page, 1); > + f2fs_put_page(sum_page, 1); > + } > } > > int f2fs_gc(struct f2fs_sb_info *sbi) > { > struct list_head ilist; > - unsigned int segno, i; > + unsigned int segno; > int gc_type = BG_GC; > int nfree = 0; > int ret = -1; > @@ -688,8 +696,7 @@ gc_more: > goto stop; > ret = 0; > > - for (i = 0; i < sbi->segs_per_sec; i++) > - do_garbage_collect(sbi, segno + i, &ilist, gc_type); > + do_garbage_collect(sbi, segno, &ilist, gc_type); > > if (gc_type == FG_GC) { > sbi->cur_victim_sec = NULL_SEGNO; -- Jaegeuk Kim Samsung ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [f2fs-dev] [PATCH RESEND] f2fs: Modify do_garbage_collect() to collect all the segs in once 2013-07-14 23:56 ` Jaegeuk Kim @ 2013-07-15 1:47 ` Gu Zheng 0 siblings, 0 replies; 3+ messages in thread From: Gu Zheng @ 2013-07-15 1:47 UTC (permalink / raw) To: jaegeuk.kim; +Cc: linux-f2fs-devel On 07/15/2013 07:56 AM, Jaegeuk Kim wrote: > Hi Gu, > Hi Kim, > Sorry for the delay. > Could you explain why this should be applied? > > IMO, nothing was different: no removal of loops, no better code > readability, no performance improvement. > Rather than that, it seems that it'd be better that do_garbage_collect > conducts GC just for one segment. > Any opinion? Yeah, in the default case, per section contains one segment. But we can set more than one segments in one section, in this case, we need to call do_garbage_collect() many times to collect all the segments. If we move the loop into do_garbage_collect(), we only need to call do_garbage_collect() one time, no matter how many segments in one section. I think this is a improvement, though it seems tiny.:) If you think this change is needless, please feel free to ignore it.:) Best regards, Gu > > Thanks, > > 2013-07-12 (금), 13:51 +0800, Gu Zheng: >> Current do_garbage_collect() collect per segment per time. If there are more >> than one segments in section, we need to call do_garbage_collect() many times to >> collect all the segments(current is a for loop). We can move the loop into the >> do_garbage_collect(), so that we can collect all the segs of section in one time. >> >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> >> --- >> fs/f2fs/gc.c | 59 ++++++++++++++++++++++++++++++++------------------------- >> 1 files changed, 33 insertions(+), 26 deletions(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 35f9b1a..9589ffe 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -634,42 +634,50 @@ static int __get_victim(struct f2fs_sb_info *sbi, unsigned >> int *victim, >> return ret; >> } >> >> -static void do_garbage_collect(struct f2fs_sb_info *sbi, unsigned int segno, >> - struct list_head *ilist, int gc_type) >> +static void do_garbage_collect(struct f2fs_sb_info *sbi, >> + unsigned int start_segno, struct list_head *ilist, int gc_type) >> { >> - struct page *sum_page; >> - struct f2fs_summary_block *sum; >> - struct blk_plug plug; >> + unsigned int segno = start_segno; >> >> - /* read segment summary of victim */ >> - sum_page = get_sum_page(sbi, segno); >> - if (IS_ERR(sum_page)) >> - return; >> + for (; sbi->segs_per_sec--; segno++) { >> + struct page *sum_page; >> + struct f2fs_summary_block *sum; >> + struct blk_plug plug; >> >> - blk_start_plug(&plug); >> + /* read segment summary of victim */ >> + sum_page = get_sum_page(sbi, segno); >> + if (IS_ERR(sum_page)) >> + continue; >> >> - sum = page_address(sum_page); >> + blk_start_plug(&plug); >> >> - switch (GET_SUM_TYPE((&sum->footer))) { >> - case SUM_TYPE_NODE: >> - gc_node_segment(sbi, sum->entries, segno, gc_type); >> - break; >> - case SUM_TYPE_DATA: >> - gc_data_segment(sbi, sum->entries, ilist, segno, gc_type); >> - break; >> - } >> - blk_finish_plug(&plug); >> + sum = page_address(sum_page); >> >> - stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); >> - stat_inc_call_count(sbi->stat_info); >> + switch (GET_SUM_TYPE((&sum->footer))) { >> + case SUM_TYPE_NODE: >> + gc_node_segment(sbi, sum->entries, >> + segno, gc_type); >> + break; >> + case SUM_TYPE_DATA: >> + gc_data_segment(sbi, sum->entries, ilist, >> + segno, gc_type); >> + break; >> + default: >> + BUG(); >> + } >> + blk_finish_plug(&plug); >> + >> + stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer))); >> + stat_inc_call_count(sbi->stat_info); >> >> - f2fs_put_page(sum_page, 1); >> + f2fs_put_page(sum_page, 1); >> + } >> } >> >> int f2fs_gc(struct f2fs_sb_info *sbi) >> { >> struct list_head ilist; >> - unsigned int segno, i; >> + unsigned int segno; >> int gc_type = BG_GC; >> int nfree = 0; >> int ret = -1; >> @@ -688,8 +696,7 @@ gc_more: >> goto stop; >> ret = 0; >> >> - for (i = 0; i < sbi->segs_per_sec; i++) >> - do_garbage_collect(sbi, segno + i, &ilist, gc_type); >> + do_garbage_collect(sbi, segno, &ilist, gc_type); >> >> if (gc_type == FG_GC) { >> sbi->cur_victim_sec = NULL_SEGNO; > ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-07-15 1:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-12 5:51 [f2fs-dev] [PATCH RESEND] f2fs: Modify do_garbage_collect() to collect all the segs in once Gu Zheng 2013-07-14 23:56 ` Jaegeuk Kim 2013-07-15 1:47 ` Gu Zheng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.