> 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
