A bunch of small fixes and extra checks for pblk. Non is critical, though
("lightnvm: pblk: check for chunk size before allocating it") might be nice to
get into 4.17 as it is a fix for the 2.0 pblk patches.
Javier
Javier González (11):
lightnvm: pblk: fail gracefully on line alloc. failure
lightnvm: pblk: recheck for bad lines at runtime
lightnvm: pblk: check read lba on gc path
lightnvn: pblk: improve error msg on corrupted LBAs
lightnvm: pblk: warn in case of corrupted write buffer
lightnvm: pblk: return NVM_ error on failed submission
lightnvm: pblk: remove unnecessary indirection
lightnvm: pblk: remove unnecessary argument
lightnvm: pblk: check for chunk size before allocating it
lightnvn: pass flag on graceful teardown to targets
lightnvm: pblk: remove dead function
drivers/lightnvm/core.c | 10 +++---
drivers/lightnvm/pblk-core.c | 86 ++++++++++++++++++++++++++------------------
drivers/lightnvm/pblk-gc.c | 10 +++---
drivers/lightnvm/pblk-init.c | 38 ++++++++++++--------
drivers/lightnvm/pblk-map.c | 33 ++++++++++++-----
drivers/lightnvm/pblk-rb.c | 5 ++-
drivers/lightnvm/pblk-read.c | 73 ++++++++++++++++++++++++++++---------
drivers/lightnvm/pblk.h | 7 ++--
include/linux/lightnvm.h | 2 +-
9 files changed, 173 insertions(+), 91 deletions(-)
--
2.7.4
Remove unnecessary indirection on the read path.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-read.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 89aed634333a..2f8224354c62 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -102,16 +102,6 @@ static void pblk_read_ppalist_rq(struct pblk *pblk, struct nvm_rq *rqd,
#endif
}
-static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
-{
- int err;
-
- err = pblk_submit_io(pblk, rqd);
- if (err)
- return NVM_IO_ERR;
-
- return NVM_IO_OK;
-}
static void pblk_read_check_seq(struct pblk *pblk, struct nvm_rq *rqd,
sector_t blba)
@@ -485,7 +475,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
rqd->bio = int_bio;
r_ctx->private = bio;
- ret = pblk_submit_read_io(pblk, rqd);
+ ret = pblk_submit_io(pblk, rqd);
if (ret) {
pr_err("pblk: read IO submission failed\n");
if (int_bio)
--
2.7.4
If the namespace is unregistered before the LightNVM target is removed
(e.g., on hot unplug) it is too late for the target to store any metadata
on the device - any attempt to write to the device will fail. In this
case, pass on a "gracefull teardown" flag to the target to let it know
when this happens.
In the case of pblk, we pad the open line (close all open chunks) to
improve data retention. In the event of an ungraceful shutdown, avoid
this part and just clean up.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/core.c | 10 +++++-----
drivers/lightnvm/pblk-core.c | 13 ++++++++++++-
drivers/lightnvm/pblk-gc.c | 10 ++++++----
drivers/lightnvm/pblk-init.c | 14 ++++++++------
drivers/lightnvm/pblk.h | 4 +++-
include/linux/lightnvm.h | 2 +-
6 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 63171cdce270..60aa7bc5a630 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -431,7 +431,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
return 0;
err_sysfs:
if (tt->exit)
- tt->exit(targetdata);
+ tt->exit(targetdata, true);
err_init:
blk_cleanup_queue(tqueue);
tdisk->queue = NULL;
@@ -446,7 +446,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
return ret;
}
-static void __nvm_remove_target(struct nvm_target *t)
+static void __nvm_remove_target(struct nvm_target *t, bool graceful)
{
struct nvm_tgt_type *tt = t->type;
struct gendisk *tdisk = t->disk;
@@ -459,7 +459,7 @@ static void __nvm_remove_target(struct nvm_target *t)
tt->sysfs_exit(tdisk);
if (tt->exit)
- tt->exit(tdisk->private_data);
+ tt->exit(tdisk->private_data, graceful);
nvm_remove_tgt_dev(t->dev, 1);
put_disk(tdisk);
@@ -489,7 +489,7 @@ static int nvm_remove_tgt(struct nvm_dev *dev, struct nvm_ioctl_remove *remove)
mutex_unlock(&dev->mlock);
return 1;
}
- __nvm_remove_target(t);
+ __nvm_remove_target(t, true);
mutex_unlock(&dev->mlock);
return 0;
@@ -963,7 +963,7 @@ void nvm_unregister(struct nvm_dev *dev)
list_for_each_entry_safe(t, tmp, &dev->targets, list) {
if (t->dev->parent != dev)
continue;
- __nvm_remove_target(t);
+ __nvm_remove_target(t, false);
}
mutex_unlock(&dev->mlock);
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 1b871441d816..cc34d5d9652d 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1461,7 +1461,7 @@ static void pblk_line_close_meta_sync(struct pblk *pblk)
flush_workqueue(pblk->close_wq);
}
-void pblk_pipeline_stop(struct pblk *pblk)
+void __pblk_pipeline_flush(struct pblk *pblk)
{
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
int ret;
@@ -1486,6 +1486,11 @@ void pblk_pipeline_stop(struct pblk *pblk)
flush_workqueue(pblk->bb_wq);
pblk_line_close_meta_sync(pblk);
+}
+
+void __pblk_pipeline_stop(struct pblk *pblk)
+{
+ struct pblk_line_mgmt *l_mg = &pblk->l_mg;
spin_lock(&l_mg->free_lock);
pblk->state = PBLK_STATE_STOPPED;
@@ -1494,6 +1499,12 @@ void pblk_pipeline_stop(struct pblk *pblk)
spin_unlock(&l_mg->free_lock);
}
+void pblk_pipeline_stop(struct pblk *pblk)
+{
+ __pblk_pipeline_flush(pblk);
+ __pblk_pipeline_stop(pblk);
+}
+
struct pblk_line *pblk_line_replace_data(struct pblk *pblk)
{
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 6851a5c67189..b0cc277bf972 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -649,7 +649,7 @@ int pblk_gc_init(struct pblk *pblk)
return ret;
}
-void pblk_gc_exit(struct pblk *pblk)
+void pblk_gc_exit(struct pblk *pblk, bool graceful)
{
struct pblk_gc *gc = &pblk->gc;
@@ -663,10 +663,12 @@ void pblk_gc_exit(struct pblk *pblk)
if (gc->gc_reader_ts)
kthread_stop(gc->gc_reader_ts);
- flush_workqueue(gc->gc_reader_wq);
+ if (graceful) {
+ flush_workqueue(gc->gc_reader_wq);
+ flush_workqueue(gc->gc_line_reader_wq);
+ }
+
destroy_workqueue(gc->gc_reader_wq);
-
- flush_workqueue(gc->gc_line_reader_wq);
destroy_workqueue(gc->gc_line_reader_wq);
if (gc->gc_writer_ts)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 9e3a43346d4c..bfc488d0dda9 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -1118,23 +1118,25 @@ static void pblk_free(struct pblk *pblk)
kfree(pblk);
}
-static void pblk_tear_down(struct pblk *pblk)
+static void pblk_tear_down(struct pblk *pblk, bool graceful)
{
- pblk_pipeline_stop(pblk);
+ if (graceful)
+ __pblk_pipeline_flush(pblk);
+ __pblk_pipeline_stop(pblk);
pblk_writer_stop(pblk);
pblk_rb_sync_l2p(&pblk->rwb);
pblk_rl_free(&pblk->rl);
- pr_debug("pblk: consistent tear down\n");
+ pr_debug("pblk: consistent tear down (graceful:%d)\n", graceful);
}
-static void pblk_exit(void *private)
+static void pblk_exit(void *private, bool graceful)
{
struct pblk *pblk = private;
down_write(&pblk_lock);
- pblk_gc_exit(pblk);
- pblk_tear_down(pblk);
+ pblk_gc_exit(pblk, graceful);
+ pblk_tear_down(pblk, graceful);
#ifdef CONFIG_NVM_DEBUG
pr_info("pblk exit: L2P CRC: %x\n", pblk_l2p_crc(pblk));
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index ff0e5e6421dd..4b725bedc36a 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -771,6 +771,8 @@ void pblk_line_close_meta(struct pblk *pblk, struct pblk_line *line);
void pblk_line_close(struct pblk *pblk, struct pblk_line *line);
void pblk_line_close_ws(struct work_struct *work);
void pblk_pipeline_stop(struct pblk *pblk);
+void __pblk_pipeline_stop(struct pblk *pblk);
+void __pblk_pipeline_flush(struct pblk *pblk);
void pblk_gen_run_ws(struct pblk *pblk, struct pblk_line *line, void *priv,
void (*work)(struct work_struct *), gfp_t gfp_mask,
struct workqueue_struct *wq);
@@ -865,7 +867,7 @@ int pblk_recov_setup_rq(struct pblk *pblk, struct pblk_c_ctx *c_ctx,
#define PBLK_GC_RSV_LINE 1 /* Reserved lines for GC */
int pblk_gc_init(struct pblk *pblk);
-void pblk_gc_exit(struct pblk *pblk);
+void pblk_gc_exit(struct pblk *pblk, bool graceful);
void pblk_gc_should_start(struct pblk *pblk);
void pblk_gc_should_stop(struct pblk *pblk);
void pblk_gc_should_kick(struct pblk *pblk);
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 6e0859b9d4d2..e9e0d1c7eaf5 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -489,7 +489,7 @@ typedef blk_qc_t (nvm_tgt_make_rq_fn)(struct request_queue *, struct bio *);
typedef sector_t (nvm_tgt_capacity_fn)(void *);
typedef void *(nvm_tgt_init_fn)(struct nvm_tgt_dev *, struct gendisk *,
int flags);
-typedef void (nvm_tgt_exit_fn)(void *);
+typedef void (nvm_tgt_exit_fn)(void *, bool);
typedef int (nvm_tgt_sysfs_init_fn)(struct gendisk *);
typedef void (nvm_tgt_sysfs_exit_fn)(struct gendisk *);
--
2.7.4
Remove dead function for manual sync. I/O
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-core.c | 7 -------
drivers/lightnvm/pblk.h | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index cc34d5d9652d..5f960a6609c8 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -342,13 +342,6 @@ void pblk_write_should_kick(struct pblk *pblk)
pblk_write_kick(pblk);
}
-void pblk_end_io_sync(struct nvm_rq *rqd)
-{
- struct completion *waiting = rqd->private;
-
- complete(waiting);
-}
-
static void pblk_wait_for_meta(struct pblk *pblk)
{
do {
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 4b725bedc36a..9838d03e1f21 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -796,7 +796,6 @@ void pblk_down_rq(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas,
void pblk_down_page(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas);
void pblk_up_rq(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas,
unsigned long *lun_bitmap);
-void pblk_end_io_sync(struct nvm_rq *rqd);
int pblk_bio_add_pages(struct pblk *pblk, struct bio *bio, gfp_t flags,
int nr_pages);
void pblk_bio_free_pages(struct pblk *pblk, struct bio *bio, int off,
--
2.7.4
Return a meaningful error when the sanity vector I/O check fails.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-core.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 128101f9e606..6bc0c7f61aac 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -467,16 +467,13 @@ int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd)
{
struct nvm_tgt_dev *dev = pblk->dev;
+ atomic_inc(&pblk->inflight_io);
+
#ifdef CONFIG_NVM_DEBUG
- int ret;
-
- ret = pblk_check_io(pblk, rqd);
- if (ret)
- return ret;
+ if (pblk_check_io(pblk, rqd))
+ return NVM_IO_ERR;
#endif
- atomic_inc(&pblk->inflight_io);
-
return nvm_submit_io(dev, rqd);
}
@@ -484,16 +481,13 @@ int pblk_submit_io_sync(struct pblk *pblk, struct nvm_rq *rqd)
{
struct nvm_tgt_dev *dev = pblk->dev;
+ atomic_inc(&pblk->inflight_io);
+
#ifdef CONFIG_NVM_DEBUG
- int ret;
-
- ret = pblk_check_io(pblk, rqd);
- if (ret)
- return ret;
+ if (pblk_check_io(pblk, rqd))
+ return NVM_IO_ERR;
#endif
- atomic_inc(&pblk->inflight_io);
-
return nvm_submit_io_sync(dev, rqd);
}
--
2.7.4
Bad blocks can grow at runtime. Check that the number of valid blocks in
a line are within the sanity threshold before allocating the line for
new writes.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-core.c | 38 ++++++++++++++++++++++++++++----------
drivers/lightnvm/pblk-init.c | 11 +++++++----
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index ceacd10a043e..128101f9e606 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1174,7 +1174,8 @@ static int pblk_prepare_new_line(struct pblk *pblk, struct pblk_line *line)
static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
{
struct pblk_line_meta *lm = &pblk->lm;
- int blk_to_erase;
+ int blk_in_line = atomic_read(&line->blk_in_line);
+ int blk_to_erase, ret;
line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_ATOMIC);
if (!line->map_bitmap)
@@ -1183,8 +1184,8 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
/* will be initialized using bb info from map_bitmap */
line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_ATOMIC);
if (!line->invalid_bitmap) {
- kfree(line->map_bitmap);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto fail_free_map_bitmap;
}
/* Bad blocks do not need to be erased */
@@ -1199,16 +1200,19 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
blk_to_erase = pblk_prepare_new_line(pblk, line);
line->state = PBLK_LINESTATE_FREE;
} else {
- blk_to_erase = atomic_read(&line->blk_in_line);
+ blk_to_erase = blk_in_line;
+ }
+
+ if (blk_in_line < lm->min_blk_line) {
+ ret = -EAGAIN;
+ goto fail_free_invalid_bitmap;
}
if (line->state != PBLK_LINESTATE_FREE) {
- kfree(line->map_bitmap);
- kfree(line->invalid_bitmap);
- spin_unlock(&line->lock);
WARN(1, "pblk: corrupted line %d, state %d\n",
line->id, line->state);
- return -EAGAIN;
+ ret = -EINTR;
+ goto fail_free_invalid_bitmap;
}
line->state = PBLK_LINESTATE_OPEN;
@@ -1222,6 +1226,16 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
kref_init(&line->ref);
return 0;
+
+fail_free_invalid_bitmap:
+ spin_unlock(&line->lock);
+ kfree(line->invalid_bitmap);
+ line->invalid_bitmap = NULL;
+fail_free_map_bitmap:
+ kfree(line->map_bitmap);
+ line->map_bitmap = NULL;
+
+ return ret;
}
int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line)
@@ -1292,10 +1306,14 @@ struct pblk_line *pblk_line_get(struct pblk *pblk)
ret = pblk_line_prepare(pblk, line);
if (ret) {
- if (ret == -EAGAIN) {
+ switch (ret) {
+ case -EAGAIN:
+ list_add(&line->list, &l_mg->bad_list);
+ goto retry;
+ case -EINTR:
list_add(&line->list, &l_mg->corrupt_list);
goto retry;
- } else {
+ default:
pr_err("pblk: failed to prepare line %d\n", line->id);
list_add(&line->list, &l_mg->free_list);
l_mg->nr_free_lines++;
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index dee64f91227d..8f8c9abd14fc 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -127,10 +127,8 @@ static int pblk_l2p_recover(struct pblk *pblk, bool factory_init)
if (!line) {
/* Configure next line for user data */
line = pblk_line_get_first_data(pblk);
- if (!line) {
- pr_err("pblk: line list corrupted\n");
+ if (!line)
return -EFAULT;
- }
}
return 0;
@@ -141,6 +139,7 @@ static int pblk_l2p_init(struct pblk *pblk, bool factory_init)
sector_t i;
struct ppa_addr ppa;
size_t map_size;
+ int ret = 0;
map_size = pblk_trans_map_size(pblk);
pblk->trans_map = vmalloc(map_size);
@@ -152,7 +151,11 @@ static int pblk_l2p_init(struct pblk *pblk, bool factory_init)
for (i = 0; i < pblk->rl.nr_secs; i++)
pblk_trans_map_set(pblk, i, ppa);
- return pblk_l2p_recover(pblk, factory_init);
+ ret = pblk_l2p_recover(pblk, factory_init);
+ if (ret)
+ vfree(pblk->trans_map);
+
+ return ret;
}
static void pblk_rwb_free(struct pblk *pblk)
--
2.7.4
When cleaning up buffer entries as we wrap up, their state should be
"completed". If any of the entries is in "submitted" state, it means
that something bad has happened. Trigger a warning immediately instead of
waiting for the state flag to eventually be updated, thus hiding the
issue.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-rb.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index 7a632913475f..024a366a995c 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -142,10 +142,9 @@ static void clean_wctx(struct pblk_w_ctx *w_ctx)
{
int flags;
-try:
flags = READ_ONCE(w_ctx->flags);
- if (!(flags & PBLK_SUBMITTED_ENTRY))
- goto try;
+ WARN_ONCE(!(flags & PBLK_SUBMITTED_ENTRY),
+ "pblk: overwriting unsubmitted data\n");
/* Release flags on context. Protect from writes and reads */
smp_store_release(&w_ctx->flags, PBLK_WRITABLE_ENTRY);
--
2.7.4
In the event of a mismatch between the read LBA and the metadata pointer
reported by the device, improve the error message to be able to detect
the offending physical address (PPA) mapped to the corrupted LBA.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-read.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 0d45d4ffc370..89aed634333a 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -113,10 +113,11 @@ static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
return NVM_IO_OK;
}
-static void pblk_read_check_seq(struct pblk *pblk, void *meta_list,
- sector_t blba, int nr_lbas)
+static void pblk_read_check_seq(struct pblk *pblk, struct nvm_rq *rqd,
+ sector_t blba)
{
- struct pblk_sec_meta *meta_lba_list = meta_list;
+ struct pblk_sec_meta *meta_lba_list = rqd->meta_list;
+ int nr_lbas = rqd->nr_ppas;
int i;
for (i = 0; i < nr_lbas; i++) {
@@ -125,17 +126,27 @@ static void pblk_read_check_seq(struct pblk *pblk, void *meta_list,
if (lba == ADDR_EMPTY)
continue;
- WARN(lba != blba + i, "pblk: corrupted read LBA\n");
+ if (lba != blba + i) {
+#ifdef CONFIG_NVM_DEBUG
+ struct ppa_addr *p;
+
+ p = (nr_lbas == 1) ? &rqd->ppa_list[i] : &rqd->ppa_addr;
+ print_ppa(&pblk->dev->geo, p, "seq", i);
+#endif
+ pr_err("pblk: corrupted read LBA (%llu/%llu)\n",
+ lba, (u64)blba + i);
+ WARN_ON(1);
+ }
}
}
/*
* There can be wholes in the lba list.
*/
-static void pblk_read_check_rand(struct pblk *pblk, void *meta_list,
- u64 *lba_list, int nr_lbas)
+static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd,
+ u64 *lba_list, int nr_lbas)
{
- struct pblk_sec_meta *meta_lba_list = meta_list;
+ struct pblk_sec_meta *meta_lba_list = rqd->meta_list;
int i, j;
for (i = 0, j = 0; i < nr_lbas; i++) {
@@ -145,14 +156,25 @@ static void pblk_read_check_rand(struct pblk *pblk, void *meta_list,
if (lba == ADDR_EMPTY)
continue;
- meta_lba = le64_to_cpu(meta_lba_list[j++].lba);
+ meta_lba = le64_to_cpu(meta_lba_list[j].lba);
if (lba != meta_lba) {
+#ifdef CONFIG_NVM_DEBUG
+ struct ppa_addr *p;
+ int nr_ppas = rqd->nr_ppas;
+
+ p = (nr_ppas == 1) ? &rqd->ppa_list[j] : &rqd->ppa_addr;
+ print_ppa(&pblk->dev->geo, p, "seq", j);
+#endif
pr_err("pblk: corrupted read LBA (%llu/%llu)\n",
lba, meta_lba);
WARN_ON(1);
}
+
+ j++;
}
+
+ WARN_ONCE(j != rqd->nr_ppas, "pblk: corrupted random request\n");
}
static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
@@ -197,7 +219,7 @@ static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
WARN_ONCE(bio->bi_status, "pblk: corrupted read error\n");
#endif
- pblk_read_check_seq(pblk, rqd->meta_list, r_ctx->lba, rqd->nr_ppas);
+ pblk_read_check_seq(pblk, rqd, r_ctx->lba);
bio_put(bio);
if (r_ctx->private)
@@ -610,7 +632,7 @@ int pblk_submit_read_gc(struct pblk *pblk, struct pblk_gc_rq *gc_rq)
goto err_free_bio;
}
- pblk_read_check_rand(pblk, rqd.meta_list, gc_rq->lba_list, rqd.nr_ppas);
+ pblk_read_check_rand(pblk, &rqd, gc_rq->lba_list, gc_rq->nr_secs);
atomic_dec(&pblk->inflight_io);
--
2.7.4
Remove unnecessary argument on pblk_line_free()
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-core.c | 6 +++---
drivers/lightnvm/pblk-init.c | 2 +-
drivers/lightnvm/pblk.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 6bc0c7f61aac..1b871441d816 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1337,7 +1337,7 @@ static struct pblk_line *pblk_line_retry(struct pblk *pblk,
retry_line->emeta = line->emeta;
retry_line->meta_line = line->meta_line;
- pblk_line_free(pblk, line);
+ pblk_line_free(line);
l_mg->data_line = retry_line;
spin_unlock(&l_mg->free_lock);
@@ -1562,7 +1562,7 @@ struct pblk_line *pblk_line_replace_data(struct pblk *pblk)
return new;
}
-void pblk_line_free(struct pblk *pblk, struct pblk_line *line)
+void pblk_line_free(struct pblk_line *line)
{
kfree(line->map_bitmap);
kfree(line->invalid_bitmap);
@@ -1584,7 +1584,7 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
WARN_ON(line->state != PBLK_LINESTATE_GC);
line->state = PBLK_LINESTATE_FREE;
line->gc_group = PBLK_LINEGC_NONE;
- pblk_line_free(pblk, line);
+ pblk_line_free(line);
spin_unlock(&line->lock);
atomic_dec(&gc->pipeline_gc);
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 8f8c9abd14fc..b52855f9336b 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -509,7 +509,7 @@ static void pblk_lines_free(struct pblk *pblk)
for (i = 0; i < l_mg->nr_lines; i++) {
line = &pblk->lines[i];
- pblk_line_free(pblk, line);
+ pblk_line_free(line);
pblk_line_meta_free(line);
}
spin_unlock(&l_mg->free_lock);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 856ac41e1201..ff0e5e6421dd 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -766,7 +766,7 @@ struct pblk_line *pblk_line_get_data(struct pblk *pblk);
struct pblk_line *pblk_line_get_erase(struct pblk *pblk);
int pblk_line_erase(struct pblk *pblk, struct pblk_line *line);
int pblk_line_is_full(struct pblk_line *line);
-void pblk_line_free(struct pblk *pblk, struct pblk_line *line);
+void pblk_line_free(struct pblk_line *line);
void pblk_line_close_meta(struct pblk *pblk, struct pblk_line *line);
void pblk_line_close(struct pblk *pblk, struct pblk_line *line);
void pblk_line_close_ws(struct work_struct *work);
--
2.7.4
Do the check for the chunk state after making sure that the chunk type
is supported.
Fixes: 32ef9412c114 ("lightnvm: pblk: implement get log report chunk")
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index b52855f9336b..9e3a43346d4c 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -751,14 +751,14 @@ static int pblk_setup_line_meta_20(struct pblk *pblk, struct pblk_line *line,
chunk->cnlb = chunk_meta->cnlb;
chunk->wp = chunk_meta->wp;
- if (!(chunk->state & NVM_CHK_ST_OFFLINE))
- continue;
-
if (chunk->type & NVM_CHK_TP_SZ_SPEC) {
WARN_ONCE(1, "pblk: custom-sized chunks unsupported\n");
continue;
}
+ if (!(chunk->state & NVM_CHK_ST_OFFLINE))
+ continue;
+
set_bit(pos, line->blk_bitmap);
nr_bad_chks++;
}
--
2.7.4
In the event of a line failing to allocate, fail gracefully and stop the
pipeline to avoid more write failing in the same place.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-init.c | 5 +++++
drivers/lightnvm/pblk-map.c | 33 ++++++++++++++++++++++++---------
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 91a5bc2556a3..dee64f91227d 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -1047,6 +1047,11 @@ static int pblk_lines_init(struct pblk *pblk)
nr_free_chks += pblk_setup_line_meta(pblk, line, chunk_meta, i);
}
+ if (!nr_free_chks) {
+ pr_err("pblk: too many bad blocks prevent for sane instance\n");
+ return -EINTR;
+ }
+
pblk_set_provision(pblk, nr_free_chks);
kfree(chunk_meta);
diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
index 20dbaa89c9df..953ca31dda68 100644
--- a/drivers/lightnvm/pblk-map.c
+++ b/drivers/lightnvm/pblk-map.c
@@ -18,11 +18,11 @@
#include "pblk.h"
-static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
- struct ppa_addr *ppa_list,
- unsigned long *lun_bitmap,
- struct pblk_sec_meta *meta_list,
- unsigned int valid_secs)
+static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
+ struct ppa_addr *ppa_list,
+ unsigned long *lun_bitmap,
+ struct pblk_sec_meta *meta_list,
+ unsigned int valid_secs)
{
struct pblk_line *line = pblk_line_get_data(pblk);
struct pblk_emeta *emeta;
@@ -35,8 +35,14 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
if (pblk_line_is_full(line)) {
struct pblk_line *prev_line = line;
+ /* If we cannot allocate a new line, make sure to store metadata
+ * on current line and then fail
+ */
line = pblk_line_replace_data(pblk);
pblk_line_close_meta(pblk, prev_line);
+
+ if (!line)
+ return -EINTR;
}
emeta = line->emeta;
@@ -74,6 +80,7 @@ static void pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
}
pblk_down_rq(pblk, ppa_list, nr_secs, lun_bitmap);
+ return 0;
}
void pblk_map_rq(struct pblk *pblk, struct nvm_rq *rqd, unsigned int sentry,
@@ -87,8 +94,12 @@ void pblk_map_rq(struct pblk *pblk, struct nvm_rq *rqd, unsigned int sentry,
for (i = off; i < rqd->nr_ppas; i += min) {
map_secs = (i + min > valid_secs) ? (valid_secs % min) : min;
- pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
- lun_bitmap, &meta_list[i], map_secs);
+ if (pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
+ lun_bitmap, &meta_list[i], map_secs)) {
+ bio_put(rqd->bio);
+ pblk_free_rqd(pblk, rqd, PBLK_WRITE);
+ pblk_pipeline_stop(pblk);
+ }
}
}
@@ -108,8 +119,12 @@ void pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd,
for (i = 0; i < rqd->nr_ppas; i += min) {
map_secs = (i + min > valid_secs) ? (valid_secs % min) : min;
- pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
- lun_bitmap, &meta_list[i], map_secs);
+ if (pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
+ lun_bitmap, &meta_list[i], map_secs)) {
+ bio_put(rqd->bio);
+ pblk_free_rqd(pblk, rqd, PBLK_WRITE);
+ pblk_pipeline_stop(pblk);
+ }
erase_lun = pblk_ppa_to_pos(geo, rqd->ppa_list[i]);
--
2.7.4
Check that the lba stored in the LBA metadata is correct in the GC path
too. This requires a new helper function to check random reads in the
vector read.
Signed-off-by: Javier González <[email protected]>
---
drivers/lightnvm/pblk-read.c | 39 +++++++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 9eee10f69df0..0d45d4ffc370 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -113,15 +113,14 @@ static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
return NVM_IO_OK;
}
-static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
- sector_t blba)
+static void pblk_read_check_seq(struct pblk *pblk, void *meta_list,
+ sector_t blba, int nr_lbas)
{
- struct pblk_sec_meta *meta_list = rqd->meta_list;
- int nr_lbas = rqd->nr_ppas;
+ struct pblk_sec_meta *meta_lba_list = meta_list;
int i;
for (i = 0; i < nr_lbas; i++) {
- u64 lba = le64_to_cpu(meta_list[i].lba);
+ u64 lba = le64_to_cpu(meta_lba_list[i].lba);
if (lba == ADDR_EMPTY)
continue;
@@ -130,6 +129,32 @@ static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
}
}
+/*
+ * There can be wholes in the lba list.
+ */
+static void pblk_read_check_rand(struct pblk *pblk, void *meta_list,
+ u64 *lba_list, int nr_lbas)
+{
+ struct pblk_sec_meta *meta_lba_list = meta_list;
+ int i, j;
+
+ for (i = 0, j = 0; i < nr_lbas; i++) {
+ u64 lba = lba_list[i];
+ u64 meta_lba;
+
+ if (lba == ADDR_EMPTY)
+ continue;
+
+ meta_lba = le64_to_cpu(meta_lba_list[j++].lba);
+
+ if (lba != meta_lba) {
+ pr_err("pblk: corrupted read LBA (%llu/%llu)\n",
+ lba, meta_lba);
+ WARN_ON(1);
+ }
+ }
+}
+
static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
{
struct ppa_addr *ppa_list;
@@ -172,7 +197,7 @@ static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
WARN_ONCE(bio->bi_status, "pblk: corrupted read error\n");
#endif
- pblk_read_check(pblk, rqd, r_ctx->lba);
+ pblk_read_check_seq(pblk, rqd->meta_list, r_ctx->lba, rqd->nr_ppas);
bio_put(bio);
if (r_ctx->private)
@@ -585,6 +610,8 @@ int pblk_submit_read_gc(struct pblk *pblk, struct pblk_gc_rq *gc_rq)
goto err_free_bio;
}
+ pblk_read_check_rand(pblk, rqd.meta_list, gc_rq->lba_list, rqd.nr_ppas);
+
atomic_dec(&pblk->inflight_io);
if (rqd.error) {
--
2.7.4
On 4/16/18 12:25 PM, Javier González wrote:
> Check that the lba stored in the LBA metadata is correct in the GC path
> too. This requires a new helper function to check random reads in the
> vector read.
>
> Signed-off-by: Javier González <[email protected]>
> ---
> drivers/lightnvm/pblk-read.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> index 9eee10f69df0..0d45d4ffc370 100644
> --- a/drivers/lightnvm/pblk-read.c
> +++ b/drivers/lightnvm/pblk-read.c
> @@ -113,15 +113,14 @@ static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
> return NVM_IO_OK;
> }
>
> -static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
> - sector_t blba)
> +static void pblk_read_check_seq(struct pblk *pblk, void *meta_list,
> + sector_t blba, int nr_lbas)
> {
> - struct pblk_sec_meta *meta_list = rqd->meta_list;
> - int nr_lbas = rqd->nr_ppas;
> + struct pblk_sec_meta *meta_lba_list = meta_list;
> int i;
>
> for (i = 0; i < nr_lbas; i++) {
> - u64 lba = le64_to_cpu(meta_list[i].lba);
> + u64 lba = le64_to_cpu(meta_lba_list[i].lba);
>
> if (lba == ADDR_EMPTY)
> continue;
> @@ -130,6 +129,32 @@ static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
> }
> }
>
> +/*
> + * There can be wholes in the lba list.
> + */
holes
> +static void pblk_read_check_rand(struct pblk *pblk, void *meta_list,
> + u64 *lba_list, int nr_lbas)
> +{
> + struct pblk_sec_meta *meta_lba_list = meta_list;
> + int i, j;
> +
> + for (i = 0, j = 0; i < nr_lbas; i++) {
> + u64 lba = lba_list[i];
> + u64 meta_lba;
> +
> + if (lba == ADDR_EMPTY)
> + continue;
> +
> + meta_lba = le64_to_cpu(meta_lba_list[j++].lba);
You can move the j++ into the for loop. Although, why not use just the i
iterator?
> +
> + if (lba != meta_lba) {
> + pr_err("pblk: corrupted read LBA (%llu/%llu)\n",
> + lba, meta_lba);
> + WARN_ON(1);
I don't understand this, if a corrupted LBA is found here, how is it
communicated back to pblk and the LBA can be recovered from elsewhere.
If the data is wrong, this should have been communicated by the drive on
the completion path. The drive is defect if this happens.
> + }
> + }
> +}
> +
> static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
> {
> struct ppa_addr *ppa_list;
> @@ -172,7 +197,7 @@ static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
> WARN_ONCE(bio->bi_status, "pblk: corrupted read error\n");
> #endif
>
> - pblk_read_check(pblk, rqd, r_ctx->lba);
> + pblk_read_check_seq(pblk, rqd->meta_list, r_ctx->lba, rqd->nr_ppas);
>
> bio_put(bio);
> if (r_ctx->private)
> @@ -585,6 +610,8 @@ int pblk_submit_read_gc(struct pblk *pblk, struct pblk_gc_rq *gc_rq)
> goto err_free_bio;
> }
>
> + pblk_read_check_rand(pblk, rqd.meta_list, gc_rq->lba_list, rqd.nr_ppas);
> +
> atomic_dec(&pblk->inflight_io);
>
> if (rqd.error) {
>
On 4/16/18 12:25 PM, Javier González wrote:
> Remove unnecessary indirection on the read path.
>
Title and description are the same. Can you elaborate what changed since
pblk_submit_io now directly can be returned, and doesn't have its return
value rewritten to NVM_IO_ERR?
> Signed-off-by: Javier González <[email protected]>
> ---
> drivers/lightnvm/pblk-read.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> index 89aed634333a..2f8224354c62 100644
> --- a/drivers/lightnvm/pblk-read.c
> +++ b/drivers/lightnvm/pblk-read.c
> @@ -102,16 +102,6 @@ static void pblk_read_ppalist_rq(struct pblk *pblk, struct nvm_rq *rqd,
> #endif
> }
>
> -static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
> -{
> - int err;
> -
> - err = pblk_submit_io(pblk, rqd);
> - if (err)
> - return NVM_IO_ERR;
> -
> - return NVM_IO_OK;
> -}
>
> static void pblk_read_check_seq(struct pblk *pblk, struct nvm_rq *rqd,
> sector_t blba)
> @@ -485,7 +475,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
> rqd->bio = int_bio;
> r_ctx->private = bio;
>
> - ret = pblk_submit_read_io(pblk, rqd);
> + ret = pblk_submit_io(pblk, rqd);
> if (ret) {
> pr_err("pblk: read IO submission failed\n");
> if (int_bio)
>
On 4/16/18 12:25 PM, Javier González wrote:
> Remove unnecessary argument on pblk_line_free()
>
Why was the argument no longer needed?
> Signed-off-by: Javier González <[email protected]>
> ---
> drivers/lightnvm/pblk-core.c | 6 +++---
> drivers/lightnvm/pblk-init.c | 2 +-
> drivers/lightnvm/pblk.h | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 6bc0c7f61aac..1b871441d816 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1337,7 +1337,7 @@ static struct pblk_line *pblk_line_retry(struct pblk *pblk,
> retry_line->emeta = line->emeta;
> retry_line->meta_line = line->meta_line;
>
> - pblk_line_free(pblk, line);
> + pblk_line_free(line);
> l_mg->data_line = retry_line;
> spin_unlock(&l_mg->free_lock);
>
> @@ -1562,7 +1562,7 @@ struct pblk_line *pblk_line_replace_data(struct pblk *pblk)
> return new;
> }
>
> -void pblk_line_free(struct pblk *pblk, struct pblk_line *line)
> +void pblk_line_free(struct pblk_line *line)
> {
> kfree(line->map_bitmap);
> kfree(line->invalid_bitmap);
> @@ -1584,7 +1584,7 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> WARN_ON(line->state != PBLK_LINESTATE_GC);
> line->state = PBLK_LINESTATE_FREE;
> line->gc_group = PBLK_LINEGC_NONE;
> - pblk_line_free(pblk, line);
> + pblk_line_free(line);
> spin_unlock(&line->lock);
>
> atomic_dec(&gc->pipeline_gc);
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 8f8c9abd14fc..b52855f9336b 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -509,7 +509,7 @@ static void pblk_lines_free(struct pblk *pblk)
> for (i = 0; i < l_mg->nr_lines; i++) {
> line = &pblk->lines[i];
>
> - pblk_line_free(pblk, line);
> + pblk_line_free(line);
> pblk_line_meta_free(line);
> }
> spin_unlock(&l_mg->free_lock);
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index 856ac41e1201..ff0e5e6421dd 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -766,7 +766,7 @@ struct pblk_line *pblk_line_get_data(struct pblk *pblk);
> struct pblk_line *pblk_line_get_erase(struct pblk *pblk);
> int pblk_line_erase(struct pblk *pblk, struct pblk_line *line);
> int pblk_line_is_full(struct pblk_line *line);
> -void pblk_line_free(struct pblk *pblk, struct pblk_line *line);
> +void pblk_line_free(struct pblk_line *line);
> void pblk_line_close_meta(struct pblk *pblk, struct pblk_line *line);
> void pblk_line_close(struct pblk *pblk, struct pblk_line *line);
> void pblk_line_close_ws(struct work_struct *work);
>
Javier
> On 17 Apr 2018, at 05.12, Matias Bjørling <[email protected]> wrote:
>
> On 4/16/18 12:25 PM, Javier González wrote:
>> Remove unnecessary argument on pblk_line_free()
>
> Why was the argument no longer needed?
You can see it is not used... It a straightforward clean up.
Javier
> On 17 Apr 2018, at 05.03, Matias Bjørling <[email protected]> wrote:
>
> On 4/16/18 12:25 PM, Javier González wrote:
>> Check that the lba stored in the LBA metadata is correct in the GC path
>> too. This requires a new helper function to check random reads in the
>> vector read.
>> Signed-off-by: Javier González <[email protected]>
>> ---
>> drivers/lightnvm/pblk-read.c | 39 +++++++++++++++++++++++++++++++++------
>> 1 file changed, 33 insertions(+), 6 deletions(-)
>> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
>> index 9eee10f69df0..0d45d4ffc370 100644
>> --- a/drivers/lightnvm/pblk-read.c
>> +++ b/drivers/lightnvm/pblk-read.c
>> @@ -113,15 +113,14 @@ static int pblk_submit_read_io(struct pblk *pblk, struct nvm_rq *rqd)
>> return NVM_IO_OK;
>> }
>> -static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
>> - sector_t blba)
>> +static void pblk_read_check_seq(struct pblk *pblk, void *meta_list,
>> + sector_t blba, int nr_lbas)
>> {
>> - struct pblk_sec_meta *meta_list = rqd->meta_list;
>> - int nr_lbas = rqd->nr_ppas;
>> + struct pblk_sec_meta *meta_lba_list = meta_list;
>> int i;
>> for (i = 0; i < nr_lbas; i++) {
>> - u64 lba = le64_to_cpu(meta_list[i].lba);
>> + u64 lba = le64_to_cpu(meta_lba_list[i].lba);
>> if (lba == ADDR_EMPTY)
>> continue;
>> @@ -130,6 +129,32 @@ static void pblk_read_check(struct pblk *pblk, struct nvm_rq *rqd,
>> }
>> }
>> +/*
>> + * There can be wholes in the lba list.
>> + */
>
> holes
Can you change it?
>
>> +static void pblk_read_check_rand(struct pblk *pblk, void *meta_list,
>> + u64 *lba_list, int nr_lbas)
>> +{
>> + struct pblk_sec_meta *meta_lba_list = meta_list;
>> + int i, j;
>> +
>> + for (i = 0, j = 0; i < nr_lbas; i++) {
>> + u64 lba = lba_list[i];
>> + u64 meta_lba;
>> +
>> + if (lba == ADDR_EMPTY)
>> + continue;
>> +
>> + meta_lba = le64_to_cpu(meta_lba_list[j++].lba);
>
> You can move the j++ into the for loop. Although, why not use just the i iterator?
Look above, we only increase on valid lbas.
>
>> +
>> + if (lba != meta_lba) {
>> + pr_err("pblk: corrupted read LBA (%llu/%llu)\n",
>> + lba, meta_lba);
>> + WARN_ON(1);
>
> I don't understand this, if a corrupted LBA is found here, how is it
> communicated back to pblk and the LBA can be recovered from elsewhere.
> If the data is wrong, this should have been communicated by the drive
> on the completion path. The drive is defect if this happens.
>
This happens if a read does not match the lba stored on the LBA. This is
definitely an error and it has helped us in the past to find race
conditions on pblk's read path, specially when forming partial I/Os.
If the read failed from the drive (e.g., when enabling HECC), we still
want information about where it failed for debugging purposes. Same we
do on the normal read path.
Javier
> On 17 Apr 2018, at 05.11, Matias Bjørling <[email protected]> wrote:
>
> On 4/16/18 12:25 PM, Javier González wrote:
>> Remove unnecessary indirection on the read path.
>
> Title and description are the same. Can you elaborate what changed
> since pblk_submit_io now directly can be returned, and doesn't have
> its return value rewritten to NVM_IO_ERR?
>
My bad - I assumed submit_io was giving NVM_ errors. I'll resend
returning NVM_IO_ERR - the indirection is still unnecessary (and it's
not used anywhere else in the code).
Javier