linux-kernel - Re: [PATCH] lightnvm: pblk: fix changing GC group list for a line

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <5656C93D-F3DF-4054-906C-C27542D265CA@lightnvm.io>
Date:   Mon, 2 Oct 2017 13:27:42 +0200
From:   Javier González <jg@...htnvm.io>
To:     Rakesh Pandit <rakesh@...era.com>
Cc:     Matias Bjørling <mb@...htnvm.io>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lightnvm: pblk: fix changing GC group list for a line

> On 28 Sep 2017, at 16.40, Rakesh Pandit <rakesh@...era.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@...era.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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)