* [PATCH 1/9] lightnvm: pblk: prevent gc kicks when gc is not operational
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:27 ` Javier González
2017-10-03 10:05 ` [PATCH 2/9] lightnvm: pblk: recover partially written lines correctly Hans Holmberg
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
GC can be kicked after it has been shut down when closing the last
line during exit, resulting in accesses to freed structures.
Make sure that GC is not triggered while it is not operational.
Also make sure that GC won't be re-activated during exit when
running on another processor by using timer_del_sync.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-gc.c | 9 +++++----
drivers/lightnvm/pblk-init.c | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 7b103bc..81efac1 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -478,10 +478,10 @@ void pblk_gc_should_start(struct pblk *pblk)
{
struct pblk_gc *gc = &pblk->gc;
- if (gc->gc_enabled && !gc->gc_active)
+ if (gc->gc_enabled && !gc->gc_active) {
pblk_gc_start(pblk);
-
- pblk_gc_kick(pblk);
+ pblk_gc_kick(pblk);
+ }
}
/*
@@ -620,7 +620,8 @@ void pblk_gc_exit(struct pblk *pblk)
flush_workqueue(gc->gc_reader_wq);
flush_workqueue(gc->gc_line_reader_wq);
- del_timer(&gc->gc_timer);
+ gc->gc_enabled = 0;
+ del_timer_sync(&gc->gc_timer);
pblk_gc_stop(pblk, 1);
if (gc->gc_ts)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 0163914..ec7974b 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -928,6 +928,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
pblk->dev = dev;
pblk->disk = tdisk;
pblk->state = PBLK_STATE_RUNNING;
+ pblk->gc.gc_enabled = 0;
spin_lock_init(&pblk->trans_lock);
spin_lock_init(&pblk->lock);
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/9] lightnvm: pblk: prevent gc kicks when gc is not operational
2017-10-03 10:05 ` [PATCH 1/9] lightnvm: pblk: prevent gc kicks when gc is not operational Hans Holmberg
@ 2017-10-03 10:27 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:27 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> GC can be kicked after it has been shut down when closing the last
> line during exit, resulting in accesses to freed structures.
>
> Make sure that GC is not triggered while it is not operational.
> Also make sure that GC won't be re-activated during exit when
> running on another processor by using timer_del_sync.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-gc.c | 9 +++++----
> drivers/lightnvm/pblk-init.c | 1 +
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 7b103bc..81efac1 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -478,10 +478,10 @@ void pblk_gc_should_start(struct pblk *pblk)
> {
> struct pblk_gc *gc = &pblk->gc;
>
> - if (gc->gc_enabled && !gc->gc_active)
> + if (gc->gc_enabled && !gc->gc_active) {
> pblk_gc_start(pblk);
> -
> - pblk_gc_kick(pblk);
> + pblk_gc_kick(pblk);
> + }
> }
>
> /*
> @@ -620,7 +620,8 @@ void pblk_gc_exit(struct pblk *pblk)
> flush_workqueue(gc->gc_reader_wq);
> flush_workqueue(gc->gc_line_reader_wq);
>
> - del_timer(&gc->gc_timer);
> + gc->gc_enabled = 0;
> + del_timer_sync(&gc->gc_timer);
> pblk_gc_stop(pblk, 1);
>
> if (gc->gc_ts)
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 0163914..ec7974b 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -928,6 +928,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
> pblk->dev = dev;
> pblk->disk = tdisk;
> pblk->state = PBLK_STATE_RUNNING;
> + pblk->gc.gc_enabled = 0;
>
> spin_lock_init(&pblk->trans_lock);
> spin_lock_init(&pblk->lock);
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/9] lightnvm: pblk: recover partially written lines correctly
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
2017-10-03 10:05 ` [PATCH 1/9] lightnvm: pblk: prevent gc kicks when gc is not operational Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:28 ` Javier González
2017-10-03 10:05 ` [PATCH 3/9] lightnvm: pblk: free full lines during recovery Hans Holmberg
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
When recovering partially written lines, the valid sector
count must be decreased by the number of padded sectors
in the line.
Update line recovery to take all ADDR_EMPTY(padded) sectors
into account.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-recovery.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 1869eef..74b3b86 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -133,8 +133,8 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
struct pblk_emeta *emeta = line->emeta;
struct line_emeta *emeta_buf = emeta->buf;
__le64 *lba_list;
- int data_start;
- int nr_data_lbas, nr_valid_lbas, nr_lbas = 0;
+ int data_start, data_end;
+ int nr_valid_lbas, nr_lbas = 0;
int i;
lba_list = pblk_recov_get_lba_list(pblk, emeta_buf);
@@ -142,10 +142,10 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
return 1;
data_start = pblk_line_smeta_start(pblk, line) + lm->smeta_sec;
- nr_data_lbas = lm->sec_per_line - lm->emeta_sec[0];
+ data_end = lm->sec_per_line - lm->emeta_sec[0];
nr_valid_lbas = le64_to_cpu(emeta_buf->nr_valid_lbas);
- for (i = data_start; i < nr_data_lbas && nr_lbas < nr_valid_lbas; i++) {
+ for (i = data_start; i < data_end; i++) {
struct ppa_addr ppa;
int pos;
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/9] lightnvm: pblk: recover partially written lines correctly
2017-10-03 10:05 ` [PATCH 2/9] lightnvm: pblk: recover partially written lines correctly Hans Holmberg
@ 2017-10-03 10:28 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:28 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> When recovering partially written lines, the valid sector
> count must be decreased by the number of padded sectors
> in the line.
>
> Update line recovery to take all ADDR_EMPTY(padded) sectors
> into account.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-recovery.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 1869eef..74b3b86 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -133,8 +133,8 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> struct pblk_emeta *emeta = line->emeta;
> struct line_emeta *emeta_buf = emeta->buf;
> __le64 *lba_list;
> - int data_start;
> - int nr_data_lbas, nr_valid_lbas, nr_lbas = 0;
> + int data_start, data_end;
> + int nr_valid_lbas, nr_lbas = 0;
> int i;
>
> lba_list = pblk_recov_get_lba_list(pblk, emeta_buf);
> @@ -142,10 +142,10 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> return 1;
>
> data_start = pblk_line_smeta_start(pblk, line) + lm->smeta_sec;
> - nr_data_lbas = lm->sec_per_line - lm->emeta_sec[0];
> + data_end = lm->sec_per_line - lm->emeta_sec[0];
> nr_valid_lbas = le64_to_cpu(emeta_buf->nr_valid_lbas);
>
> - for (i = data_start; i < nr_data_lbas && nr_lbas < nr_valid_lbas; i++) {
> + for (i = data_start; i < data_end; i++) {
> struct ppa_addr ppa;
> int pos;
>
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/9] lightnvm: pblk: free full lines during recovery
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
2017-10-03 10:05 ` [PATCH 1/9] lightnvm: pblk: prevent gc kicks when gc is not operational Hans Holmberg
2017-10-03 10:05 ` [PATCH 2/9] lightnvm: pblk: recover partially written lines correctly Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:36 ` Javier González
2017-10-03 10:05 ` [PATCH 4/9] lightnvm: pblk: start gc if needed during init Hans Holmberg
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
When rebuilding the L2P table, any full lines(lines without any
valid sectors) will be identified. If these lines are not freed,
we risk not being able to allocate the first data line.
This patch refactors the part of GC that frees empty lines
into a separate function and adds a call to this after the
L2P table has been rebuilt.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-gc.c | 32 ++++++++++++++++++++------------
drivers/lightnvm/pblk-init.c | 3 +++
drivers/lightnvm/pblk.h | 1 +
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 81efac1..374089f 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -330,26 +330,16 @@ static bool pblk_gc_should_run(struct pblk_gc *gc, struct pblk_rl *rl)
return ((gc->gc_active) && (nr_blocks_need > nr_blocks_free));
}
-/*
- * Lines with no valid sectors will be returned to the free list immediately. If
- * GC is activated - either because the free block count is under the determined
- * threshold, or because it is being forced from user space - only lines with a
- * high count of invalid sectors will be recycled.
- */
-static void pblk_gc_run(struct pblk *pblk)
+void pblk_gc_free_full_lines(struct pblk *pblk)
{
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
- struct pblk_gc *gc = &pblk->gc;
struct pblk_line *line;
- struct list_head *group_list;
- bool run_gc;
- int inflight_gc, gc_group = 0, prev_group = 0;
do {
spin_lock(&l_mg->gc_lock);
if (list_empty(&l_mg->gc_full_list)) {
spin_unlock(&l_mg->gc_lock);
- break;
+ return;
}
line = list_first_entry(&l_mg->gc_full_list,
@@ -365,6 +355,24 @@ static void pblk_gc_run(struct pblk *pblk)
kref_put(&line->ref, pblk_line_put);
} while (1);
+}
+
+/*
+ * Lines with no valid sectors will be returned to the free list immediately. If
+ * GC is activated - either because the free block count is under the determined
+ * threshold, or because it is being forced from user space - only lines with a
+ * high count of invalid sectors will be recycled.
+ */
+static void pblk_gc_run(struct pblk *pblk)
+{
+ struct pblk_line_mgmt *l_mg = &pblk->l_mg;
+ struct pblk_gc *gc = &pblk->gc;
+ struct pblk_line *line;
+ struct list_head *group_list;
+ bool run_gc;
+ int inflight_gc, gc_group = 0, prev_group = 0;
+
+ pblk_gc_free_full_lines(pblk);
run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
if (!run_gc || (atomic_read(&gc->inflight_gc) >= PBLK_GC_L_QD))
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index ec7974b..1cc3de4 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -508,6 +508,9 @@ static int pblk_lines_configure(struct pblk *pblk, int flags)
}
}
+ /* Free full lines directly as GC has not been started yet */
+ pblk_gc_free_full_lines(pblk);
+
if (!line) {
/* Configure next line for user data */
line = pblk_line_get_first_data(pblk);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index eaf5397..f76583b 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -831,6 +831,7 @@ void pblk_gc_should_start(struct pblk *pblk);
void pblk_gc_should_stop(struct pblk *pblk);
void pblk_gc_should_kick(struct pblk *pblk);
void pblk_gc_kick(struct pblk *pblk);
+void pblk_gc_free_full_lines(struct pblk *pblk);
void pblk_gc_sysfs_state_show(struct pblk *pblk, int *gc_enabled,
int *gc_active);
int pblk_gc_sysfs_force(struct pblk *pblk, int force);
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/9] lightnvm: pblk: free full lines during recovery
2017-10-03 10:05 ` [PATCH 3/9] lightnvm: pblk: free full lines during recovery Hans Holmberg
@ 2017-10-03 10:36 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:36 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 4050 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> When rebuilding the L2P table, any full lines(lines without any
> valid sectors) will be identified. If these lines are not freed,
> we risk not being able to allocate the first data line.
>
> This patch refactors the part of GC that frees empty lines
> into a separate function and adds a call to this after the
> L2P table has been rebuilt.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-gc.c | 32 ++++++++++++++++++++------------
> drivers/lightnvm/pblk-init.c | 3 +++
> drivers/lightnvm/pblk.h | 1 +
> 3 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 81efac1..374089f 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -330,26 +330,16 @@ static bool pblk_gc_should_run(struct pblk_gc *gc, struct pblk_rl *rl)
> return ((gc->gc_active) && (nr_blocks_need > nr_blocks_free));
> }
>
> -/*
> - * Lines with no valid sectors will be returned to the free list immediately. If
> - * GC is activated - either because the free block count is under the determined
> - * threshold, or because it is being forced from user space - only lines with a
> - * high count of invalid sectors will be recycled.
> - */
> -static void pblk_gc_run(struct pblk *pblk)
> +void pblk_gc_free_full_lines(struct pblk *pblk)
> {
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> - struct pblk_gc *gc = &pblk->gc;
> struct pblk_line *line;
> - struct list_head *group_list;
> - bool run_gc;
> - int inflight_gc, gc_group = 0, prev_group = 0;
>
> do {
> spin_lock(&l_mg->gc_lock);
> if (list_empty(&l_mg->gc_full_list)) {
> spin_unlock(&l_mg->gc_lock);
> - break;
> + return;
> }
>
> line = list_first_entry(&l_mg->gc_full_list,
> @@ -365,6 +355,24 @@ static void pblk_gc_run(struct pblk *pblk)
>
> kref_put(&line->ref, pblk_line_put);
> } while (1);
> +}
> +
> +/*
> + * Lines with no valid sectors will be returned to the free list immediately. If
> + * GC is activated - either because the free block count is under the determined
> + * threshold, or because it is being forced from user space - only lines with a
> + * high count of invalid sectors will be recycled.
> + */
> +static void pblk_gc_run(struct pblk *pblk)
> +{
> + struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> + struct pblk_gc *gc = &pblk->gc;
> + struct pblk_line *line;
> + struct list_head *group_list;
> + bool run_gc;
> + int inflight_gc, gc_group = 0, prev_group = 0;
> +
> + pblk_gc_free_full_lines(pblk);
>
> run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
> if (!run_gc || (atomic_read(&gc->inflight_gc) >= PBLK_GC_L_QD))
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index ec7974b..1cc3de4 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -508,6 +508,9 @@ static int pblk_lines_configure(struct pblk *pblk, int flags)
> }
> }
>
> + /* Free full lines directly as GC has not been started yet */
> + pblk_gc_free_full_lines(pblk);
> +
> if (!line) {
> /* Configure next line for user data */
> line = pblk_line_get_first_data(pblk);
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index eaf5397..f76583b 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -831,6 +831,7 @@ void pblk_gc_should_start(struct pblk *pblk);
> void pblk_gc_should_stop(struct pblk *pblk);
> void pblk_gc_should_kick(struct pblk *pblk);
> void pblk_gc_kick(struct pblk *pblk);
> +void pblk_gc_free_full_lines(struct pblk *pblk);
> void pblk_gc_sysfs_state_show(struct pblk *pblk, int *gc_enabled,
> int *gc_active);
> int pblk_gc_sysfs_force(struct pblk *pblk, int force);
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/9] lightnvm: pblk: start gc if needed during init
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (2 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 3/9] lightnvm: pblk: free full lines during recovery Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:36 ` Javier González
2017-10-03 10:05 ` [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery Hans Holmberg
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
Start GC, if needed, directly after init, as we might
need to garbage collect to make room for user writes.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-init.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 1cc3de4..fb41182 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -1022,6 +1022,10 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
pblk->rwb.nr_entries);
wake_up_process(pblk->writer_ts);
+
+ /* Check if we need to start GC */
+ pblk_gc_should_kick(pblk);
+
return pblk;
fail_stop_writer:
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/9] lightnvm: pblk: start gc if needed during init
2017-10-03 10:05 ` [PATCH 4/9] lightnvm: pblk: start gc if needed during init Hans Holmberg
@ 2017-10-03 10:36 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:36 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 964 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> Start GC, if needed, directly after init, as we might
> need to garbage collect to make room for user writes.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-init.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 1cc3de4..fb41182 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -1022,6 +1022,10 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
> pblk->rwb.nr_entries);
>
> wake_up_process(pblk->writer_ts);
> +
> + /* Check if we need to start GC */
> + pblk_gc_should_kick(pblk);
> +
> return pblk;
>
> fail_stop_writer:
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (3 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 4/9] lightnvm: pblk: start gc if needed during init Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:38 ` Javier González
2017-10-03 10:05 ` [PATCH 6/9] lightnvm: pblk: shut down gc gracefully during exit Hans Holmberg
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
When recovering lines we need to consider that bad blocks in a line
affects the emeta area size.
Previously it was assumed that the emeta area would grow by the
number of sectors per page * number of bad blocks in the line.
This assumtion not correct - the number of "extra" pages that are
consumed could be both smaller (depending on emeta size) and bigger
(depending on the placement of the bad blocks).
Fix this by calculating the emeta start by iterating backwards
through the line, skipping ppas that map to bad blocks.
Also fix the data types used for ppa indices/counts in
pblk_recov_l2p_from_emeta - we should use u64.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-recovery.c | 44 +++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 74b3b86..b5a2275 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -133,16 +133,16 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
struct pblk_emeta *emeta = line->emeta;
struct line_emeta *emeta_buf = emeta->buf;
__le64 *lba_list;
- int data_start, data_end;
- int nr_valid_lbas, nr_lbas = 0;
- int i;
+ u64 data_start, data_end;
+ u64 nr_valid_lbas, nr_lbas = 0;
+ u64 i;
lba_list = pblk_recov_get_lba_list(pblk, emeta_buf);
if (!lba_list)
return 1;
data_start = pblk_line_smeta_start(pblk, line) + lm->smeta_sec;
- data_end = lm->sec_per_line - lm->emeta_sec[0];
+ data_end = line->emeta_ssec;
nr_valid_lbas = le64_to_cpu(emeta_buf->nr_valid_lbas);
for (i = data_start; i < data_end; i++) {
@@ -172,8 +172,8 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
}
if (nr_valid_lbas != nr_lbas)
- pr_err("pblk: line %d - inconsistent lba list(%llu/%d)\n",
- line->id, emeta_buf->nr_valid_lbas, nr_lbas);
+ pr_err("pblk: line %d - inconsistent lba list(%llu/%llu)\n",
+ line->id, nr_valid_lbas, nr_lbas);
line->left_msecs = 0;
@@ -827,11 +827,33 @@ static void pblk_recov_line_add_ordered(struct list_head *head,
__list_add(&line->list, t->list.prev, &t->list);
}
-struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
+static u64 pblk_line_emeta_start(struct pblk *pblk, struct pblk_line *line)
{
struct nvm_tgt_dev *dev = pblk->dev;
struct nvm_geo *geo = &dev->geo;
struct pblk_line_meta *lm = &pblk->lm;
+ unsigned int emeta_secs;
+ u64 emeta_start;
+ struct ppa_addr ppa;
+ int pos;
+
+ emeta_secs = lm->emeta_sec[0];
+ emeta_start = lm->sec_per_line;
+
+ while (emeta_secs) {
+ emeta_start--;
+ ppa = addr_to_pblk_ppa(pblk, emeta_start, line->id);
+ pos = pblk_ppa_to_pos(geo, ppa);
+ if (!test_bit(pos, line->blk_bitmap))
+ emeta_secs--;
+ }
+
+ return emeta_start;
+}
+
+struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
+{
+ struct pblk_line_meta *lm = &pblk->lm;
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
struct pblk_line *line, *tline, *data_line = NULL;
struct pblk_smeta *smeta;
@@ -930,15 +952,9 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
/* Verify closed blocks and recover this portion of L2P table*/
list_for_each_entry_safe(line, tline, &recov_list, list) {
- int off, nr_bb;
-
recovered_lines++;
- /* Calculate where emeta starts based on the line bb */
- off = lm->sec_per_line - lm->emeta_sec[0];
- nr_bb = bitmap_weight(line->blk_bitmap, lm->blk_per_line);
- off -= nr_bb * geo->sec_per_pl;
- line->emeta_ssec = off;
+ line->emeta_ssec = pblk_line_emeta_start(pblk, line);
line->emeta = emeta;
memset(line->emeta->buf, 0, lm->emeta_len[0]);
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery
2017-10-03 10:05 ` [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery Hans Holmberg
@ 2017-10-03 10:38 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:38 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 4288 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> When recovering lines we need to consider that bad blocks in a line
> affects the emeta area size.
>
> Previously it was assumed that the emeta area would grow by the
> number of sectors per page * number of bad blocks in the line.
>
> This assumtion not correct - the number of "extra" pages that are
> consumed could be both smaller (depending on emeta size) and bigger
> (depending on the placement of the bad blocks).
This assumption is not correct >> missing _is_
>
> Fix this by calculating the emeta start by iterating backwards
> through the line, skipping ppas that map to bad blocks.
>
> Also fix the data types used for ppa indices/counts in
> pblk_recov_l2p_from_emeta - we should use u64.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-recovery.c | 44 +++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 74b3b86..b5a2275 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -133,16 +133,16 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> struct pblk_emeta *emeta = line->emeta;
> struct line_emeta *emeta_buf = emeta->buf;
> __le64 *lba_list;
> - int data_start, data_end;
> - int nr_valid_lbas, nr_lbas = 0;
> - int i;
> + u64 data_start, data_end;
> + u64 nr_valid_lbas, nr_lbas = 0;
> + u64 i;
>
> lba_list = pblk_recov_get_lba_list(pblk, emeta_buf);
> if (!lba_list)
> return 1;
>
> data_start = pblk_line_smeta_start(pblk, line) + lm->smeta_sec;
> - data_end = lm->sec_per_line - lm->emeta_sec[0];
> + data_end = line->emeta_ssec;
> nr_valid_lbas = le64_to_cpu(emeta_buf->nr_valid_lbas);
>
> for (i = data_start; i < data_end; i++) {
> @@ -172,8 +172,8 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> }
>
> if (nr_valid_lbas != nr_lbas)
> - pr_err("pblk: line %d - inconsistent lba list(%llu/%d)\n",
> - line->id, emeta_buf->nr_valid_lbas, nr_lbas);
> + pr_err("pblk: line %d - inconsistent lba list(%llu/%llu)\n",
> + line->id, nr_valid_lbas, nr_lbas);
>
> line->left_msecs = 0;
>
> @@ -827,11 +827,33 @@ static void pblk_recov_line_add_ordered(struct list_head *head,
> __list_add(&line->list, t->list.prev, &t->list);
> }
>
> -struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
> +static u64 pblk_line_emeta_start(struct pblk *pblk, struct pblk_line *line)
> {
> struct nvm_tgt_dev *dev = pblk->dev;
> struct nvm_geo *geo = &dev->geo;
> struct pblk_line_meta *lm = &pblk->lm;
> + unsigned int emeta_secs;
> + u64 emeta_start;
> + struct ppa_addr ppa;
> + int pos;
> +
> + emeta_secs = lm->emeta_sec[0];
> + emeta_start = lm->sec_per_line;
> +
> + while (emeta_secs) {
> + emeta_start--;
> + ppa = addr_to_pblk_ppa(pblk, emeta_start, line->id);
> + pos = pblk_ppa_to_pos(geo, ppa);
> + if (!test_bit(pos, line->blk_bitmap))
> + emeta_secs--;
> + }
> +
> + return emeta_start;
> +}
> +
> +struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
> +{
> + struct pblk_line_meta *lm = &pblk->lm;
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct pblk_line *line, *tline, *data_line = NULL;
> struct pblk_smeta *smeta;
> @@ -930,15 +952,9 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
>
> /* Verify closed blocks and recover this portion of L2P table*/
> list_for_each_entry_safe(line, tline, &recov_list, list) {
> - int off, nr_bb;
> -
> recovered_lines++;
> - /* Calculate where emeta starts based on the line bb */
> - off = lm->sec_per_line - lm->emeta_sec[0];
> - nr_bb = bitmap_weight(line->blk_bitmap, lm->blk_per_line);
> - off -= nr_bb * geo->sec_per_pl;
>
> - line->emeta_ssec = off;
> + line->emeta_ssec = pblk_line_emeta_start(pblk, line);
> line->emeta = emeta;
> memset(line->emeta->buf, 0, lm->emeta_len[0]);
>
> --
> 2.7.4
Apart from the commit message, it looks good.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/9] lightnvm: pblk: shut down gc gracefully during exit
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (4 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:38 ` Javier González
2017-10-03 10:05 ` [PATCH 7/9] lightnvm: pblk: add l2p crc debug printouts Hans Holmberg
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
Shut down the GC workqueues and tasks in the right order.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-gc.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 374089f..f343f90 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -422,10 +422,15 @@ void pblk_gc_kick(struct pblk *pblk)
{
struct pblk_gc *gc = &pblk->gc;
- wake_up_process(gc->gc_ts);
pblk_gc_writer_kick(gc);
pblk_gc_reader_kick(gc);
- mod_timer(&gc->gc_timer, jiffies + msecs_to_jiffies(GC_TIME_MSECS));
+
+ /* If we're shutting down GC, let's not start it up again */
+ if (gc->gc_enabled) {
+ wake_up_process(gc->gc_ts);
+ mod_timer(&gc->gc_timer,
+ jiffies + msecs_to_jiffies(GC_TIME_MSECS));
+ }
}
static void pblk_gc_timer(unsigned long data)
@@ -625,9 +630,6 @@ void pblk_gc_exit(struct pblk *pblk)
{
struct pblk_gc *gc = &pblk->gc;
- flush_workqueue(gc->gc_reader_wq);
- flush_workqueue(gc->gc_line_reader_wq);
-
gc->gc_enabled = 0;
del_timer_sync(&gc->gc_timer);
pblk_gc_stop(pblk, 1);
@@ -635,15 +637,17 @@ void pblk_gc_exit(struct pblk *pblk)
if (gc->gc_ts)
kthread_stop(gc->gc_ts);
+ if (gc->gc_reader_ts)
+ kthread_stop(gc->gc_reader_ts);
+
+ flush_workqueue(gc->gc_reader_wq);
if (gc->gc_reader_wq)
destroy_workqueue(gc->gc_reader_wq);
+ flush_workqueue(gc->gc_line_reader_wq);
if (gc->gc_line_reader_wq)
destroy_workqueue(gc->gc_line_reader_wq);
if (gc->gc_writer_ts)
kthread_stop(gc->gc_writer_ts);
-
- if (gc->gc_reader_ts)
- kthread_stop(gc->gc_reader_ts);
}
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 6/9] lightnvm: pblk: shut down gc gracefully during exit
2017-10-03 10:05 ` [PATCH 6/9] lightnvm: pblk: shut down gc gracefully during exit Hans Holmberg
@ 2017-10-03 10:38 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:38 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> Shut down the GC workqueues and tasks in the right order.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-gc.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 374089f..f343f90 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -422,10 +422,15 @@ void pblk_gc_kick(struct pblk *pblk)
> {
> struct pblk_gc *gc = &pblk->gc;
>
> - wake_up_process(gc->gc_ts);
> pblk_gc_writer_kick(gc);
> pblk_gc_reader_kick(gc);
> - mod_timer(&gc->gc_timer, jiffies + msecs_to_jiffies(GC_TIME_MSECS));
> +
> + /* If we're shutting down GC, let's not start it up again */
> + if (gc->gc_enabled) {
> + wake_up_process(gc->gc_ts);
> + mod_timer(&gc->gc_timer,
> + jiffies + msecs_to_jiffies(GC_TIME_MSECS));
> + }
> }
>
> static void pblk_gc_timer(unsigned long data)
> @@ -625,9 +630,6 @@ void pblk_gc_exit(struct pblk *pblk)
> {
> struct pblk_gc *gc = &pblk->gc;
>
> - flush_workqueue(gc->gc_reader_wq);
> - flush_workqueue(gc->gc_line_reader_wq);
> -
> gc->gc_enabled = 0;
> del_timer_sync(&gc->gc_timer);
> pblk_gc_stop(pblk, 1);
> @@ -635,15 +637,17 @@ void pblk_gc_exit(struct pblk *pblk)
> if (gc->gc_ts)
> kthread_stop(gc->gc_ts);
>
> + if (gc->gc_reader_ts)
> + kthread_stop(gc->gc_reader_ts);
> +
> + flush_workqueue(gc->gc_reader_wq);
> if (gc->gc_reader_wq)
> destroy_workqueue(gc->gc_reader_wq);
>
> + flush_workqueue(gc->gc_line_reader_wq);
> if (gc->gc_line_reader_wq)
> destroy_workqueue(gc->gc_line_reader_wq);
>
> if (gc->gc_writer_ts)
> kthread_stop(gc->gc_writer_ts);
> -
> - if (gc->gc_reader_ts)
> - kthread_stop(gc->gc_reader_ts);
> }
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/9] lightnvm: pblk: add l2p crc debug printouts
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (5 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 6/9] lightnvm: pblk: shut down gc gracefully during exit Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:39 ` Javier González
2017-10-03 10:05 ` [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before exit Hans Holmberg
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
Print the CRC of the logical-to-physical mapping during exit and
after recovering the L2P table to facilitate detection of meta
data corruption/recovery issues.
The CRC printed after recovery should match the CRC printed during
the previous exit - if it doesn't this indicates that either the meta
data written to the disk is corrupt or recovery failed.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-init.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index fb41182..c452478 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -76,6 +76,28 @@ static blk_qc_t pblk_make_rq(struct request_queue *q, struct bio *bio)
return BLK_QC_T_NONE;
}
+static size_t pblk_trans_map_size(struct pblk *pblk)
+{
+ int entry_size = 8;
+
+ if (pblk->ppaf_bitsize < 32)
+ entry_size = 4;
+
+ return entry_size * pblk->rl.nr_secs;
+}
+
+#ifdef CONFIG_NVM_DEBUG
+static u32 pblk_l2p_crc(struct pblk *pblk)
+{
+ size_t map_size;
+ u32 crc = ~(u32)0;
+
+ map_size = pblk_trans_map_size(pblk);
+ crc = crc32_le(crc, pblk->trans_map, map_size);
+ return crc;
+}
+#endif
+
static void pblk_l2p_free(struct pblk *pblk)
{
vfree(pblk->trans_map);
@@ -85,12 +107,10 @@ static int pblk_l2p_init(struct pblk *pblk)
{
sector_t i;
struct ppa_addr ppa;
- int entry_size = 8;
-
- if (pblk->ppaf_bitsize < 32)
- entry_size = 4;
+ size_t map_size;
- pblk->trans_map = vmalloc(entry_size * pblk->rl.nr_secs);
+ map_size = pblk_trans_map_size(pblk);
+ pblk->trans_map = vmalloc(map_size);
if (!pblk->trans_map)
return -ENOMEM;
@@ -508,6 +528,10 @@ static int pblk_lines_configure(struct pblk *pblk, int flags)
}
}
+#ifdef CONFIG_NVM_DEBUG
+ pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk));
+#endif
+
/* Free full lines directly as GC has not been started yet */
pblk_gc_free_full_lines(pblk);
@@ -898,6 +922,11 @@ static void pblk_exit(void *private)
down_write(&pblk_lock);
pblk_gc_exit(pblk);
pblk_tear_down(pblk);
+
+#ifdef CONFIG_NVM_DEBUG
+ pr_info("pblk exit: L2P CRC: %x\n", pblk_l2p_crc(pblk));
+#endif
+
pblk_free(pblk);
up_write(&pblk_lock);
}
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 7/9] lightnvm: pblk: add l2p crc debug printouts
2017-10-03 10:05 ` [PATCH 7/9] lightnvm: pblk: add l2p crc debug printouts Hans Holmberg
@ 2017-10-03 10:39 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:39 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> Print the CRC of the logical-to-physical mapping during exit and
> after recovering the L2P table to facilitate detection of meta
> data corruption/recovery issues.
>
> The CRC printed after recovery should match the CRC printed during
> the previous exit - if it doesn't this indicates that either the meta
> data written to the disk is corrupt or recovery failed.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-init.c | 39 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index fb41182..c452478 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -76,6 +76,28 @@ static blk_qc_t pblk_make_rq(struct request_queue *q, struct bio *bio)
> return BLK_QC_T_NONE;
> }
>
> +static size_t pblk_trans_map_size(struct pblk *pblk)
> +{
> + int entry_size = 8;
> +
> + if (pblk->ppaf_bitsize < 32)
> + entry_size = 4;
> +
> + return entry_size * pblk->rl.nr_secs;
> +}
> +
> +#ifdef CONFIG_NVM_DEBUG
> +static u32 pblk_l2p_crc(struct pblk *pblk)
> +{
> + size_t map_size;
> + u32 crc = ~(u32)0;
> +
> + map_size = pblk_trans_map_size(pblk);
> + crc = crc32_le(crc, pblk->trans_map, map_size);
> + return crc;
> +}
> +#endif
> +
> static void pblk_l2p_free(struct pblk *pblk)
> {
> vfree(pblk->trans_map);
> @@ -85,12 +107,10 @@ static int pblk_l2p_init(struct pblk *pblk)
> {
> sector_t i;
> struct ppa_addr ppa;
> - int entry_size = 8;
> -
> - if (pblk->ppaf_bitsize < 32)
> - entry_size = 4;
> + size_t map_size;
>
> - pblk->trans_map = vmalloc(entry_size * pblk->rl.nr_secs);
> + map_size = pblk_trans_map_size(pblk);
> + pblk->trans_map = vmalloc(map_size);
> if (!pblk->trans_map)
> return -ENOMEM;
>
> @@ -508,6 +528,10 @@ static int pblk_lines_configure(struct pblk *pblk, int flags)
> }
> }
>
> +#ifdef CONFIG_NVM_DEBUG
> + pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk));
> +#endif
> +
> /* Free full lines directly as GC has not been started yet */
> pblk_gc_free_full_lines(pblk);
>
> @@ -898,6 +922,11 @@ static void pblk_exit(void *private)
> down_write(&pblk_lock);
> pblk_gc_exit(pblk);
> pblk_tear_down(pblk);
> +
> +#ifdef CONFIG_NVM_DEBUG
> + pr_info("pblk exit: L2P CRC: %x\n", pblk_l2p_crc(pblk));
> +#endif
> +
> pblk_free(pblk);
> up_write(&pblk_lock);
> }
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before exit
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (6 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 7/9] lightnvm: pblk: add l2p crc debug printouts Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:40 ` Javier González
2017-10-03 10:05 ` [PATCH 9/9] lightnvm: pblk: correct valid lba count calculation Hans Holmberg
2017-10-03 10:51 ` [PATCH 0/9] recovery robustness improvements Javier González
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
Finish garbage collect of the lines that are in the gc pipeline
before exiting. Ensure that all lines already in in the pipeline
goes through, from read to write.
Do this by keeping track of how many lines are in the pipeline
and waiting for that number to reach zero before exiting the gc
reader task.
Since we're adding a new gc line counter, change the name of
inflight_gc to read_inflight_gc to make the distinction clear.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-core.c | 3 +++
drivers/lightnvm/pblk-gc.c | 31 ++++++++++++++++++++++++-------
drivers/lightnvm/pblk-sysfs.c | 2 +-
drivers/lightnvm/pblk.h | 5 ++++-
4 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 15e1e28..e62e6eb 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1463,6 +1463,7 @@ void pblk_line_free(struct pblk *pblk, struct pblk_line *line)
static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
{
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
+ struct pblk_gc *gc = &pblk->gc;
spin_lock(&line->lock);
WARN_ON(line->state != PBLK_LINESTATE_GC);
@@ -1471,6 +1472,8 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
pblk_line_free(pblk, line);
spin_unlock(&line->lock);
+ atomic_dec(&gc->pipeline_gc);
+
spin_lock(&l_mg->free_lock);
list_add_tail(&line->list, &l_mg->free_list);
l_mg->nr_free_lines++;
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index f343f90..475a13e 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -234,7 +234,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
kfree(invalid_bitmap);
kref_put(&line->ref, pblk_line_put);
- atomic_dec(&gc->inflight_gc);
+ atomic_dec(&gc->read_inflight_gc);
return;
@@ -249,7 +249,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
pblk_put_line_back(pblk, line);
kref_put(&line->ref, pblk_line_put);
- atomic_dec(&gc->inflight_gc);
+ atomic_dec(&gc->read_inflight_gc);
pr_err("pblk: Failed to GC line %d\n", line->id);
}
@@ -268,6 +268,7 @@ static int pblk_gc_line(struct pblk *pblk, struct pblk_line *line)
line_ws->pblk = pblk;
line_ws->line = line;
+ atomic_inc(&gc->pipeline_gc);
INIT_WORK(&line_ws->ws, pblk_gc_line_prepare_ws);
queue_work(gc->gc_reader_wq, &line_ws->ws);
@@ -333,6 +334,7 @@ static bool pblk_gc_should_run(struct pblk_gc *gc, struct pblk_rl *rl)
void pblk_gc_free_full_lines(struct pblk *pblk)
{
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
+ struct pblk_gc *gc = &pblk->gc;
struct pblk_line *line;
do {
@@ -353,6 +355,7 @@ void pblk_gc_free_full_lines(struct pblk *pblk)
list_del(&line->list);
spin_unlock(&l_mg->gc_lock);
+ atomic_inc(&gc->pipeline_gc);
kref_put(&line->ref, pblk_line_put);
} while (1);
}
@@ -370,12 +373,12 @@ static void pblk_gc_run(struct pblk *pblk)
struct pblk_line *line;
struct list_head *group_list;
bool run_gc;
- int inflight_gc, gc_group = 0, prev_group = 0;
+ int read_inflight_gc, gc_group = 0, prev_group = 0;
pblk_gc_free_full_lines(pblk);
run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
- if (!run_gc || (atomic_read(&gc->inflight_gc) >= PBLK_GC_L_QD))
+ if (!run_gc || (atomic_read(&gc->read_inflight_gc) >= PBLK_GC_L_QD))
return;
next_gc_group:
@@ -402,14 +405,14 @@ static void pblk_gc_run(struct pblk *pblk)
list_add_tail(&line->list, &gc->r_list);
spin_unlock(&gc->r_lock);
- inflight_gc = atomic_inc_return(&gc->inflight_gc);
+ read_inflight_gc = atomic_inc_return(&gc->read_inflight_gc);
pblk_gc_reader_kick(gc);
prev_group = 1;
/* No need to queue up more GC lines than we can handle */
run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
- if (!run_gc || inflight_gc >= PBLK_GC_L_QD)
+ if (!run_gc || read_inflight_gc >= PBLK_GC_L_QD)
break;
} while (1);
@@ -470,6 +473,7 @@ static int pblk_gc_writer_ts(void *data)
static int pblk_gc_reader_ts(void *data)
{
struct pblk *pblk = data;
+ struct pblk_gc *gc = &pblk->gc;
while (!kthread_should_stop()) {
if (!pblk_gc_read(pblk))
@@ -478,6 +482,18 @@ static int pblk_gc_reader_ts(void *data)
io_schedule();
}
+#ifdef CONFIG_NVM_DEBUG
+ pr_info("pblk: flushing gc pipeline, %d lines left\n",
+ atomic_read(&gc->pipeline_gc));
+#endif
+
+ do {
+ if (!atomic_read(&gc->pipeline_gc))
+ break;
+
+ schedule();
+ } while (1);
+
return 0;
}
@@ -581,7 +597,8 @@ int pblk_gc_init(struct pblk *pblk)
gc->gc_forced = 0;
gc->gc_enabled = 1;
gc->w_entries = 0;
- atomic_set(&gc->inflight_gc, 0);
+ atomic_set(&gc->read_inflight_gc, 0);
+ atomic_set(&gc->pipeline_gc, 0);
/* Workqueue that reads valid sectors from a line and submit them to the
* GC writer to be recycled.
diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c
index 95fb434..cd49e88 100644
--- a/drivers/lightnvm/pblk-sysfs.c
+++ b/drivers/lightnvm/pblk-sysfs.c
@@ -253,7 +253,7 @@ static ssize_t pblk_sysfs_lines(struct pblk *pblk, char *page)
sz += snprintf(page + sz, PAGE_SIZE - sz,
"GC: full:%d, high:%d, mid:%d, low:%d, empty:%d, queue:%d\n",
gc_full, gc_high, gc_mid, gc_low, gc_empty,
- atomic_read(&pblk->gc.inflight_gc));
+ atomic_read(&pblk->gc.read_inflight_gc));
sz += snprintf(page + sz, PAGE_SIZE - sz,
"data (%d) cur:%d, left:%d, vsc:%d, s:%d, map:%d/%d (%d)\n",
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index f76583b..03965da 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -238,7 +238,10 @@ struct pblk_gc {
struct timer_list gc_timer;
struct semaphore gc_sem;
- atomic_t inflight_gc;
+ atomic_t read_inflight_gc; /* Number of lines with inflight GC reads */
+ atomic_t pipeline_gc; /* Number of lines in the GC pipeline -
+ * started reads to finished writes
+ */
int w_entries;
struct list_head w_list;
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before exit
2017-10-03 10:05 ` [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before exit Hans Holmberg
@ 2017-10-03 10:40 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:40 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 6679 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> Finish garbage collect of the lines that are in the gc pipeline
> before exiting. Ensure that all lines already in in the pipeline
> goes through, from read to write.
>
> Do this by keeping track of how many lines are in the pipeline
> and waiting for that number to reach zero before exiting the gc
> reader task.
>
> Since we're adding a new gc line counter, change the name of
> inflight_gc to read_inflight_gc to make the distinction clear.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-core.c | 3 +++
> drivers/lightnvm/pblk-gc.c | 31 ++++++++++++++++++++++++-------
> drivers/lightnvm/pblk-sysfs.c | 2 +-
> drivers/lightnvm/pblk.h | 5 ++++-
> 4 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 15e1e28..e62e6eb 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1463,6 +1463,7 @@ void pblk_line_free(struct pblk *pblk, struct pblk_line *line)
> static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> {
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> + struct pblk_gc *gc = &pblk->gc;
>
> spin_lock(&line->lock);
> WARN_ON(line->state != PBLK_LINESTATE_GC);
> @@ -1471,6 +1472,8 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> pblk_line_free(pblk, line);
> spin_unlock(&line->lock);
>
> + atomic_dec(&gc->pipeline_gc);
> +
> spin_lock(&l_mg->free_lock);
> list_add_tail(&line->list, &l_mg->free_list);
> l_mg->nr_free_lines++;
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index f343f90..475a13e 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -234,7 +234,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> kfree(invalid_bitmap);
>
> kref_put(&line->ref, pblk_line_put);
> - atomic_dec(&gc->inflight_gc);
> + atomic_dec(&gc->read_inflight_gc);
>
> return;
>
> @@ -249,7 +249,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
>
> pblk_put_line_back(pblk, line);
> kref_put(&line->ref, pblk_line_put);
> - atomic_dec(&gc->inflight_gc);
> + atomic_dec(&gc->read_inflight_gc);
>
> pr_err("pblk: Failed to GC line %d\n", line->id);
> }
> @@ -268,6 +268,7 @@ static int pblk_gc_line(struct pblk *pblk, struct pblk_line *line)
> line_ws->pblk = pblk;
> line_ws->line = line;
>
> + atomic_inc(&gc->pipeline_gc);
> INIT_WORK(&line_ws->ws, pblk_gc_line_prepare_ws);
> queue_work(gc->gc_reader_wq, &line_ws->ws);
>
> @@ -333,6 +334,7 @@ static bool pblk_gc_should_run(struct pblk_gc *gc, struct pblk_rl *rl)
> void pblk_gc_free_full_lines(struct pblk *pblk)
> {
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> + struct pblk_gc *gc = &pblk->gc;
> struct pblk_line *line;
>
> do {
> @@ -353,6 +355,7 @@ void pblk_gc_free_full_lines(struct pblk *pblk)
> list_del(&line->list);
> spin_unlock(&l_mg->gc_lock);
>
> + atomic_inc(&gc->pipeline_gc);
> kref_put(&line->ref, pblk_line_put);
> } while (1);
> }
> @@ -370,12 +373,12 @@ static void pblk_gc_run(struct pblk *pblk)
> struct pblk_line *line;
> struct list_head *group_list;
> bool run_gc;
> - int inflight_gc, gc_group = 0, prev_group = 0;
> + int read_inflight_gc, gc_group = 0, prev_group = 0;
>
> pblk_gc_free_full_lines(pblk);
>
> run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
> - if (!run_gc || (atomic_read(&gc->inflight_gc) >= PBLK_GC_L_QD))
> + if (!run_gc || (atomic_read(&gc->read_inflight_gc) >= PBLK_GC_L_QD))
> return;
>
> next_gc_group:
> @@ -402,14 +405,14 @@ static void pblk_gc_run(struct pblk *pblk)
> list_add_tail(&line->list, &gc->r_list);
> spin_unlock(&gc->r_lock);
>
> - inflight_gc = atomic_inc_return(&gc->inflight_gc);
> + read_inflight_gc = atomic_inc_return(&gc->read_inflight_gc);
> pblk_gc_reader_kick(gc);
>
> prev_group = 1;
>
> /* No need to queue up more GC lines than we can handle */
> run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
> - if (!run_gc || inflight_gc >= PBLK_GC_L_QD)
> + if (!run_gc || read_inflight_gc >= PBLK_GC_L_QD)
> break;
> } while (1);
>
> @@ -470,6 +473,7 @@ static int pblk_gc_writer_ts(void *data)
> static int pblk_gc_reader_ts(void *data)
> {
> struct pblk *pblk = data;
> + struct pblk_gc *gc = &pblk->gc;
>
> while (!kthread_should_stop()) {
> if (!pblk_gc_read(pblk))
> @@ -478,6 +482,18 @@ static int pblk_gc_reader_ts(void *data)
> io_schedule();
> }
>
> +#ifdef CONFIG_NVM_DEBUG
> + pr_info("pblk: flushing gc pipeline, %d lines left\n",
> + atomic_read(&gc->pipeline_gc));
> +#endif
> +
> + do {
> + if (!atomic_read(&gc->pipeline_gc))
> + break;
> +
> + schedule();
> + } while (1);
> +
> return 0;
> }
>
> @@ -581,7 +597,8 @@ int pblk_gc_init(struct pblk *pblk)
> gc->gc_forced = 0;
> gc->gc_enabled = 1;
> gc->w_entries = 0;
> - atomic_set(&gc->inflight_gc, 0);
> + atomic_set(&gc->read_inflight_gc, 0);
> + atomic_set(&gc->pipeline_gc, 0);
>
> /* Workqueue that reads valid sectors from a line and submit them to the
> * GC writer to be recycled.
> diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c
> index 95fb434..cd49e88 100644
> --- a/drivers/lightnvm/pblk-sysfs.c
> +++ b/drivers/lightnvm/pblk-sysfs.c
> @@ -253,7 +253,7 @@ static ssize_t pblk_sysfs_lines(struct pblk *pblk, char *page)
> sz += snprintf(page + sz, PAGE_SIZE - sz,
> "GC: full:%d, high:%d, mid:%d, low:%d, empty:%d, queue:%d\n",
> gc_full, gc_high, gc_mid, gc_low, gc_empty,
> - atomic_read(&pblk->gc.inflight_gc));
> + atomic_read(&pblk->gc.read_inflight_gc));
>
> sz += snprintf(page + sz, PAGE_SIZE - sz,
> "data (%d) cur:%d, left:%d, vsc:%d, s:%d, map:%d/%d (%d)\n",
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index f76583b..03965da 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -238,7 +238,10 @@ struct pblk_gc {
> struct timer_list gc_timer;
>
> struct semaphore gc_sem;
> - atomic_t inflight_gc;
> + atomic_t read_inflight_gc; /* Number of lines with inflight GC reads */
> + atomic_t pipeline_gc; /* Number of lines in the GC pipeline -
> + * started reads to finished writes
> + */
> int w_entries;
>
> struct list_head w_list;
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 9/9] lightnvm: pblk: correct valid lba count calculation
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (7 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before exit Hans Holmberg
@ 2017-10-03 10:05 ` Hans Holmberg
2017-10-03 10:41 ` Javier González
2017-10-03 10:51 ` [PATCH 0/9] recovery robustness improvements Javier González
9 siblings, 1 reply; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 10:05 UTC (permalink / raw)
To: Matias Bjorling
Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg, Hans Holmberg
From: Hans Holmberg <hans.holmberg@cnexlabs.com>
During garbage collect, lbas being written can end up
being invalidated. Make sure that this is reflected in
the valid lba count.
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
drivers/lightnvm/pblk-map.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
index 3bc4c94..6f3ecde 100644
--- a/drivers/lightnvm/pblk-map.c
+++ b/drivers/lightnvm/pblk-map.c
@@ -45,6 +45,8 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
paddr = pblk_alloc_page(pblk, line, nr_secs);
for (i = 0; i < nr_secs; i++, paddr++) {
+ __le64 addr_empty = cpu_to_le64(ADDR_EMPTY);
+
/* ppa to be sent to the device */
ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id);
@@ -61,10 +63,9 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
w_ctx->ppa = ppa_list[i];
meta_list[i].lba = cpu_to_le64(w_ctx->lba);
lba_list[paddr] = cpu_to_le64(w_ctx->lba);
- line->nr_valid_lbas++;
+ if (lba_list[paddr] != addr_empty)
+ line->nr_valid_lbas++;
} else {
- __le64 addr_empty = cpu_to_le64(ADDR_EMPTY);
-
lba_list[paddr] = meta_list[i].lba = addr_empty;
__pblk_map_invalidate(pblk, line, paddr);
}
--
2.7.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 9/9] lightnvm: pblk: correct valid lba count calculation
2017-10-03 10:05 ` [PATCH 9/9] lightnvm: pblk: correct valid lba count calculation Hans Holmberg
@ 2017-10-03 10:41 ` Javier González
0 siblings, 0 replies; 21+ messages in thread
From: Javier González @ 2017-10-03 10:41 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> During garbage collect, lbas being written can end up
> being invalidated. Make sure that this is reflected in
> the valid lba count.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-map.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
> index 3bc4c94..6f3ecde 100644
> --- a/drivers/lightnvm/pblk-map.c
> +++ b/drivers/lightnvm/pblk-map.c
> @@ -45,6 +45,8 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
> paddr = pblk_alloc_page(pblk, line, nr_secs);
>
> for (i = 0; i < nr_secs; i++, paddr++) {
> + __le64 addr_empty = cpu_to_le64(ADDR_EMPTY);
> +
> /* ppa to be sent to the device */
> ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id);
>
> @@ -61,10 +63,9 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
> w_ctx->ppa = ppa_list[i];
> meta_list[i].lba = cpu_to_le64(w_ctx->lba);
> lba_list[paddr] = cpu_to_le64(w_ctx->lba);
> - line->nr_valid_lbas++;
> + if (lba_list[paddr] != addr_empty)
> + line->nr_valid_lbas++;
> } else {
> - __le64 addr_empty = cpu_to_le64(ADDR_EMPTY);
> -
> lba_list[paddr] = meta_list[i].lba = addr_empty;
> __pblk_map_invalidate(pblk, line, paddr);
> }
> --
> 2.7.4
LGTM.
Reviewed-by: Javier González <javier@cnexlabs.com>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] recovery robustness improvements
2017-10-03 10:05 [PATCH 0/9] recovery robustness improvements Hans Holmberg
` (8 preceding siblings ...)
2017-10-03 10:05 ` [PATCH 9/9] lightnvm: pblk: correct valid lba count calculation Hans Holmberg
@ 2017-10-03 10:51 ` Javier González
2017-10-03 13:53 ` Hans Holmberg
9 siblings, 1 reply; 21+ messages in thread
From: Javier González @ 2017-10-03 10:51 UTC (permalink / raw)
To: Hans Holmberg
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]
> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>
> This patchset improves the robustness of recovery - fixing a bunch of
> issues that occurs when removing and re-creating a pblk instance.
> It also adds a couple of debug-only prints to facilitate detection
> of L2P table corruptions.
>
> The patches apply on top of:
> https://github.com/OpenChannelSSD/linux branch for-4.15/pblk
>
> Hans Holmberg (9):
> lightnvm: pblk: prevent gc kicks when gc is not operational
> lightnvm: pblk: recover partially written lines correctly
> lightnvm: pblk: free full lines during recovery
> lightnvm: pblk: start gc if needed during init
> lightnvm: pblk: consider bad sectors in emeta during recovery
> lightnvm: pblk: shut down gc gracefully during exit
> lightnvm: pblk: add l2p crc debug printouts
> lightnvm: pblk: gc all lines in the pipeline before exit
> lightnvm: pblk: correct valid lba count calculation
>
> drivers/lightnvm/pblk-core.c | 3 ++
> drivers/lightnvm/pblk-gc.c | 88 +++++++++++++++++++++++++++-------------
> drivers/lightnvm/pblk-init.c | 47 ++++++++++++++++++---
> drivers/lightnvm/pblk-map.c | 7 ++--
> drivers/lightnvm/pblk-recovery.c | 46 ++++++++++++++-------
> drivers/lightnvm/pblk-sysfs.c | 2 +-
> drivers/lightnvm/pblk.h | 6 ++-
> 7 files changed, 145 insertions(+), 54 deletions(-)
>
> --
> 2.7.4
Thanks Hans.
Matias: I picked the patches on for-4.15/pblk on top of the rest of the
series and fixed some of the commit messages. Also, note that Rakesh's
patches needed some re-work, so please pick them from this tree instead.
They should apply clean on your for-4.15/core.
Thanks,
Javier
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/9] recovery robustness improvements
2017-10-03 10:51 ` [PATCH 0/9] recovery robustness improvements Javier González
@ 2017-10-03 13:53 ` Hans Holmberg
0 siblings, 0 replies; 21+ messages in thread
From: Hans Holmberg @ 2017-10-03 13:53 UTC (permalink / raw)
To: Javier González
Cc: Matias Bjørling, linux-block, linux-kernel, Hans Holmberg
Cheers!
Thanks,
Hans
On Tue, Oct 3, 2017 at 12:51 PM, Javier González <jg@lightnvm.io> wrote:
>> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>>
>> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
>>
>> This patchset improves the robustness of recovery - fixing a bunch of
>> issues that occurs when removing and re-creating a pblk instance.
>> It also adds a couple of debug-only prints to facilitate detection
>> of L2P table corruptions.
>>
>> The patches apply on top of:
>> https://github.com/OpenChannelSSD/linux branch for-4.15/pblk
>>
>> Hans Holmberg (9):
>> lightnvm: pblk: prevent gc kicks when gc is not operational
>> lightnvm: pblk: recover partially written lines correctly
>> lightnvm: pblk: free full lines during recovery
>> lightnvm: pblk: start gc if needed during init
>> lightnvm: pblk: consider bad sectors in emeta during recovery
>> lightnvm: pblk: shut down gc gracefully during exit
>> lightnvm: pblk: add l2p crc debug printouts
>> lightnvm: pblk: gc all lines in the pipeline before exit
>> lightnvm: pblk: correct valid lba count calculation
>>
>> drivers/lightnvm/pblk-core.c | 3 ++
>> drivers/lightnvm/pblk-gc.c | 88 +++++++++++++++++++++++++++-------------
>> drivers/lightnvm/pblk-init.c | 47 ++++++++++++++++++---
>> drivers/lightnvm/pblk-map.c | 7 ++--
>> drivers/lightnvm/pblk-recovery.c | 46 ++++++++++++++-------
>> drivers/lightnvm/pblk-sysfs.c | 2 +-
>> drivers/lightnvm/pblk.h | 6 ++-
>> 7 files changed, 145 insertions(+), 54 deletions(-)
>>
>> --
>> 2.7.4
>
> Thanks Hans.
>
> Matias: I picked the patches on for-4.15/pblk on top of the rest of the
> series and fixed some of the commit messages. Also, note that Rakesh's
> patches needed some re-work, so please pick them from this tree instead.
>
> They should apply clean on your for-4.15/core.
>
> Thanks,
> Javier
^ permalink raw reply [flat|nested] 21+ messages in thread