| Message ID | 5656C93D-F3DF-4054-906C-C27542D265CA@lightnvm.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 02, 2017 at 01:27:42PM +0200, Javier González wrote: > > On 28 Sep 2017, at 16.40, Rakesh Pandit <rakesh@tuxera.com> wrote: > > > > pblk_line_gc_list seems to had a bug since the introduction of pblk in > > getting GC list for a line. In b20ba1bc7 while redesigning GC > > algorithm it was not fixed correctly. The problem is that even if > > valid sector count (vsc) is less that mid_thrs (threshold for GC mid > > list) it would always satisfy 'vsc < high_thrs' as high_thrs > > > mid_thrs always. > > > > Fixes: a4bd217b4("lightnvm: physical block device (pblk) target") > > Signed-off-by: Rakesh Pandit <rakesh@tuxera.com> > > --- > > drivers/lightnvm/pblk-core.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > > index 8150164..93a58ed 100644 > > --- a/drivers/lightnvm/pblk-core.c > > +++ b/drivers/lightnvm/pblk-core.c > > @@ -295,16 +295,16 @@ struct list_head *pblk_line_gc_list(struct pblk *pblk, struct pblk_line *line) > > line->gc_group = PBLK_LINEGC_FULL; > > move_list = &l_mg->gc_full_list; > > } > > - } else if (vsc < lm->high_thrs) { > > - if (line->gc_group != PBLK_LINEGC_HIGH) { > > - line->gc_group = PBLK_LINEGC_HIGH; > > - move_list = &l_mg->gc_high_list; > > - } > > } else if (vsc < lm->mid_thrs) { > > if (line->gc_group != PBLK_LINEGC_MID) { > > line->gc_group = PBLK_LINEGC_MID; > > move_list = &l_mg->gc_mid_list; > > } > > + } else if (vsc < lm->high_thrs) { > > + if (line->gc_group != PBLK_LINEGC_HIGH) { > > + line->gc_group = PBLK_LINEGC_HIGH; > > + move_list = &l_mg->gc_high_list; > > + } > > } else if (vsc < line->sec_in_line) { > > if (line->gc_group != PBLK_LINEGC_LOW) { > > line->gc_group = PBLK_LINEGC_LOW; > > -- > > 2.9.3 > > You're right that the naming for high/mid/low was not updated when > aligning vsc with GC thresholds. But the fix should be making high, > being high, instead of reordering when moving into the GC bucket. > > Does it make sense to you? Yes. > > diff --git i/drivers/lightnvm/pblk-init.c w/drivers/lightnvm/pblk-init.c > index 7cf4b536d899..bc5c6cc12ad5 100644 > --- i/drivers/lightnvm/pblk-init.c > +++ w/drivers/lightnvm/pblk-init.c > @@ -675,8 +675,8 @@ static int pblk_lines_init(struct pblk *pblk) > lm->blk_bitmap_len = BITS_TO_LONGS(geo->nr_luns) * sizeof(long); > lm->sec_bitmap_len = BITS_TO_LONGS(lm->sec_per_line) * sizeof(long); > lm->lun_bitmap_len = BITS_TO_LONGS(geo->nr_luns) * sizeof(long); > - lm->high_thrs = lm->sec_per_line / 2; > - lm->mid_thrs = lm->sec_per_line / 4; > + lm->high_thrs = lm->sec_per_line / 4; > + lm->mid_thrs = lm->sec_per_line / 2; > lm->meta_distance = (geo->nr_luns / 2) * pblk->min_write_pgs; > > /* Calculate necessary pages for smeta. See comment over struct Regards,
> On 2 Oct 2017, at 13.43, Rakesh Pandit <rakesh@tuxera.com> wrote: > > On Mon, Oct 02, 2017 at 01:27:42PM +0200, Javier González wrote: >>> On 28 Sep 2017, at 16.40, Rakesh Pandit <rakesh@tuxera.com> wrote: >>> >>> pblk_line_gc_list seems to had a bug since the introduction of pblk in >>> getting GC list for a line. In b20ba1bc7 while redesigning GC >>> algorithm it was not fixed correctly. The problem is that even if >>> valid sector count (vsc) is less that mid_thrs (threshold for GC mid >>> list) it would always satisfy 'vsc < high_thrs' as high_thrs > >>> mid_thrs always. >>> >>> Fixes: a4bd217b4("lightnvm: physical block device (pblk) target") >>> Signed-off-by: Rakesh Pandit <rakesh@tuxera.com> >>> --- >>> drivers/lightnvm/pblk-core.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >>> index 8150164..93a58ed 100644 >>> --- a/drivers/lightnvm/pblk-core.c >>> +++ b/drivers/lightnvm/pblk-core.c >>> @@ -295,16 +295,16 @@ struct list_head *pblk_line_gc_list(struct pblk *pblk, struct pblk_line *line) >>> line->gc_group = PBLK_LINEGC_FULL; >>> move_list = &l_mg->gc_full_list; >>> } >>> - } else if (vsc < lm->high_thrs) { >>> - if (line->gc_group != PBLK_LINEGC_HIGH) { >>> - line->gc_group = PBLK_LINEGC_HIGH; >>> - move_list = &l_mg->gc_high_list; >>> - } >>> } else if (vsc < lm->mid_thrs) { >>> if (line->gc_group != PBLK_LINEGC_MID) { >>> line->gc_group = PBLK_LINEGC_MID; >>> move_list = &l_mg->gc_mid_list; >>> } >>> + } else if (vsc < lm->high_thrs) { >>> + if (line->gc_group != PBLK_LINEGC_HIGH) { >>> + line->gc_group = PBLK_LINEGC_HIGH; >>> + move_list = &l_mg->gc_high_list; >>> + } >>> } else if (vsc < line->sec_in_line) { >>> if (line->gc_group != PBLK_LINEGC_LOW) { >>> line->gc_group = PBLK_LINEGC_LOW; >>> -- >>> 2.9.3 >> >> You're right that the naming for high/mid/low was not updated when >> aligning vsc with GC thresholds. But the fix should be making high, >> being high, instead of reordering when moving into the GC bucket. >> >> Does it make sense to you? > > Yes. > Cool. I'll fix it when picking it up. Javier
diff --git i/drivers/lightnvm/pblk-init.c w/drivers/lightnvm/pblk-init.c index 7cf4b536d899..bc5c6cc12ad5 100644 --- i/drivers/lightnvm/pblk-init.c +++ w/drivers/lightnvm/pblk-init.c @@ -675,8 +675,8 @@ static int pblk_lines_init(struct pblk *pblk) lm->blk_bitmap_len = BITS_TO_LONGS(geo->nr_luns) * sizeof(long); lm->sec_bitmap_len = BITS_TO_LONGS(lm->sec_per_line) * sizeof(long); lm->lun_bitmap_len = BITS_TO_LONGS(geo->nr_luns) * sizeof(long); - lm->high_thrs = lm->sec_per_line / 2; - lm->mid_thrs = lm->sec_per_line / 4; + lm->high_thrs = lm->sec_per_line / 4; + lm->mid_thrs = lm->sec_per_line / 2; lm->meta_distance = (geo->nr_luns / 2) * pblk->min_write_pgs; /* Calculate necessary pages for smeta. See comment over struct