Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741606 > unrolled thread

[PATCH] lightnvm: pblk: fix changing GC group list for a line

Started byRakesh Pandit <rakesh@tuxera.com>
First post2017-09-28 16:50 +0200
Last post2017-10-02 13:50 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] lightnvm: pblk: fix changing GC group list for a line Rakesh Pandit <rakesh@tuxera.com> - 2017-09-28 16:50 +0200
    Re: [PATCH] lightnvm: pblk: fix changing GC group list for a line Javier González <jg@lightnvm.io> - 2017-10-02 13:30 +0200
      Re: [PATCH] lightnvm: pblk: fix changing GC group list for a line Javier González <jg@lightnvm.io> - 2017-10-02 13:50 +0200
      Re: [PATCH] lightnvm: pblk: fix changing GC group list for a line Rakesh Pandit <rakesh@tuxera.com> - 2017-10-02 13:50 +0200

#1741606 — [PATCH] lightnvm: pblk: fix changing GC group list for a line

FromRakesh Pandit <rakesh@tuxera.com>
Date2017-09-28 16:50 +0200
Subject[PATCH] lightnvm: pblk: fix changing GC group list for a line
Message-ID<uuIiu-1KL-5@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1743128

FromJavier González <jg@lightnvm.io>
Date2017-10-02 13:30 +0200
Message-ID<uw758-6FT-11@gated-at.bofh.it>
In reply to#1741606

[Multipart message — attachments visible in raw view] — view raw

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

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

[toc] | [prev] | [next] | [standalone]

#1743136

FromJavier González <jg@lightnvm.io>
Date2017-10-02 13:50 +0200
Message-ID<uw7ou-6Mk-11@gated-at.bofh.it>
In reply to#1743128

[Multipart message — attachments visible in raw view] — view raw

> 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

[toc] | [prev] | [next] | [standalone]

#1743138

FromRakesh Pandit <rakesh@tuxera.com>
Date2017-10-02 13:50 +0200
Message-ID<uw7ou-6Mk-13@gated-at.bofh.it>
In reply to#1743128
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,

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web