1. Forum
  2. Usenet
  3. LINUX.KERNEL

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

    From =?utf-8?Q?Javier_Gonz=C3=A1lez?=@21:1/5 to All on Mon Oct 2 13:30:02 2017
    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

    -----BEGIN PGP SIGNATURE-----

    iQIzBAEBCgAdFiEEm1mT7zen+vs9+T8kYx8FO3WZGMoFAlnSIq4ACgkQYx8FO3WZ GMo6Og//UIEjQGgxNm5Ewi/eA+vmBixWmrLsr20CiNsB1H/85Q5Lh9aEZDlThxyz HAq6DTwbEMZ13MceF9ZaFG+/3q2F4emR4VvVBsPN4iqRhAsinh+4arxUCDef+442 pTdpKik4H6cZCDXc3T3/rvi503Fq5PzgXRuoaRqDPZYCrAIJ1XRx7DT2GSNzaL2I PRh7hBfzrO7Q4M0jLkOYgX6ylUTN
  • From =?utf-8?Q?Javier_Gonz=C3=A1lez?=@21:1/5 to All on Mon Oct 2 13:50:02 2017
    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

    -----BEGIN PGP SIGNATURE-----

    iQIzBAEBCgAdFiEEm1mT7zen+vs9+T8kYx8FO3WZGMoFAlnSJqMACgkQYx8FO3WZ GMpruRAA3mQCua56fhSvDs30qB94NwS36SraQCQXfig1/S0GEVyDFiU4+9Tby45K IhAkgQBh16BKNYRseWx1D8b63Ayh1bPJ0wXU0LzcJyNcTYp8CWgIguShK1F4TIiX J/513i4V16knbZKDCqy2eJYy+YCYPwEJiAFA3OtqljO6LEpW3JrZk4qUvVPPB1Th KGXbywvHeTvjWaC5D1feyjJZpZ1o9xGYIPw9JaiAMQv0GYDoJZtaTBPY+6Di1qzQ hOaffR3FBWpGCWMP/Ue3PzIuwQ/U1AQaM/UQBoSldfjcCOBAdIYwHC1kVD7xrLgx cGbDSU0Zo9f6gVTbXiOLiMrYAKpkZ5pNOTDBp0nT8K1dqM6yWSOkgaqh0qeEZASG KKphFxSN1SNe2Z1vbRh4HUkYeCIQJvPSj+AxzDv2oKdtiRDMxZ55rnMMz/QcnOr7 YPTAaK/g5JuEfF5FfWH7SrezDy4HKI2ovYgBJgkuTsqnB6vZqIa84U861ViHl9BS wuexdFN0kwmWxFRvcWga4YekUiYvup4kZzXZCIE/5zNv9RLNQz8aAtwf9kP5LXE2 l7GVAIVFVEHODscWTp01OfMExY+7OxXIWnNqIL4T2TGE/Q5/ArMSFFWslwk47F5X lBPVu2fTbVR5tmewk1bhClFQDO8xg0uG8PskronPYJUpmsiqSX8=
    =Qp3t
    -----END PGP SIGNATURE-----

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Rakesh Pandit@21:1/5 to All on Mon Oct 2 13:50:02 2017
    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,

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)