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

[Javier GonzÃlez]

> On 2 Oct 2017, at 13.43, Rakesh Pandit <rakesh@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>> 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

Attachment: signature.asc
Description: Message signed with OpenPGP