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] [day] [month] [year] [list]

Message-Id: <01F673AF-1707-4717-A822-3A554E4EA229@lightnvm.io>
Date:   Mon, 2 Oct 2017 13:44:35 +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 2 Oct 2017, at 13.43, Rakesh Pandit <rakesh@...era.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@...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?
> 
> Yes.
> 

Cool. I'll fix it when picking it up.

Javier

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