All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] lightnvm: pblk: fix race condition on GC
@ 2019-02-01  2:38 Heiner Litz
  2019-02-04  8:19 ` Javier González
  2019-02-05  9:14 ` Matias Bjørling
  0 siblings, 2 replies; 4+ messages in thread
From: Heiner Litz @ 2019-02-01  2:38 UTC (permalink / raw)
  To: mb; +Cc: javier, hans.holmberg, linux-block, linux-kernel, Heiner Litz

This patch fixes a race condition where a write is mapped to the last
sectors of a line. The write is synced to the device but the L2P is not
updated yet. When the line is garbage collected before the L2P update is
performed, the sectors are ignored by the GC logic and the line is freed
before all sectors are moved. When the L2P is finally updated, it contains
a mapping to a freed line, subsequent reads of the corresponding LBAs fail.

This patch introduces a per line counter specifying the number of sectors
that are synced to the device but have not been updated in the L2P. Lines
with a counter of greater than zero will not be selected for GC.

Signed-off-by: Heiner Litz <hlitz@ucsc.edu>
---

v2: changed according to Javier's comment. Instead of performing check
while holding the trans_lock, add an atomic per line counter

 drivers/lightnvm/pblk-core.c  |  1 +
 drivers/lightnvm/pblk-gc.c    | 20 +++++++++++++-------
 drivers/lightnvm/pblk-map.c   |  1 +
 drivers/lightnvm/pblk-rb.c    |  1 +
 drivers/lightnvm/pblk-write.c |  1 +
 drivers/lightnvm/pblk.h       |  1 +
 6 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index eabcbc119681..b7ed0502abef 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1278,6 +1278,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
 	spin_unlock(&line->lock);
 
 	kref_init(&line->ref);
+	atomic_set(&line->sec_to_update, 0);
 
 	return 0;
 }
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 2fa118c8eb71..26a52ea7ec45 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -365,16 +365,22 @@ static struct pblk_line *pblk_gc_get_victim_line(struct pblk *pblk,
 						 struct list_head *group_list)
 {
 	struct pblk_line *line, *victim;
-	int line_vsc, victim_vsc;
+	unsigned int line_vsc = ~0x0L, victim_vsc = ~0x0L;
 
 	victim = list_first_entry(group_list, struct pblk_line, list);
+
 	list_for_each_entry(line, group_list, list) {
-		line_vsc = le32_to_cpu(*line->vsc);
-		victim_vsc = le32_to_cpu(*victim->vsc);
-		if (line_vsc < victim_vsc)
+		if (!atomic_read(&line->sec_to_update))
+			line_vsc = le32_to_cpu(*line->vsc);
+		if (line_vsc < victim_vsc) {
 			victim = line;
+			victim_vsc = le32_to_cpu(*victim->vsc);
+		}
 	}
 
+	if (victim_vsc == ~0x0)
+		return NULL;
+
 	return victim;
 }
 
@@ -448,13 +454,13 @@ static void pblk_gc_run(struct pblk *pblk)
 
 	do {
 		spin_lock(&l_mg->gc_lock);
-		if (list_empty(group_list)) {
+
+		line = pblk_gc_get_victim_line(pblk, group_list);
+		if (!line) {
 			spin_unlock(&l_mg->gc_lock);
 			break;
 		}
 
-		line = pblk_gc_get_victim_line(pblk, group_list);
-
 		spin_lock(&line->lock);
 		WARN_ON(line->state != PBLK_LINESTATE_CLOSED);
 		line->state = PBLK_LINESTATE_GC;
diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
index 79df583ea709..7fbc99b60cac 100644
--- a/drivers/lightnvm/pblk-map.c
+++ b/drivers/lightnvm/pblk-map.c
@@ -73,6 +73,7 @@ static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
 		 */
 		if (i < valid_secs) {
 			kref_get(&line->ref);
+			atomic_inc(&line->sec_to_update);
 			w_ctx = pblk_rb_w_ctx(&pblk->rwb, sentry + i);
 			w_ctx->ppa = ppa_list[i];
 			meta->lba = cpu_to_le64(w_ctx->lba);
diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index a6133b50ed9c..03c241b340ea 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -260,6 +260,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
 							entry->cacheline);
 
 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
+		atomic_dec(&line->sec_to_update);
 		kref_put(&line->ref, pblk_line_put);
 		clean_wctx(w_ctx);
 		rb->l2p_update = pblk_rb_ptr_wrap(rb, rb->l2p_update, 1);
diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index 06d56deb645d..6593deab52da 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -177,6 +177,7 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
 		 * re-map these entries
 		 */
 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
+		atomic_dec(&line->sec_to_update);
 		kref_put(&line->ref, pblk_line_put);
 	}
 	spin_unlock(&pblk->trans_lock);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index a6386d5acd73..ac3ab778e976 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -487,6 +487,7 @@ struct pblk_line {
 	__le32 *vsc;			/* Valid sector count in line */
 
 	struct kref ref;		/* Write buffer L2P references */
+	atomic_t sec_to_update;         /* Outstanding L2P updates to ppa */
 
 	struct pblk_w_err_gc *w_err_gc;	/* Write error gc recovery metadata */
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] lightnvm: pblk: fix race condition on GC
  2019-02-01  2:38 [PATCH V2] lightnvm: pblk: fix race condition on GC Heiner Litz
@ 2019-02-04  8:19 ` Javier González
  2019-02-04 21:01   ` Hans Holmberg
  2019-02-05  9:14 ` Matias Bjørling
  1 sibling, 1 reply; 4+ messages in thread
From: Javier González @ 2019-02-04  8:19 UTC (permalink / raw)
  To: Heiner Litz
  Cc: Matias Bjørling, Hans Holmberg, linux-block, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5315 bytes --]

> On 1 Feb 2019, at 03.38, Heiner Litz <hlitz@ucsc.edu> wrote:
> 
> This patch fixes a race condition where a write is mapped to the last
> sectors of a line. The write is synced to the device but the L2P is not
> updated yet. When the line is garbage collected before the L2P update is
> performed, the sectors are ignored by the GC logic and the line is freed
> before all sectors are moved. When the L2P is finally updated, it contains
> a mapping to a freed line, subsequent reads of the corresponding LBAs fail.
> 
> This patch introduces a per line counter specifying the number of sectors
> that are synced to the device but have not been updated in the L2P. Lines
> with a counter of greater than zero will not be selected for GC.
> 
> Signed-off-by: Heiner Litz <hlitz@ucsc.edu>
> ---
> 
> v2: changed according to Javier's comment. Instead of performing check
> while holding the trans_lock, add an atomic per line counter
> 
> drivers/lightnvm/pblk-core.c  |  1 +
> drivers/lightnvm/pblk-gc.c    | 20 +++++++++++++-------
> drivers/lightnvm/pblk-map.c   |  1 +
> drivers/lightnvm/pblk-rb.c    |  1 +
> drivers/lightnvm/pblk-write.c |  1 +
> drivers/lightnvm/pblk.h       |  1 +
> 6 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index eabcbc119681..b7ed0502abef 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1278,6 +1278,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
> 	spin_unlock(&line->lock);
> 
> 	kref_init(&line->ref);
> +	atomic_set(&line->sec_to_update, 0);
> 
> 	return 0;
> }
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 2fa118c8eb71..26a52ea7ec45 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -365,16 +365,22 @@ static struct pblk_line *pblk_gc_get_victim_line(struct pblk *pblk,
> 						 struct list_head *group_list)
> {
> 	struct pblk_line *line, *victim;
> -	int line_vsc, victim_vsc;
> +	unsigned int line_vsc = ~0x0L, victim_vsc = ~0x0L;
> 
> 	victim = list_first_entry(group_list, struct pblk_line, list);
> +
> 	list_for_each_entry(line, group_list, list) {
> -		line_vsc = le32_to_cpu(*line->vsc);
> -		victim_vsc = le32_to_cpu(*victim->vsc);
> -		if (line_vsc < victim_vsc)
> +		if (!atomic_read(&line->sec_to_update))
> +			line_vsc = le32_to_cpu(*line->vsc);
> +		if (line_vsc < victim_vsc) {
> 			victim = line;
> +			victim_vsc = le32_to_cpu(*victim->vsc);
> +		}
> 	}
> 
> +	if (victim_vsc == ~0x0)
> +		return NULL;
> +
> 	return victim;
> }
> 
> @@ -448,13 +454,13 @@ static void pblk_gc_run(struct pblk *pblk)
> 
> 	do {
> 		spin_lock(&l_mg->gc_lock);
> -		if (list_empty(group_list)) {
> +
> +		line = pblk_gc_get_victim_line(pblk, group_list);
> +		if (!line) {
> 			spin_unlock(&l_mg->gc_lock);
> 			break;
> 		}
> 
> -		line = pblk_gc_get_victim_line(pblk, group_list);
> -
> 		spin_lock(&line->lock);
> 		WARN_ON(line->state != PBLK_LINESTATE_CLOSED);
> 		line->state = PBLK_LINESTATE_GC;
> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
> index 79df583ea709..7fbc99b60cac 100644
> --- a/drivers/lightnvm/pblk-map.c
> +++ b/drivers/lightnvm/pblk-map.c
> @@ -73,6 +73,7 @@ static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
> 		 */
> 		if (i < valid_secs) {
> 			kref_get(&line->ref);
> +			atomic_inc(&line->sec_to_update);
> 			w_ctx = pblk_rb_w_ctx(&pblk->rwb, sentry + i);
> 			w_ctx->ppa = ppa_list[i];
> 			meta->lba = cpu_to_le64(w_ctx->lba);
> diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
> index a6133b50ed9c..03c241b340ea 100644
> --- a/drivers/lightnvm/pblk-rb.c
> +++ b/drivers/lightnvm/pblk-rb.c
> @@ -260,6 +260,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
> 							entry->cacheline);
> 
> 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> +		atomic_dec(&line->sec_to_update);
> 		kref_put(&line->ref, pblk_line_put);
> 		clean_wctx(w_ctx);
> 		rb->l2p_update = pblk_rb_ptr_wrap(rb, rb->l2p_update, 1);
> diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> index 06d56deb645d..6593deab52da 100644
> --- a/drivers/lightnvm/pblk-write.c
> +++ b/drivers/lightnvm/pblk-write.c
> @@ -177,6 +177,7 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
> 		 * re-map these entries
> 		 */
> 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> +		atomic_dec(&line->sec_to_update);
> 		kref_put(&line->ref, pblk_line_put);
> 	}
> 	spin_unlock(&pblk->trans_lock);
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index a6386d5acd73..ac3ab778e976 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -487,6 +487,7 @@ struct pblk_line {
> 	__le32 *vsc;			/* Valid sector count in line */
> 
> 	struct kref ref;		/* Write buffer L2P references */
> +	atomic_t sec_to_update;         /* Outstanding L2P updates to ppa */
> 
> 	struct pblk_w_err_gc *w_err_gc;	/* Write error gc recovery metadata */
> 
> --
> 2.17.1


Looks good to me. Again, good marathon-catch! :)

Reviewed-by: Javier González <javier@javigon.com>



[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] lightnvm: pblk: fix race condition on GC
  2019-02-04  8:19 ` Javier González
@ 2019-02-04 21:01   ` Hans Holmberg
  0 siblings, 0 replies; 4+ messages in thread
From: Hans Holmberg @ 2019-02-04 21:01 UTC (permalink / raw)
  To: Javier González
  Cc: Heiner Litz, Matias Bjørling, Hans Holmberg, linux-block,
	Linux Kernel Mailing List

Thanks Heiner, it's awesome to get this fixed!

Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>

On Mon, Feb 4, 2019 at 9:19 AM Javier González <javier@javigon.com> wrote:
>
> > On 1 Feb 2019, at 03.38, Heiner Litz <hlitz@ucsc.edu> wrote:
> >
> > This patch fixes a race condition where a write is mapped to the last
> > sectors of a line. The write is synced to the device but the L2P is not
> > updated yet. When the line is garbage collected before the L2P update is
> > performed, the sectors are ignored by the GC logic and the line is freed
> > before all sectors are moved. When the L2P is finally updated, it contains
> > a mapping to a freed line, subsequent reads of the corresponding LBAs fail.
> >
> > This patch introduces a per line counter specifying the number of sectors
> > that are synced to the device but have not been updated in the L2P. Lines
> > with a counter of greater than zero will not be selected for GC.
> >
> > Signed-off-by: Heiner Litz <hlitz@ucsc.edu>
> > ---
> >
> > v2: changed according to Javier's comment. Instead of performing check
> > while holding the trans_lock, add an atomic per line counter
> >
> > drivers/lightnvm/pblk-core.c  |  1 +
> > drivers/lightnvm/pblk-gc.c    | 20 +++++++++++++-------
> > drivers/lightnvm/pblk-map.c   |  1 +
> > drivers/lightnvm/pblk-rb.c    |  1 +
> > drivers/lightnvm/pblk-write.c |  1 +
> > drivers/lightnvm/pblk.h       |  1 +
> > 6 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> > index eabcbc119681..b7ed0502abef 100644
> > --- a/drivers/lightnvm/pblk-core.c
> > +++ b/drivers/lightnvm/pblk-core.c
> > @@ -1278,6 +1278,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
> >       spin_unlock(&line->lock);
> >
> >       kref_init(&line->ref);
> > +     atomic_set(&line->sec_to_update, 0);
> >
> >       return 0;
> > }
> > diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> > index 2fa118c8eb71..26a52ea7ec45 100644
> > --- a/drivers/lightnvm/pblk-gc.c
> > +++ b/drivers/lightnvm/pblk-gc.c
> > @@ -365,16 +365,22 @@ static struct pblk_line *pblk_gc_get_victim_line(struct pblk *pblk,
> >                                                struct list_head *group_list)
> > {
> >       struct pblk_line *line, *victim;
> > -     int line_vsc, victim_vsc;
> > +     unsigned int line_vsc = ~0x0L, victim_vsc = ~0x0L;
> >
> >       victim = list_first_entry(group_list, struct pblk_line, list);
> > +
> >       list_for_each_entry(line, group_list, list) {
> > -             line_vsc = le32_to_cpu(*line->vsc);
> > -             victim_vsc = le32_to_cpu(*victim->vsc);
> > -             if (line_vsc < victim_vsc)
> > +             if (!atomic_read(&line->sec_to_update))
> > +                     line_vsc = le32_to_cpu(*line->vsc);
> > +             if (line_vsc < victim_vsc) {
> >                       victim = line;
> > +                     victim_vsc = le32_to_cpu(*victim->vsc);
> > +             }
> >       }
> >
> > +     if (victim_vsc == ~0x0)
> > +             return NULL;
> > +
> >       return victim;
> > }
> >
> > @@ -448,13 +454,13 @@ static void pblk_gc_run(struct pblk *pblk)
> >
> >       do {
> >               spin_lock(&l_mg->gc_lock);
> > -             if (list_empty(group_list)) {
> > +
> > +             line = pblk_gc_get_victim_line(pblk, group_list);
> > +             if (!line) {
> >                       spin_unlock(&l_mg->gc_lock);
> >                       break;
> >               }
> >
> > -             line = pblk_gc_get_victim_line(pblk, group_list);
> > -
> >               spin_lock(&line->lock);
> >               WARN_ON(line->state != PBLK_LINESTATE_CLOSED);
> >               line->state = PBLK_LINESTATE_GC;
> > diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
> > index 79df583ea709..7fbc99b60cac 100644
> > --- a/drivers/lightnvm/pblk-map.c
> > +++ b/drivers/lightnvm/pblk-map.c
> > @@ -73,6 +73,7 @@ static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
> >                */
> >               if (i < valid_secs) {
> >                       kref_get(&line->ref);
> > +                     atomic_inc(&line->sec_to_update);
> >                       w_ctx = pblk_rb_w_ctx(&pblk->rwb, sentry + i);
> >                       w_ctx->ppa = ppa_list[i];
> >                       meta->lba = cpu_to_le64(w_ctx->lba);
> > diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
> > index a6133b50ed9c..03c241b340ea 100644
> > --- a/drivers/lightnvm/pblk-rb.c
> > +++ b/drivers/lightnvm/pblk-rb.c
> > @@ -260,6 +260,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
> >                                                       entry->cacheline);
> >
> >               line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> > +             atomic_dec(&line->sec_to_update);
> >               kref_put(&line->ref, pblk_line_put);
> >               clean_wctx(w_ctx);
> >               rb->l2p_update = pblk_rb_ptr_wrap(rb, rb->l2p_update, 1);
> > diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> > index 06d56deb645d..6593deab52da 100644
> > --- a/drivers/lightnvm/pblk-write.c
> > +++ b/drivers/lightnvm/pblk-write.c
> > @@ -177,6 +177,7 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
> >                * re-map these entries
> >                */
> >               line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> > +             atomic_dec(&line->sec_to_update);
> >               kref_put(&line->ref, pblk_line_put);
> >       }
> >       spin_unlock(&pblk->trans_lock);
> > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> > index a6386d5acd73..ac3ab778e976 100644
> > --- a/drivers/lightnvm/pblk.h
> > +++ b/drivers/lightnvm/pblk.h
> > @@ -487,6 +487,7 @@ struct pblk_line {
> >       __le32 *vsc;                    /* Valid sector count in line */
> >
> >       struct kref ref;                /* Write buffer L2P references */
> > +     atomic_t sec_to_update;         /* Outstanding L2P updates to ppa */
> >
> >       struct pblk_w_err_gc *w_err_gc; /* Write error gc recovery metadata */
> >
> > --
> > 2.17.1
>
>
> Looks good to me. Again, good marathon-catch! :)
>
> Reviewed-by: Javier González <javier@javigon.com>
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] lightnvm: pblk: fix race condition on GC
  2019-02-01  2:38 [PATCH V2] lightnvm: pblk: fix race condition on GC Heiner Litz
  2019-02-04  8:19 ` Javier González
@ 2019-02-05  9:14 ` Matias Bjørling
  1 sibling, 0 replies; 4+ messages in thread
From: Matias Bjørling @ 2019-02-05  9:14 UTC (permalink / raw)
  To: Heiner Litz; +Cc: javier, hans.holmberg, linux-block, linux-kernel

On 2/1/19 3:38 AM, Heiner Litz wrote:
> This patch fixes a race condition where a write is mapped to the last
> sectors of a line. The write is synced to the device but the L2P is not
> updated yet. When the line is garbage collected before the L2P update is
> performed, the sectors are ignored by the GC logic and the line is freed
> before all sectors are moved. When the L2P is finally updated, it contains
> a mapping to a freed line, subsequent reads of the corresponding LBAs fail.
> 
> This patch introduces a per line counter specifying the number of sectors
> that are synced to the device but have not been updated in the L2P. Lines
> with a counter of greater than zero will not be selected for GC.
> 
> Signed-off-by: Heiner Litz <hlitz@ucsc.edu>
> ---
> 
> v2: changed according to Javier's comment. Instead of performing check
> while holding the trans_lock, add an atomic per line counter
> 
>   drivers/lightnvm/pblk-core.c  |  1 +
>   drivers/lightnvm/pblk-gc.c    | 20 +++++++++++++-------
>   drivers/lightnvm/pblk-map.c   |  1 +
>   drivers/lightnvm/pblk-rb.c    |  1 +
>   drivers/lightnvm/pblk-write.c |  1 +
>   drivers/lightnvm/pblk.h       |  1 +
>   6 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index eabcbc119681..b7ed0502abef 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1278,6 +1278,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
>   	spin_unlock(&line->lock);
>   
>   	kref_init(&line->ref);
> +	atomic_set(&line->sec_to_update, 0);
>   
>   	return 0;
>   }
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 2fa118c8eb71..26a52ea7ec45 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -365,16 +365,22 @@ static struct pblk_line *pblk_gc_get_victim_line(struct pblk *pblk,
>   						 struct list_head *group_list)
>   {
>   	struct pblk_line *line, *victim;
> -	int line_vsc, victim_vsc;
> +	unsigned int line_vsc = ~0x0L, victim_vsc = ~0x0L;
>   
>   	victim = list_first_entry(group_list, struct pblk_line, list);
> +
>   	list_for_each_entry(line, group_list, list) {
> -		line_vsc = le32_to_cpu(*line->vsc);
> -		victim_vsc = le32_to_cpu(*victim->vsc);
> -		if (line_vsc < victim_vsc)
> +		if (!atomic_read(&line->sec_to_update))
> +			line_vsc = le32_to_cpu(*line->vsc);
> +		if (line_vsc < victim_vsc) {
>   			victim = line;
> +			victim_vsc = le32_to_cpu(*victim->vsc);
> +		}
>   	}
>   
> +	if (victim_vsc == ~0x0)
> +		return NULL;
> +
>   	return victim;
>   }
>   
> @@ -448,13 +454,13 @@ static void pblk_gc_run(struct pblk *pblk)
>   
>   	do {
>   		spin_lock(&l_mg->gc_lock);
> -		if (list_empty(group_list)) {
> +
> +		line = pblk_gc_get_victim_line(pblk, group_list);
> +		if (!line) {
>   			spin_unlock(&l_mg->gc_lock);
>   			break;
>   		}
>   
> -		line = pblk_gc_get_victim_line(pblk, group_list);
> -
>   		spin_lock(&line->lock);
>   		WARN_ON(line->state != PBLK_LINESTATE_CLOSED);
>   		line->state = PBLK_LINESTATE_GC;
> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
> index 79df583ea709..7fbc99b60cac 100644
> --- a/drivers/lightnvm/pblk-map.c
> +++ b/drivers/lightnvm/pblk-map.c
> @@ -73,6 +73,7 @@ static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
>   		 */
>   		if (i < valid_secs) {
>   			kref_get(&line->ref);
> +			atomic_inc(&line->sec_to_update);
>   			w_ctx = pblk_rb_w_ctx(&pblk->rwb, sentry + i);
>   			w_ctx->ppa = ppa_list[i];
>   			meta->lba = cpu_to_le64(w_ctx->lba);
> diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
> index a6133b50ed9c..03c241b340ea 100644
> --- a/drivers/lightnvm/pblk-rb.c
> +++ b/drivers/lightnvm/pblk-rb.c
> @@ -260,6 +260,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
>   							entry->cacheline);
>   
>   		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> +		atomic_dec(&line->sec_to_update);
>   		kref_put(&line->ref, pblk_line_put);
>   		clean_wctx(w_ctx);
>   		rb->l2p_update = pblk_rb_ptr_wrap(rb, rb->l2p_update, 1);
> diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> index 06d56deb645d..6593deab52da 100644
> --- a/drivers/lightnvm/pblk-write.c
> +++ b/drivers/lightnvm/pblk-write.c
> @@ -177,6 +177,7 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
>   		 * re-map these entries
>   		 */
>   		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
> +		atomic_dec(&line->sec_to_update);
>   		kref_put(&line->ref, pblk_line_put);
>   	}
>   	spin_unlock(&pblk->trans_lock);
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index a6386d5acd73..ac3ab778e976 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -487,6 +487,7 @@ struct pblk_line {
>   	__le32 *vsc;			/* Valid sector count in line */
>   
>   	struct kref ref;		/* Write buffer L2P references */
> +	atomic_t sec_to_update;         /* Outstanding L2P updates to ppa */
>   
>   	struct pblk_w_err_gc *w_err_gc;	/* Write error gc recovery metadata */
>   
> 

Thanks Heiner. Applied for 5.1.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-05  9:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01  2:38 [PATCH V2] lightnvm: pblk: fix race condition on GC Heiner Litz
2019-02-04  8:19 ` Javier González
2019-02-04 21:01   ` Hans Holmberg
2019-02-05  9:14 ` Matias Bjørling

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.