On 2 Oct 2017, at 13.43, Rakesh Pandit [off-list ref] wrote:

On Mon, Oct 02, 2017 at 01:27:42PM +0200, Javier González wrote:

quoted

quoted

On 28 Sep 2017, at 16.40, Rakesh Pandit [off-list ref] 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 <redacted>
---
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