From: Hans Holmberg <[email protected]>
This patchset contains a bunch of cleanups and bugfixes for 4.20.
The patches depend on some of Javier's for-4.20 patches.
For ordering, see:
Branch: for-4.20/pblk
Remote: ssh://github.com/OpenChannelSSD/linux
Hans Holmberg (7):
lightnvm: introduce nvm_rq_to_ppa_list
lightnvm: pblk: fix mapping issue on failed writes
lightnvm: pblk: allocate line map bitmaps using a mempool
lightnvm: pblk: move global caches to module init/exit
lightnvm: pblk: remove unused parameters in pblk_up_rq
lightnvm: pblk: fix up prints in pblk_read_check_rand
lightnvm: pblk: fix write amplificiation calculation
drivers/lightnvm/core.c | 14 +++------
drivers/lightnvm/pblk-core.c | 25 +++++++++------
drivers/lightnvm/pblk-init.c | 60 ++++++++++++++++++++++-------------
drivers/lightnvm/pblk-map.c | 10 +++---
drivers/lightnvm/pblk-read.c | 13 +++-----
drivers/lightnvm/pblk-recovery.c | 11 ++++---
drivers/lightnvm/pblk-sysfs.c | 3 +-
drivers/lightnvm/pblk-write.c | 68 +++++++++++++++++++++++++++-------------
drivers/lightnvm/pblk.h | 11 ++++---
include/linux/lightnvm.h | 5 +++
10 files changed, 134 insertions(+), 86 deletions(-)
--
2.7.4
From: Hans Holmberg <[email protected]>
When the user data counter exceeds 32 bits, the write amplification
calculation does not provide the right value. Fix this by using
div64_u64 in stead of div64.
Fixes: 76758390f83e ("lightnvm: pblk: export write amplification counters to sysfs")
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-sysfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c
index 9fc3dfa168b4..e56eeeeb914e 100644
--- a/drivers/lightnvm/pblk-sysfs.c
+++ b/drivers/lightnvm/pblk-sysfs.c
@@ -337,7 +337,6 @@ static ssize_t pblk_get_write_amp(u64 user, u64 gc, u64 pad,
{
int sz;
-
sz = snprintf(page, PAGE_SIZE,
"user:%lld gc:%lld pad:%lld WA:",
user, gc, pad);
@@ -349,7 +348,7 @@ static ssize_t pblk_get_write_amp(u64 user, u64 gc, u64 pad,
u32 wa_frac;
wa_int = (user + gc + pad) * 100000;
- wa_int = div_u64(wa_int, user);
+ wa_int = div64_u64(wa_int, user);
wa_int = div_u64_rem(wa_int, 100000, &wa_frac);
sz += snprintf(page + sz, PAGE_SIZE - sz, "%llu.%05u\n",
--
2.7.4
From: Hans Holmberg <[email protected]>
Pblk's global caches should not be initialized every time
a pblk instance is created, so move the creation and destruction
of the caches to module init/exit.
Also remove the global pblk lock as it is no longer needed after
the move.
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-init.c | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 76a4a271b9cf..0be64220b5d8 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -27,7 +27,6 @@ MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
*pblk_w_rq_cache;
-static DECLARE_RWSEM(pblk_lock);
struct bio_set pblk_bio_set;
static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
@@ -306,21 +305,17 @@ static int pblk_set_addrf(struct pblk *pblk)
return 0;
}
-static int pblk_init_global_caches(struct pblk *pblk)
+static int pblk_init_global_caches(void)
{
- down_write(&pblk_lock);
pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
sizeof(struct pblk_line_ws), 0, 0, NULL);
- if (!pblk_ws_cache) {
- up_write(&pblk_lock);
+ if (!pblk_ws_cache)
return -ENOMEM;
- }
pblk_rec_cache = kmem_cache_create("pblk_rec",
sizeof(struct pblk_rec_ctx), 0, 0, NULL);
if (!pblk_rec_cache) {
kmem_cache_destroy(pblk_ws_cache);
- up_write(&pblk_lock);
return -ENOMEM;
}
@@ -329,7 +324,6 @@ static int pblk_init_global_caches(struct pblk *pblk)
if (!pblk_g_rq_cache) {
kmem_cache_destroy(pblk_ws_cache);
kmem_cache_destroy(pblk_rec_cache);
- up_write(&pblk_lock);
return -ENOMEM;
}
@@ -339,15 +333,13 @@ static int pblk_init_global_caches(struct pblk *pblk)
kmem_cache_destroy(pblk_ws_cache);
kmem_cache_destroy(pblk_rec_cache);
kmem_cache_destroy(pblk_g_rq_cache);
- up_write(&pblk_lock);
return -ENOMEM;
}
- up_write(&pblk_lock);
return 0;
}
-static void pblk_free_global_caches(struct pblk *pblk)
+static void pblk_free_global_caches(void)
{
kmem_cache_destroy(pblk_ws_cache);
kmem_cache_destroy(pblk_rec_cache);
@@ -381,13 +373,10 @@ static int pblk_core_init(struct pblk *pblk)
if (!pblk->pad_dist)
return -ENOMEM;
- if (pblk_init_global_caches(pblk))
- goto fail_free_pad_dist;
-
/* Internal bios can be at most the sectors signaled by the device. */
ret = mempool_init_page_pool(&pblk->page_bio_pool, NVM_MAX_VLBA, 0);
if (ret)
- goto free_global_caches;
+ goto free_pad_dist;
ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
pblk_ws_cache);
@@ -455,9 +444,7 @@ static int pblk_core_init(struct pblk *pblk)
mempool_exit(&pblk->gen_ws_pool);
free_page_bio_pool:
mempool_exit(&pblk->page_bio_pool);
-free_global_caches:
- pblk_free_global_caches(pblk);
-fail_free_pad_dist:
+free_pad_dist:
kfree(pblk->pad_dist);
return -ENOMEM;
}
@@ -480,7 +467,6 @@ static void pblk_core_free(struct pblk *pblk)
mempool_exit(&pblk->e_rq_pool);
mempool_exit(&pblk->w_rq_pool);
- pblk_free_global_caches(pblk);
kfree(pblk->pad_dist);
}
@@ -1067,7 +1053,6 @@ static void pblk_exit(void *private, bool graceful)
{
struct pblk *pblk = private;
- down_write(&pblk_lock);
pblk_gc_exit(pblk, graceful);
pblk_tear_down(pblk, graceful);
@@ -1076,7 +1061,6 @@ static void pblk_exit(void *private, bool graceful)
#endif
pblk_free(pblk);
- up_write(&pblk_lock);
}
static sector_t pblk_capacity(void *private)
@@ -1237,9 +1221,22 @@ static int __init pblk_module_init(void)
ret = bioset_init(&pblk_bio_set, BIO_POOL_SIZE, 0, 0);
if (ret)
return ret;
+
ret = nvm_register_tgt_type(&tt_pblk);
if (ret)
- bioset_exit(&pblk_bio_set);
+ goto fail_exit_bioset;
+
+ ret = pblk_init_global_caches();
+ if (ret)
+ goto fail_unregister_tgt_type;
+
+ return 0;
+
+fail_unregister_tgt_type:
+ nvm_unregister_tgt_type(&tt_pblk);
+fail_exit_bioset:
+ bioset_exit(&pblk_bio_set);
+
return ret;
}
@@ -1247,6 +1244,7 @@ static void pblk_module_exit(void)
{
bioset_exit(&pblk_bio_set);
nvm_unregister_tgt_type(&tt_pblk);
+ pblk_free_global_caches();
}
module_init(pblk_module_init);
--
2.7.4
From: Hans Holmberg <[email protected]>
The prefix when printing ppas in pblk_read_check_rand should be "rnd"
not "seq", so fix this so we can differentiate between lba missmatches
in random and sequential reads. Also change the print order so
we align with pblk_read_check_seq, printing read lba first.
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-read.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index df3c4eb13be9..6d13763f2f6a 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -149,10 +149,10 @@ static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd,
#ifdef CONFIG_NVM_PBLK_DEBUG
struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
- print_ppa(pblk, &ppa_list[j], "seq", j);
+ print_ppa(pblk, &ppa_list[j], "rnd", j);
#endif
pblk_err(pblk, "corrupted read LBA (%llu/%llu)\n",
- lba, meta_lba);
+ meta_lba, lba);
WARN_ON(1);
}
--
2.7.4
From: Hans Holmberg <[email protected]>
Line map bitmap allocations are fairly large and can fail. Allocation
failures are fatal to pblk, stoping the write pipeline. To avoid this,
allocate the bitmaps using a mempool instead.
Mempool allocations never fail if called from a process context,
and pblk *should* only allocate map bitmaps in process context,
but keep the failure handling for robustness sake.
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-core.c | 22 +++++++++++++++-------
drivers/lightnvm/pblk-init.c | 18 ++++++++++++++++++
drivers/lightnvm/pblk-recovery.c | 2 +-
drivers/lightnvm/pblk.h | 4 ++++
4 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 36ac9eff8ebd..a1033d82b9cc 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1049,15 +1049,18 @@ static int pblk_line_init_metadata(struct pblk *pblk, struct pblk_line *line,
static int pblk_line_alloc_bitmaps(struct pblk *pblk, struct pblk_line *line)
{
struct pblk_line_meta *lm = &pblk->lm;
+ struct pblk_line_mgmt *l_mg = &pblk->l_mg;
- line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_KERNEL);
+ line->map_bitmap = mempool_alloc(l_mg->bitmap_pool, GFP_KERNEL);
if (!line->map_bitmap)
return -ENOMEM;
+ memset(line->map_bitmap, 0, lm->sec_bitmap_len);
+
/* will be initialized using bb info from map_bitmap */
- line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_KERNEL);
+ line->invalid_bitmap = mempool_alloc(l_mg->bitmap_pool, GFP_KERNEL);
if (!line->invalid_bitmap) {
- kfree(line->map_bitmap);
+ mempool_free(line->map_bitmap, l_mg->bitmap_pool);
line->map_bitmap = NULL;
return -ENOMEM;
}
@@ -1243,7 +1246,9 @@ int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line)
void pblk_line_recov_close(struct pblk *pblk, struct pblk_line *line)
{
- kfree(line->map_bitmap);
+ struct pblk_line_mgmt *l_mg = &pblk->l_mg;
+
+ mempool_free(line->map_bitmap, l_mg->bitmap_pool);
line->map_bitmap = NULL;
line->smeta = NULL;
line->emeta = NULL;
@@ -1261,8 +1266,11 @@ static void pblk_line_reinit(struct pblk_line *line)
void pblk_line_free(struct pblk_line *line)
{
- kfree(line->map_bitmap);
- kfree(line->invalid_bitmap);
+ struct pblk *pblk = line->pblk;
+ struct pblk_line_mgmt *l_mg = &pblk->l_mg;
+
+ mempool_free(line->map_bitmap, l_mg->bitmap_pool);
+ mempool_free(line->invalid_bitmap, l_mg->bitmap_pool);
pblk_line_reinit(line);
}
@@ -1741,7 +1749,7 @@ void pblk_line_close(struct pblk *pblk, struct pblk_line *line)
list_add_tail(&line->list, move_list);
- kfree(line->map_bitmap);
+ mempool_free(line->map_bitmap, l_mg->bitmap_pool);
line->map_bitmap = NULL;
line->smeta = NULL;
line->emeta = NULL;
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 8adc8ac8b03c..76a4a271b9cf 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -498,6 +498,9 @@ static void pblk_line_mg_free(struct pblk *pblk)
pblk_mfree(l_mg->eline_meta[i]->buf, l_mg->emeta_alloc_type);
kfree(l_mg->eline_meta[i]);
}
+
+ mempool_destroy(l_mg->bitmap_pool);
+ kmem_cache_destroy(l_mg->bitmap_cache);
}
static void pblk_line_meta_free(struct pblk_line_mgmt *l_mg,
@@ -797,6 +800,17 @@ static int pblk_line_mg_init(struct pblk *pblk)
goto fail_free_smeta;
}
+ l_mg->bitmap_cache = kmem_cache_create("pblk_lm_bitmap",
+ lm->sec_bitmap_len, 0, 0, NULL);
+ if (!l_mg->bitmap_cache)
+ goto fail_free_smeta;
+
+ /* the bitmap pool is used for both valid and map bitmaps */
+ l_mg->bitmap_pool = mempool_create_slab_pool(PBLK_DATA_LINES * 2,
+ l_mg->bitmap_cache);
+ if (!l_mg->bitmap_pool)
+ goto fail_destroy_bitmap_cache;
+
/* emeta allocates three different buffers for managing metadata with
* in-memory and in-media layouts
*/
@@ -849,6 +863,10 @@ static int pblk_line_mg_init(struct pblk *pblk)
kfree(l_mg->eline_meta[i]->buf);
kfree(l_mg->eline_meta[i]);
}
+
+ mempool_destroy(l_mg->bitmap_pool);
+fail_destroy_bitmap_cache:
+ kmem_cache_destroy(l_mg->bitmap_cache);
fail_free_smeta:
for (i = 0; i < PBLK_DATA_LINES; i++)
kfree(l_mg->sline_meta[i]);
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 3bd2b6b0a359..eea901d7cebc 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -939,7 +939,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
list_move_tail(&line->list, move_list);
spin_unlock(&l_mg->gc_lock);
- kfree(line->map_bitmap);
+ mempool_free(line->map_bitmap, l_mg->bitmap_pool);
line->map_bitmap = NULL;
line->smeta = NULL;
line->emeta = NULL;
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index fd274985aa82..1a31fda3a9f2 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -530,6 +530,10 @@ struct pblk_line_mgmt {
struct pblk_emeta *eline_meta[PBLK_DATA_LINES];
unsigned long meta_bitmap;
+ /* Cache and mempool for map/invalid bitmaps */
+ struct kmem_cache *bitmap_cache;
+ mempool_t *bitmap_pool;
+
/* Helpers for fast bitmap calculations */
unsigned long *bb_template;
unsigned long *bb_aux;
--
2.7.4
From: Hans Holmberg <[email protected]>
The parameters nr_ppas and ppa_list are not used, so remove them.
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-core.c | 3 +--
drivers/lightnvm/pblk-write.c | 5 ++---
drivers/lightnvm/pblk.h | 3 +--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index a1033d82b9cc..f7bff3ae52ac 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1920,8 +1920,7 @@ void pblk_up_page(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas)
up(&rlun->wr_sem);
}
-void pblk_up_rq(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas,
- unsigned long *lun_bitmap)
+void pblk_up_rq(struct pblk *pblk, unsigned long *lun_bitmap)
{
struct nvm_tgt_dev *dev = pblk->dev;
struct nvm_geo *geo = &dev->geo;
diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index fd67325f6e54..8aa4b9ee5092 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -81,8 +81,7 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
#ifdef CONFIG_NVM_PBLK_DEBUG
atomic_long_sub(c_ctx->nr_valid, &pblk->inflight_writes);
#endif
-
- pblk_up_rq(pblk, rqd->ppa_list, rqd->nr_ppas, c_ctx->lun_bitmap);
+ pblk_up_rq(pblk, c_ctx->lun_bitmap);
pos = pblk_rb_sync_init(&pblk->rwb, &flags);
if (pos == c_ctx->sentry) {
@@ -242,7 +241,7 @@ static void pblk_submit_rec(struct work_struct *work)
pblk_map_remaining(pblk, ppa_list);
pblk_queue_resubmit(pblk, c_ctx);
- pblk_up_rq(pblk, rqd->ppa_list, rqd->nr_ppas, c_ctx->lun_bitmap);
+ pblk_up_rq(pblk, c_ctx->lun_bitmap);
if (c_ctx->nr_padded)
pblk_bio_free_pages(pblk, rqd->bio, c_ctx->nr_valid,
c_ctx->nr_padded);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 1a31fda3a9f2..1bdac22e7897 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -827,8 +827,7 @@ void pblk_up_page(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas);
void pblk_down_rq(struct pblk *pblk, struct ppa_addr *ppa_list, int nr_ppas,
unsigned long *lun_bitmap);
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_up_rq(struct pblk *pblk, unsigned long *lun_bitmap);
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
From: Hans Holmberg <[email protected]>
There is a number of places in the lightnvm subsystem where the user
iterates over the ppa list. Before iterating, the user must know if it
is a single or multiple LBAs due to vector commands using either the
nvm_rq ->ppa_addr or ->ppa_list fields on command submission, which
leads to open-coding the if/else statement.
Instead of having multiple if/else's, move it into a function that can
be called by its users.
A nice side effect of this cleanup is that this patch fixes up a
bunch of cases where we don't consider the single-ppa case in pblk.
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/core.c | 14 ++++----------
drivers/lightnvm/pblk-map.c | 10 ++++++----
drivers/lightnvm/pblk-read.c | 11 ++++-------
drivers/lightnvm/pblk-recovery.c | 9 ++++++---
drivers/lightnvm/pblk-write.c | 18 ++++++++----------
drivers/lightnvm/pblk.h | 4 +---
include/linux/lightnvm.h | 5 +++++
7 files changed, 34 insertions(+), 37 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 5d0ed33d7c59..f7154398ba38 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -603,22 +603,16 @@ static void nvm_ppa_dev_to_tgt(struct nvm_tgt_dev *tgt_dev,
static void nvm_rq_tgt_to_dev(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
{
- if (rqd->nr_ppas == 1) {
- nvm_ppa_tgt_to_dev(tgt_dev, &rqd->ppa_addr, 1);
- return;
- }
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
- nvm_ppa_tgt_to_dev(tgt_dev, rqd->ppa_list, rqd->nr_ppas);
+ nvm_ppa_tgt_to_dev(tgt_dev, ppa_list, rqd->nr_ppas);
}
static void nvm_rq_dev_to_tgt(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
{
- if (rqd->nr_ppas == 1) {
- nvm_ppa_dev_to_tgt(tgt_dev, &rqd->ppa_addr, 1);
- return;
- }
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
- nvm_ppa_dev_to_tgt(tgt_dev, rqd->ppa_list, rqd->nr_ppas);
+ nvm_ppa_dev_to_tgt(tgt_dev, ppa_list, rqd->nr_ppas);
}
int nvm_register_tgt_type(struct nvm_tgt_type *tt)
diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
index 953ca31dda68..dc0efb852475 100644
--- a/drivers/lightnvm/pblk-map.c
+++ b/drivers/lightnvm/pblk-map.c
@@ -88,13 +88,14 @@ void pblk_map_rq(struct pblk *pblk, struct nvm_rq *rqd, unsigned int sentry,
unsigned int off)
{
struct pblk_sec_meta *meta_list = rqd->meta_list;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
unsigned int map_secs;
int min = pblk->min_write_pgs;
int i;
for (i = off; i < rqd->nr_ppas; i += min) {
map_secs = (i + min > valid_secs) ? (valid_secs % min) : min;
- if (pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
+ if (pblk_map_page_data(pblk, sentry + i, &ppa_list[i],
lun_bitmap, &meta_list[i], map_secs)) {
bio_put(rqd->bio);
pblk_free_rqd(pblk, rqd, PBLK_WRITE);
@@ -112,6 +113,7 @@ void pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd,
struct nvm_geo *geo = &dev->geo;
struct pblk_line_meta *lm = &pblk->lm;
struct pblk_sec_meta *meta_list = rqd->meta_list;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
struct pblk_line *e_line, *d_line;
unsigned int map_secs;
int min = pblk->min_write_pgs;
@@ -119,14 +121,14 @@ 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;
- if (pblk_map_page_data(pblk, sentry + i, &rqd->ppa_list[i],
+ if (pblk_map_page_data(pblk, sentry + i, &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]);
+ erase_lun = pblk_ppa_to_pos(geo, ppa_list[i]);
/* line can change after page map. We might also be writing the
* last line.
@@ -141,7 +143,7 @@ void pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd,
set_bit(erase_lun, e_line->erase_bitmap);
atomic_dec(&e_line->left_eblks);
- *erase_ppa = rqd->ppa_list[i];
+ *erase_ppa = ppa_list[i];
erase_ppa->a.blk = e_line->id;
spin_unlock(&e_line->lock);
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 928c638af5b6..df3c4eb13be9 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -116,10 +116,9 @@ static void pblk_read_check_seq(struct pblk *pblk, struct nvm_rq *rqd,
if (lba != blba + i) {
#ifdef CONFIG_NVM_PBLK_DEBUG
- struct ppa_addr *p;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
- p = (nr_lbas == 1) ? &rqd->ppa_list[i] : &rqd->ppa_addr;
- print_ppa(pblk, p, "seq", i);
+ print_ppa(pblk, &ppa_list[i], "seq", i);
#endif
pblk_err(pblk, "corrupted read LBA (%llu/%llu)\n",
lba, (u64)blba + i);
@@ -148,11 +147,9 @@ static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd,
if (lba != meta_lba) {
#ifdef CONFIG_NVM_PBLK_DEBUG
- struct ppa_addr *p;
- int nr_ppas = rqd->nr_ppas;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
- p = (nr_ppas == 1) ? &rqd->ppa_list[j] : &rqd->ppa_addr;
- print_ppa(pblk, p, "seq", j);
+ print_ppa(pblk, &ppa_list[j], "seq", j);
#endif
pblk_err(pblk, "corrupted read LBA (%llu/%llu)\n",
lba, meta_lba);
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index cf629ab016ba..3bd2b6b0a359 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -161,6 +161,8 @@ static int pblk_recov_read_oob(struct pblk *pblk, struct pblk_line *line,
if (pblk_io_aligned(pblk, rq_ppas))
rqd->is_seq = 1;
+ ppa_list = nvm_rq_to_ppa_list(rqd);
+
for (i = 0; i < rqd->nr_ppas; ) {
struct ppa_addr ppa;
int pos;
@@ -175,7 +177,7 @@ static int pblk_recov_read_oob(struct pblk *pblk, struct pblk_line *line,
}
for (j = 0; j < pblk->min_write_pgs; j++, i++, r_ptr_int++)
- rqd->ppa_list[i] =
+ ppa_list[i] =
addr_to_gen_ppa(pblk, r_ptr_int, line->id);
}
@@ -202,7 +204,7 @@ static int pblk_recov_read_oob(struct pblk *pblk, struct pblk_line *line,
if (lba == ADDR_EMPTY || lba > pblk->rl.nr_secs)
continue;
- pblk_update_map(pblk, lba, rqd->ppa_list[i]);
+ pblk_update_map(pblk, lba, ppa_list[i]);
}
left_ppas -= rq_ppas;
@@ -221,10 +223,11 @@ static void pblk_recov_complete(struct kref *ref)
static void pblk_end_io_recov(struct nvm_rq *rqd)
{
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
struct pblk_pad_rq *pad_rq = rqd->private;
struct pblk *pblk = pad_rq->pblk;
- pblk_up_page(pblk, rqd->ppa_list, rqd->nr_ppas);
+ pblk_up_page(pblk, ppa_list, rqd->nr_ppas);
pblk_free_rqd(pblk, rqd, PBLK_WRITE_INT);
diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index c23b65aaa27b..1bc879ab679d 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -208,15 +208,10 @@ static void pblk_submit_rec(struct work_struct *work)
struct pblk *pblk = recovery->pblk;
struct nvm_rq *rqd = recovery->rqd;
struct pblk_c_ctx *c_ctx = nvm_rq_to_pdu(rqd);
- struct ppa_addr *ppa_list;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
pblk_log_write_err(pblk, rqd);
- if (rqd->nr_ppas == 1)
- ppa_list = &rqd->ppa_addr;
- else
- ppa_list = rqd->ppa_list;
-
pblk_map_remaining(pblk, ppa_list);
pblk_queue_resubmit(pblk, c_ctx);
@@ -273,9 +268,10 @@ static void pblk_end_io_write_meta(struct nvm_rq *rqd)
struct pblk_g_ctx *m_ctx = nvm_rq_to_pdu(rqd);
struct pblk_line *line = m_ctx->private;
struct pblk_emeta *emeta = line->emeta;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
int sync;
- pblk_up_page(pblk, rqd->ppa_list, rqd->nr_ppas);
+ pblk_up_page(pblk, ppa_list, rqd->nr_ppas);
if (rqd->error) {
pblk_log_write_err(pblk, rqd);
@@ -375,6 +371,7 @@ int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line)
struct pblk_line_mgmt *l_mg = &pblk->l_mg;
struct pblk_line_meta *lm = &pblk->lm;
struct pblk_emeta *emeta = meta_line->emeta;
+ struct ppa_addr *ppa_list;
struct pblk_g_ctx *m_ctx;
struct bio *bio;
struct nvm_rq *rqd;
@@ -409,12 +406,13 @@ int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line)
if (ret)
goto fail_free_bio;
+ ppa_list = nvm_rq_to_ppa_list(rqd);
for (i = 0; i < rqd->nr_ppas; ) {
spin_lock(&meta_line->lock);
paddr = __pblk_alloc_page(pblk, meta_line, rq_ppas);
spin_unlock(&meta_line->lock);
for (j = 0; j < rq_ppas; j++, i++, paddr++)
- rqd->ppa_list[i] = addr_to_gen_ppa(pblk, paddr, id);
+ ppa_list[i] = addr_to_gen_ppa(pblk, paddr, id);
}
spin_lock(&l_mg->close_lock);
@@ -423,7 +421,7 @@ int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line)
list_del(&meta_line->list);
spin_unlock(&l_mg->close_lock);
- pblk_down_page(pblk, rqd->ppa_list, rqd->nr_ppas);
+ pblk_down_page(pblk, ppa_list, rqd->nr_ppas);
ret = pblk_submit_io(pblk, rqd);
if (ret) {
@@ -434,7 +432,7 @@ int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line)
return NVM_IO_OK;
fail_rollback:
- pblk_up_page(pblk, rqd->ppa_list, rqd->nr_ppas);
+ pblk_up_page(pblk, ppa_list, rqd->nr_ppas);
spin_lock(&l_mg->close_lock);
pblk_dealloc_page(pblk, meta_line, rq_ppas);
list_add(&meta_line->list, &meta_line->list);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index eda66fed8e3a..fd274985aa82 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -1374,9 +1374,7 @@ static inline int pblk_boundary_ppa_checks(struct nvm_tgt_dev *tgt_dev,
static inline int pblk_check_io(struct pblk *pblk, struct nvm_rq *rqd)
{
struct nvm_tgt_dev *dev = pblk->dev;
- struct ppa_addr *ppa_list;
-
- ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
+ struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
if (pblk_boundary_ppa_checks(dev, ppa_list, rqd->nr_ppas)) {
WARN_ON(1);
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 0106984400bc..c3ee729feda5 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -320,6 +320,11 @@ static inline void *nvm_rq_to_pdu(struct nvm_rq *rqdata)
return rqdata + 1;
}
+static inline struct ppa_addr *nvm_rq_to_ppa_list(struct nvm_rq *rqd)
+{
+ return (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
+}
+
enum {
NVM_BLK_ST_FREE = 0x1, /* Free block */
NVM_BLK_ST_TGT = 0x2, /* Block in use by target */
--
2.7.4
From: Hans Holmberg <[email protected]>
On 1.2-devices, the mapping-out of remaning sectors in the
failed-write's block can result in an infinite loop,
stalling the write pipeline, fix this.
Fixes: 6a3abf5beef6 ("lightnvm: pblk: rework write error recovery path")
Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-write.c | 45 ++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index 1bc879ab679d..fd67325f6e54 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -103,6 +103,41 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
pblk_rb_sync_end(&pblk->rwb, &flags);
}
+static int pblk_next_ppa_in_blk(struct nvm_geo *geo, struct ppa_addr *ppa)
+{
+ int done = 0;
+
+ if (geo->version == NVM_OCSSD_SPEC_12) {
+ int sec = ppa->g.sec;
+
+ sec++;
+ if (sec == geo->ws_min) {
+ int pg = ppa->g.pg;
+
+ sec = 0;
+ pg++;
+ if (pg == geo->num_pg) {
+ int pl = ppa->g.pl;
+
+ pg = 0;
+ pl++;
+ if (pl == geo->num_pln)
+ done = 1;
+
+ ppa->g.pl = pl;
+ }
+ ppa->g.pg = pg;
+ }
+ ppa->g.sec = sec;
+ } else {
+ ppa->m.sec++;
+ if (ppa->m.sec == geo->clba)
+ done = 1;
+ }
+
+ return done;
+}
+
/* Map remaining sectors in chunk, starting from ppa */
static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
{
@@ -125,15 +160,7 @@ static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
if (!test_and_set_bit(paddr, line->invalid_bitmap))
le32_add_cpu(line->vsc, -1);
- if (geo->version == NVM_OCSSD_SPEC_12) {
- map_ppa.ppa++;
- if (map_ppa.g.pg == geo->num_pg)
- done = 1;
- } else {
- map_ppa.m.sec++;
- if (map_ppa.m.sec == geo->clba)
- done = 1;
- }
+ done = pblk_next_ppa_in_blk(geo, &map_ppa);
}
line->w_err_gc->has_write_err = 1;
--
2.7.4
On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> From: Hans Holmberg <[email protected]>
>
> On 1.2-devices, the mapping-out of remaning sectors in the
> failed-write's block can result in an infinite loop,
> stalling the write pipeline, fix this.
>
> Fixes: 6a3abf5beef6 ("lightnvm: pblk: rework write error recovery path")
>
> Signed-off-by: Hans Holmberg <[email protected]>
> ---
> drivers/lightnvm/pblk-write.c | 45 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> index 1bc879ab679d..fd67325f6e54 100644
> --- a/drivers/lightnvm/pblk-write.c
> +++ b/drivers/lightnvm/pblk-write.c
> @@ -103,6 +103,41 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
> pblk_rb_sync_end(&pblk->rwb, &flags);
> }
>
> +static int pblk_next_ppa_in_blk(struct nvm_geo *geo, struct ppa_addr *ppa)
> +{
> + int done = 0;
Could this please be renamed to last to signify it is the last ppa in
the ppa.
> +
> + if (geo->version == NVM_OCSSD_SPEC_12) {
> + int sec = ppa->g.sec;
> +
> + sec++;
> + if (sec == geo->ws_min) {
> + int pg = ppa->g.pg;
> +
> + sec = 0;
> + pg++;
> + if (pg == geo->num_pg) {
> + int pl = ppa->g.pl;
> +
> + pg = 0;
> + pl++;
> + if (pl == geo->num_pln)
> + done = 1;
> +
> + ppa->g.pl = pl;
> + }
> + ppa->g.pg = pg;
> + }
> + ppa->g.sec = sec;
> + } else {
> + ppa->m.sec++;
> + if (ppa->m.sec == geo->clba)
> + done = 1;
> + }
> +
> + return done;
> +}
> +
Can you please move this function to core? I want to avoid introducing
more if's around 1.2/2.0 in targets.
> /* Map remaining sectors in chunk, starting from ppa */
> static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
> {
> @@ -125,15 +160,7 @@ static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
> if (!test_and_set_bit(paddr, line->invalid_bitmap))
> le32_add_cpu(line->vsc, -1);
>
> - if (geo->version == NVM_OCSSD_SPEC_12) {
> - map_ppa.ppa++;
> - if (map_ppa.g.pg == geo->num_pg)
> - done = 1;
> - } else {
> - map_ppa.m.sec++;
> - if (map_ppa.m.sec == geo->clba)
> - done = 1;
> - }
> + done = pblk_next_ppa_in_blk(geo, &map_ppa);
> }
>
> line->w_err_gc->has_write_err = 1;
>
On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> From: Hans Holmberg <[email protected]>
>
> Pblk's global caches should not be initialized every time
> a pblk instance is created, so move the creation and destruction
> of the caches to module init/exit.
>
> Also remove the global pblk lock as it is no longer needed after
> the move.
>
The original goal was to not allocate memory for the caches unless a
pblk instance was initialized. This instead uses up memory without pblk
being used, which I don't think is a good idea.
> Signed-off-by: Hans Holmberg <[email protected]>
> ---
> drivers/lightnvm/pblk-init.c | 42 ++++++++++++++++++++----------------------
> 1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 76a4a271b9cf..0be64220b5d8 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -27,7 +27,6 @@ MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
>
> static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
> *pblk_w_rq_cache;
> -static DECLARE_RWSEM(pblk_lock);
> struct bio_set pblk_bio_set;
>
> static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
> @@ -306,21 +305,17 @@ static int pblk_set_addrf(struct pblk *pblk)
> return 0;
> }
>
> -static int pblk_init_global_caches(struct pblk *pblk)
> +static int pblk_init_global_caches(void)
> {
> - down_write(&pblk_lock);
> pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
> sizeof(struct pblk_line_ws), 0, 0, NULL);
> - if (!pblk_ws_cache) {
> - up_write(&pblk_lock);
> + if (!pblk_ws_cache)
> return -ENOMEM;
> - }
>
> pblk_rec_cache = kmem_cache_create("pblk_rec",
> sizeof(struct pblk_rec_ctx), 0, 0, NULL);
> if (!pblk_rec_cache) {
> kmem_cache_destroy(pblk_ws_cache);
> - up_write(&pblk_lock);
> return -ENOMEM;
> }
>
> @@ -329,7 +324,6 @@ static int pblk_init_global_caches(struct pblk *pblk)
> if (!pblk_g_rq_cache) {
> kmem_cache_destroy(pblk_ws_cache);
> kmem_cache_destroy(pblk_rec_cache);
> - up_write(&pblk_lock);
> return -ENOMEM;
> }
>
> @@ -339,15 +333,13 @@ static int pblk_init_global_caches(struct pblk *pblk)
> kmem_cache_destroy(pblk_ws_cache);
> kmem_cache_destroy(pblk_rec_cache);
> kmem_cache_destroy(pblk_g_rq_cache);
> - up_write(&pblk_lock);
> return -ENOMEM;
> }
> - up_write(&pblk_lock);
>
> return 0;
> }
>
> -static void pblk_free_global_caches(struct pblk *pblk)
> +static void pblk_free_global_caches(void)
> {
> kmem_cache_destroy(pblk_ws_cache);
> kmem_cache_destroy(pblk_rec_cache);
> @@ -381,13 +373,10 @@ static int pblk_core_init(struct pblk *pblk)
> if (!pblk->pad_dist)
> return -ENOMEM;
>
> - if (pblk_init_global_caches(pblk))
> - goto fail_free_pad_dist;
> -
> /* Internal bios can be at most the sectors signaled by the device. */
> ret = mempool_init_page_pool(&pblk->page_bio_pool, NVM_MAX_VLBA, 0);
> if (ret)
> - goto free_global_caches;
> + goto free_pad_dist;
>
> ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
> pblk_ws_cache);
> @@ -455,9 +444,7 @@ static int pblk_core_init(struct pblk *pblk)
> mempool_exit(&pblk->gen_ws_pool);
> free_page_bio_pool:
> mempool_exit(&pblk->page_bio_pool);
> -free_global_caches:
> - pblk_free_global_caches(pblk);
> -fail_free_pad_dist:
> +free_pad_dist:
> kfree(pblk->pad_dist);
> return -ENOMEM;
> }
> @@ -480,7 +467,6 @@ static void pblk_core_free(struct pblk *pblk)
> mempool_exit(&pblk->e_rq_pool);
> mempool_exit(&pblk->w_rq_pool);
>
> - pblk_free_global_caches(pblk);
> kfree(pblk->pad_dist);
> }
>
> @@ -1067,7 +1053,6 @@ static void pblk_exit(void *private, bool graceful)
> {
> struct pblk *pblk = private;
>
> - down_write(&pblk_lock);
> pblk_gc_exit(pblk, graceful);
> pblk_tear_down(pblk, graceful);
>
> @@ -1076,7 +1061,6 @@ static void pblk_exit(void *private, bool graceful)
> #endif
>
> pblk_free(pblk);
> - up_write(&pblk_lock);
> }
>
> static sector_t pblk_capacity(void *private)
> @@ -1237,9 +1221,22 @@ static int __init pblk_module_init(void)
> ret = bioset_init(&pblk_bio_set, BIO_POOL_SIZE, 0, 0);
> if (ret)
> return ret;
> +
> ret = nvm_register_tgt_type(&tt_pblk);
> if (ret)
> - bioset_exit(&pblk_bio_set);
> + goto fail_exit_bioset;
> +
> + ret = pblk_init_global_caches();
> + if (ret)
> + goto fail_unregister_tgt_type;
> +
> + return 0;
> +
> +fail_unregister_tgt_type:
> + nvm_unregister_tgt_type(&tt_pblk);
> +fail_exit_bioset:
> + bioset_exit(&pblk_bio_set);
> +
> return ret;
> }
>
> @@ -1247,6 +1244,7 @@ static void pblk_module_exit(void)
> {
> bioset_exit(&pblk_bio_set);
> nvm_unregister_tgt_type(&tt_pblk);
> + pblk_free_global_caches();
> }
>
> module_init(pblk_module_init);
>
On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> From: Hans Holmberg <[email protected]>
>
> This patchset contains a bunch of cleanups and bugfixes for 4.20.
> The patches depend on some of Javier's for-4.20 patches.
>
> For ordering, see:
>
> Branch: for-4.20/pblk
> Remote: ssh://github.com/OpenChannelSSD/linux
>
> Hans Holmberg (7):
> lightnvm: introduce nvm_rq_to_ppa_list
> lightnvm: pblk: fix mapping issue on failed writes
> lightnvm: pblk: allocate line map bitmaps using a mempool
> lightnvm: pblk: move global caches to module init/exit
> lightnvm: pblk: remove unused parameters in pblk_up_rq
> lightnvm: pblk: fix up prints in pblk_read_check_rand
> lightnvm: pblk: fix write amplificiation calculation
>
> drivers/lightnvm/core.c | 14 +++------
> drivers/lightnvm/pblk-core.c | 25 +++++++++------
> drivers/lightnvm/pblk-init.c | 60 ++++++++++++++++++++++-------------
> drivers/lightnvm/pblk-map.c | 10 +++---
> drivers/lightnvm/pblk-read.c | 13 +++-----
> drivers/lightnvm/pblk-recovery.c | 11 ++++---
> drivers/lightnvm/pblk-sysfs.c | 3 +-
> drivers/lightnvm/pblk-write.c | 68 +++++++++++++++++++++++++++-------------
> drivers/lightnvm/pblk.h | 11 ++++---
> include/linux/lightnvm.h | 5 +++
> 10 files changed, 134 insertions(+), 86 deletions(-)
>
Thanks. I've picked up 1,3,5-7 for 4.20.
On Wed, Aug 29, 2018 at 3:23 PM Matias Bjørling <[email protected]> wrote:
>
> On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> > From: Hans Holmberg <[email protected]>
> >
> > On 1.2-devices, the mapping-out of remaning sectors in the
> > failed-write's block can result in an infinite loop,
> > stalling the write pipeline, fix this.
> >
> > Fixes: 6a3abf5beef6 ("lightnvm: pblk: rework write error recovery path")
> >
> > Signed-off-by: Hans Holmberg <[email protected]>
> > ---
> > drivers/lightnvm/pblk-write.c | 45 ++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 36 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> > index 1bc879ab679d..fd67325f6e54 100644
> > --- a/drivers/lightnvm/pblk-write.c
> > +++ b/drivers/lightnvm/pblk-write.c
> > @@ -103,6 +103,41 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
> > pblk_rb_sync_end(&pblk->rwb, &flags);
> > }
> >
> > +static int pblk_next_ppa_in_blk(struct nvm_geo *geo, struct ppa_addr *ppa)
> > +{
> > + int done = 0;
>
> Could this please be renamed to last to signify it is the last ppa in
> the ppa.
Yes, that would be better.
>
> > +
> > + if (geo->version == NVM_OCSSD_SPEC_12) {
> > + int sec = ppa->g.sec;
> > +
> > + sec++;
> > + if (sec == geo->ws_min) {
> > + int pg = ppa->g.pg;
> > +
> > + sec = 0;
> > + pg++;
> > + if (pg == geo->num_pg) {
> > + int pl = ppa->g.pl;
> > +
> > + pg = 0;
> > + pl++;
> > + if (pl == geo->num_pln)
> > + done = 1;
> > +
> > + ppa->g.pl = pl;
> > + }
> > + ppa->g.pg = pg;
> > + }
> > + ppa->g.sec = sec;
> > + } else {
> > + ppa->m.sec++;
> > + if (ppa->m.sec == geo->clba)
> > + done = 1;
> > + }
> > +
> > + return done;
> > +}
> > +
>
> Can you please move this function to core? I want to avoid introducing
> more if's around 1.2/2.0 in targets.
That makes sense, i'll post a V2. Thanks for the review.
>
> > /* Map remaining sectors in chunk, starting from ppa */
> > static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
> > {
> > @@ -125,15 +160,7 @@ static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
> > if (!test_and_set_bit(paddr, line->invalid_bitmap))
> > le32_add_cpu(line->vsc, -1);
> >
> > - if (geo->version == NVM_OCSSD_SPEC_12) {
> > - map_ppa.ppa++;
> > - if (map_ppa.g.pg == geo->num_pg)
> > - done = 1;
> > - } else {
> > - map_ppa.m.sec++;
> > - if (map_ppa.m.sec == geo->clba)
> > - done = 1;
> > - }
> > + done = pblk_next_ppa_in_blk(geo, &map_ppa);
> > }
> >
> > line->w_err_gc->has_write_err = 1;
> >
>
On Wed, Aug 29, 2018 at 3:29 PM Matias Bjørling <[email protected]> wrote:
>
> On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> > From: Hans Holmberg <[email protected]>
> >
> > Pblk's global caches should not be initialized every time
> > a pblk instance is created, so move the creation and destruction
> > of the caches to module init/exit.
> >
> > Also remove the global pblk lock as it is no longer needed after
> > the move.
> >
>
> The original goal was to not allocate memory for the caches unless a
> pblk instance was initialized. This instead uses up memory without pblk
> being used, which I don't think is a good idea.
You're right, i'll rework the patch with reference counting of pblk instances.
The global caches only need to exist when there is at least one pblk instance.
Thanks,
Hans
>
> > Signed-off-by: Hans Holmberg <[email protected]>
> > ---
> > drivers/lightnvm/pblk-init.c | 42 ++++++++++++++++++++----------------------
> > 1 file changed, 20 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> > index 76a4a271b9cf..0be64220b5d8 100644
> > --- a/drivers/lightnvm/pblk-init.c
> > +++ b/drivers/lightnvm/pblk-init.c
> > @@ -27,7 +27,6 @@ MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
> >
> > static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
> > *pblk_w_rq_cache;
> > -static DECLARE_RWSEM(pblk_lock);
> > struct bio_set pblk_bio_set;
> >
> > static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
> > @@ -306,21 +305,17 @@ static int pblk_set_addrf(struct pblk *pblk)
> > return 0;
> > }
> >
> > -static int pblk_init_global_caches(struct pblk *pblk)
> > +static int pblk_init_global_caches(void)
> > {
> > - down_write(&pblk_lock);
> > pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
> > sizeof(struct pblk_line_ws), 0, 0, NULL);
> > - if (!pblk_ws_cache) {
> > - up_write(&pblk_lock);
> > + if (!pblk_ws_cache)
> > return -ENOMEM;
> > - }
> >
> > pblk_rec_cache = kmem_cache_create("pblk_rec",
> > sizeof(struct pblk_rec_ctx), 0, 0, NULL);
> > if (!pblk_rec_cache) {
> > kmem_cache_destroy(pblk_ws_cache);
> > - up_write(&pblk_lock);
> > return -ENOMEM;
> > }
> >
> > @@ -329,7 +324,6 @@ static int pblk_init_global_caches(struct pblk *pblk)
> > if (!pblk_g_rq_cache) {
> > kmem_cache_destroy(pblk_ws_cache);
> > kmem_cache_destroy(pblk_rec_cache);
> > - up_write(&pblk_lock);
> > return -ENOMEM;
> > }
> >
> > @@ -339,15 +333,13 @@ static int pblk_init_global_caches(struct pblk *pblk)
> > kmem_cache_destroy(pblk_ws_cache);
> > kmem_cache_destroy(pblk_rec_cache);
> > kmem_cache_destroy(pblk_g_rq_cache);
> > - up_write(&pblk_lock);
> > return -ENOMEM;
> > }
> > - up_write(&pblk_lock);
> >
> > return 0;
> > }
> >
> > -static void pblk_free_global_caches(struct pblk *pblk)
> > +static void pblk_free_global_caches(void)
> > {
> > kmem_cache_destroy(pblk_ws_cache);
> > kmem_cache_destroy(pblk_rec_cache);
> > @@ -381,13 +373,10 @@ static int pblk_core_init(struct pblk *pblk)
> > if (!pblk->pad_dist)
> > return -ENOMEM;
> >
> > - if (pblk_init_global_caches(pblk))
> > - goto fail_free_pad_dist;
> > -
> > /* Internal bios can be at most the sectors signaled by the device. */
> > ret = mempool_init_page_pool(&pblk->page_bio_pool, NVM_MAX_VLBA, 0);
> > if (ret)
> > - goto free_global_caches;
> > + goto free_pad_dist;
> >
> > ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
> > pblk_ws_cache);
> > @@ -455,9 +444,7 @@ static int pblk_core_init(struct pblk *pblk)
> > mempool_exit(&pblk->gen_ws_pool);
> > free_page_bio_pool:
> > mempool_exit(&pblk->page_bio_pool);
> > -free_global_caches:
> > - pblk_free_global_caches(pblk);
> > -fail_free_pad_dist:
> > +free_pad_dist:
> > kfree(pblk->pad_dist);
> > return -ENOMEM;
> > }
> > @@ -480,7 +467,6 @@ static void pblk_core_free(struct pblk *pblk)
> > mempool_exit(&pblk->e_rq_pool);
> > mempool_exit(&pblk->w_rq_pool);
> >
> > - pblk_free_global_caches(pblk);
> > kfree(pblk->pad_dist);
> > }
> >
> > @@ -1067,7 +1053,6 @@ static void pblk_exit(void *private, bool graceful)
> > {
> > struct pblk *pblk = private;
> >
> > - down_write(&pblk_lock);
> > pblk_gc_exit(pblk, graceful);
> > pblk_tear_down(pblk, graceful);
> >
> > @@ -1076,7 +1061,6 @@ static void pblk_exit(void *private, bool graceful)
> > #endif
> >
> > pblk_free(pblk);
> > - up_write(&pblk_lock);
> > }
> >
> > static sector_t pblk_capacity(void *private)
> > @@ -1237,9 +1221,22 @@ static int __init pblk_module_init(void)
> > ret = bioset_init(&pblk_bio_set, BIO_POOL_SIZE, 0, 0);
> > if (ret)
> > return ret;
> > +
> > ret = nvm_register_tgt_type(&tt_pblk);
> > if (ret)
> > - bioset_exit(&pblk_bio_set);
> > + goto fail_exit_bioset;
> > +
> > + ret = pblk_init_global_caches();
> > + if (ret)
> > + goto fail_unregister_tgt_type;
> > +
> > + return 0;
> > +
> > +fail_unregister_tgt_type:
> > + nvm_unregister_tgt_type(&tt_pblk);
> > +fail_exit_bioset:
> > + bioset_exit(&pblk_bio_set);
> > +
> > return ret;
> > }
> >
> > @@ -1247,6 +1244,7 @@ static void pblk_module_exit(void)
> > {
> > bioset_exit(&pblk_bio_set);
> > nvm_unregister_tgt_type(&tt_pblk);
> > + pblk_free_global_caches();
> > }
> >
> > module_init(pblk_module_init);
> >
>
Just sent a new patch ("lightnvm: pblk: stop recreating global
caches") which does reference counting.
On Wed, Aug 29, 2018 at 5:06 PM Hans Holmberg
<[email protected]> wrote:
>
> On Wed, Aug 29, 2018 at 3:29 PM Matias Bjørling <[email protected]> wrote:
> >
> > On 08/29/2018 02:23 PM, Hans Holmberg wrote:
> > > From: Hans Holmberg <[email protected]>
> > >
> > > Pblk's global caches should not be initialized every time
> > > a pblk instance is created, so move the creation and destruction
> > > of the caches to module init/exit.
> > >
> > > Also remove the global pblk lock as it is no longer needed after
> > > the move.
> > >
> >
> > The original goal was to not allocate memory for the caches unless a
> > pblk instance was initialized. This instead uses up memory without pblk
> > being used, which I don't think is a good idea.
>
> You're right, i'll rework the patch with reference counting of pblk instances.
> The global caches only need to exist when there is at least one pblk instance.
>
> Thanks,
> Hans
>
> >
> > > Signed-off-by: Hans Holmberg <[email protected]>
> > > ---
> > > drivers/lightnvm/pblk-init.c | 42 ++++++++++++++++++++----------------------
> > > 1 file changed, 20 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> > > index 76a4a271b9cf..0be64220b5d8 100644
> > > --- a/drivers/lightnvm/pblk-init.c
> > > +++ b/drivers/lightnvm/pblk-init.c
> > > @@ -27,7 +27,6 @@ MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
> > >
> > > static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
> > > *pblk_w_rq_cache;
> > > -static DECLARE_RWSEM(pblk_lock);
> > > struct bio_set pblk_bio_set;
> > >
> > > static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
> > > @@ -306,21 +305,17 @@ static int pblk_set_addrf(struct pblk *pblk)
> > > return 0;
> > > }
> > >
> > > -static int pblk_init_global_caches(struct pblk *pblk)
> > > +static int pblk_init_global_caches(void)
> > > {
> > > - down_write(&pblk_lock);
> > > pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
> > > sizeof(struct pblk_line_ws), 0, 0, NULL);
> > > - if (!pblk_ws_cache) {
> > > - up_write(&pblk_lock);
> > > + if (!pblk_ws_cache)
> > > return -ENOMEM;
> > > - }
> > >
> > > pblk_rec_cache = kmem_cache_create("pblk_rec",
> > > sizeof(struct pblk_rec_ctx), 0, 0, NULL);
> > > if (!pblk_rec_cache) {
> > > kmem_cache_destroy(pblk_ws_cache);
> > > - up_write(&pblk_lock);
> > > return -ENOMEM;
> > > }
> > >
> > > @@ -329,7 +324,6 @@ static int pblk_init_global_caches(struct pblk *pblk)
> > > if (!pblk_g_rq_cache) {
> > > kmem_cache_destroy(pblk_ws_cache);
> > > kmem_cache_destroy(pblk_rec_cache);
> > > - up_write(&pblk_lock);
> > > return -ENOMEM;
> > > }
> > >
> > > @@ -339,15 +333,13 @@ static int pblk_init_global_caches(struct pblk *pblk)
> > > kmem_cache_destroy(pblk_ws_cache);
> > > kmem_cache_destroy(pblk_rec_cache);
> > > kmem_cache_destroy(pblk_g_rq_cache);
> > > - up_write(&pblk_lock);
> > > return -ENOMEM;
> > > }
> > > - up_write(&pblk_lock);
> > >
> > > return 0;
> > > }
> > >
> > > -static void pblk_free_global_caches(struct pblk *pblk)
> > > +static void pblk_free_global_caches(void)
> > > {
> > > kmem_cache_destroy(pblk_ws_cache);
> > > kmem_cache_destroy(pblk_rec_cache);
> > > @@ -381,13 +373,10 @@ static int pblk_core_init(struct pblk *pblk)
> > > if (!pblk->pad_dist)
> > > return -ENOMEM;
> > >
> > > - if (pblk_init_global_caches(pblk))
> > > - goto fail_free_pad_dist;
> > > -
> > > /* Internal bios can be at most the sectors signaled by the device. */
> > > ret = mempool_init_page_pool(&pblk->page_bio_pool, NVM_MAX_VLBA, 0);
> > > if (ret)
> > > - goto free_global_caches;
> > > + goto free_pad_dist;
> > >
> > > ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
> > > pblk_ws_cache);
> > > @@ -455,9 +444,7 @@ static int pblk_core_init(struct pblk *pblk)
> > > mempool_exit(&pblk->gen_ws_pool);
> > > free_page_bio_pool:
> > > mempool_exit(&pblk->page_bio_pool);
> > > -free_global_caches:
> > > - pblk_free_global_caches(pblk);
> > > -fail_free_pad_dist:
> > > +free_pad_dist:
> > > kfree(pblk->pad_dist);
> > > return -ENOMEM;
> > > }
> > > @@ -480,7 +467,6 @@ static void pblk_core_free(struct pblk *pblk)
> > > mempool_exit(&pblk->e_rq_pool);
> > > mempool_exit(&pblk->w_rq_pool);
> > >
> > > - pblk_free_global_caches(pblk);
> > > kfree(pblk->pad_dist);
> > > }
> > >
> > > @@ -1067,7 +1053,6 @@ static void pblk_exit(void *private, bool graceful)
> > > {
> > > struct pblk *pblk = private;
> > >
> > > - down_write(&pblk_lock);
> > > pblk_gc_exit(pblk, graceful);
> > > pblk_tear_down(pblk, graceful);
> > >
> > > @@ -1076,7 +1061,6 @@ static void pblk_exit(void *private, bool graceful)
> > > #endif
> > >
> > > pblk_free(pblk);
> > > - up_write(&pblk_lock);
> > > }
> > >
> > > static sector_t pblk_capacity(void *private)
> > > @@ -1237,9 +1221,22 @@ static int __init pblk_module_init(void)
> > > ret = bioset_init(&pblk_bio_set, BIO_POOL_SIZE, 0, 0);
> > > if (ret)
> > > return ret;
> > > +
> > > ret = nvm_register_tgt_type(&tt_pblk);
> > > if (ret)
> > > - bioset_exit(&pblk_bio_set);
> > > + goto fail_exit_bioset;
> > > +
> > > + ret = pblk_init_global_caches();
> > > + if (ret)
> > > + goto fail_unregister_tgt_type;
> > > +
> > > + return 0;
> > > +
> > > +fail_unregister_tgt_type:
> > > + nvm_unregister_tgt_type(&tt_pblk);
> > > +fail_exit_bioset:
> > > + bioset_exit(&pblk_bio_set);
> > > +
> > > return ret;
> > > }
> > >
> > > @@ -1247,6 +1244,7 @@ static void pblk_module_exit(void)
> > > {
> > > bioset_exit(&pblk_bio_set);
> > > nvm_unregister_tgt_type(&tt_pblk);
> > > + pblk_free_global_caches();
> > > }
> > >
> > > module_init(pblk_module_init);
> > >
> >