2019-07-31 11:00:14

by Hans Holmberg

Subject: [PATCH 0/4] lnvm/pblk mapping cleanups

This series cleans up the metadata allocation/mapping in lnvm/pblk
by moving over to kvmalloc for metadata and moving metadata mapping
down to the lower lever driver where blk_rq_map_kern can be used.

Hans Holmberg (4):
lightnvm: remove nvm_submit_io_sync_fn
lightnvm: move metadata mapping to lower level driver
lightnvm: pblk: use kvmalloc for metadata
block: stop exporting bio_map_kern

block/bio.c | 1 -
drivers/lightnvm/core.c | 43 ++++++++++++---
drivers/lightnvm/pblk-core.c | 116 +++++----------------------------------
drivers/lightnvm/pblk-gc.c | 19 +++----
drivers/lightnvm/pblk-init.c | 38 ++++---------
drivers/lightnvm/pblk-read.c | 22 +-------
drivers/lightnvm/pblk-recovery.c | 39 ++-----------
drivers/lightnvm/pblk-write.c | 20 +------
drivers/lightnvm/pblk.h | 31 +----------
drivers/nvme/host/lightnvm.c | 45 +++++----------
include/linux/lightnvm.h | 8 +--
11 files changed, 96 insertions(+), 286 deletions(-)

--
2.7.4

2019-07-31 11:00:34

by Hans Holmberg

Subject: [PATCH 1/4] lightnvm: remove nvm_submit_io_sync_fn

Move the redundant sync handling interface and wait for a completion
in the lightnvm core instead.

Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/core.c | 35 +++++++++++++++++++++++++++++------
drivers/nvme/host/lightnvm.c | 29 -----------------------------
include/linux/lightnvm.h | 2 --
3 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index a600934fdd9c..01d098fb96ac 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -752,12 +752,36 @@ int nvm_submit_io(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
}
EXPORT_SYMBOL(nvm_submit_io);

+static void nvm_sync_end_io(struct nvm_rq *rqd)
+{
+ struct completion *waiting = rqd->private;
+
+ complete(waiting);
+}
+
+static int nvm_submit_io_wait(struct nvm_dev *dev, struct nvm_rq *rqd)
+{
+ DECLARE_COMPLETION_ONSTACK(wait);
+ int ret = 0;
+
+ rqd->end_io = nvm_sync_end_io;
+ rqd->private = &wait;
+
+ ret = dev->ops->submit_io(dev, rqd);
+ if (ret)
+ return ret;
+
+ wait_for_completion_io(&wait);
+
+ return 0;
+}
+
int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
{
struct nvm_dev *dev = tgt_dev->parent;
int ret;

- if (!dev->ops->submit_io_sync)
+ if (!dev->ops->submit_io)
return -ENODEV;

nvm_rq_tgt_to_dev(tgt_dev, rqd);
@@ -765,9 +789,7 @@ int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
rqd->dev = tgt_dev;
rqd->flags = nvm_set_flags(&tgt_dev->geo, rqd);

- /* In case of error, fail with right address format */
- ret = dev->ops->submit_io_sync(dev, rqd);
- nvm_rq_dev_to_tgt(tgt_dev, rqd);
+ ret = nvm_submit_io_wait(dev, rqd);

return ret;
}
@@ -788,12 +810,13 @@ EXPORT_SYMBOL(nvm_end_io);

static int nvm_submit_io_sync_raw(struct nvm_dev *dev, struct nvm_rq *rqd)
{
- if (!dev->ops->submit_io_sync)
+ if (!dev->ops->submit_io)
return -ENODEV;

+ rqd->dev = NULL;
rqd->flags = nvm_set_flags(&dev->geo, rqd);

- return dev->ops->submit_io_sync(dev, rqd);
+ return nvm_submit_io_wait(dev, rqd);
}

static int nvm_bb_chunk_sense(struct nvm_dev *dev, struct ppa_addr ppa)
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index ba009d4c9dfa..d6f121452d5d 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -690,34 +690,6 @@ static int nvme_nvm_submit_io(struct nvm_dev *dev, struct nvm_rq *rqd)
return 0;
}

-static int nvme_nvm_submit_io_sync(struct nvm_dev *dev, struct nvm_rq *rqd)
-{
- struct request_queue *q = dev->q;
- struct request *rq;
- struct nvme_nvm_command cmd;
- int ret = 0;
-
- memset(&cmd, 0, sizeof(struct nvme_nvm_command));
-
- rq = nvme_nvm_alloc_request(q, rqd, &cmd);
- if (IS_ERR(rq))
- return PTR_ERR(rq);
-
- /* I/Os can fail and the error is signaled through rqd. Callers must
- * handle the error accordingly.
- */
- blk_execute_rq(q, NULL, rq, 0);
- if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
- ret = -EINTR;
-
- rqd->ppa_status = le64_to_cpu(nvme_req(rq)->result.u64);
- rqd->error = nvme_req(rq)->status;
-
- blk_mq_free_request(rq);
-
- return ret;
-}
-
static void *nvme_nvm_create_dma_pool(struct nvm_dev *nvmdev, char *name,
int size)
{
@@ -754,7 +726,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
.get_chk_meta = nvme_nvm_get_chk_meta,

.submit_io = nvme_nvm_submit_io,
- .submit_io_sync = nvme_nvm_submit_io_sync,

.create_dma_pool = nvme_nvm_create_dma_pool,
.destroy_dma_pool = nvme_nvm_destroy_dma_pool,
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 4d0d5655c7b2..8891647b24b1 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -89,7 +89,6 @@ typedef int (nvm_op_set_bb_fn)(struct nvm_dev *, struct ppa_addr *, int, int);
typedef int (nvm_get_chk_meta_fn)(struct nvm_dev *, sector_t, int,
struct nvm_chk_meta *);
typedef int (nvm_submit_io_fn)(struct nvm_dev *, struct nvm_rq *);
-typedef int (nvm_submit_io_sync_fn)(struct nvm_dev *, struct nvm_rq *);
typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
typedef void (nvm_destroy_dma_pool_fn)(void *);
typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
@@ -104,7 +103,6 @@ struct nvm_dev_ops {
nvm_get_chk_meta_fn *get_chk_meta;

nvm_submit_io_fn *submit_io;
- nvm_submit_io_sync_fn *submit_io_sync;

nvm_create_dma_pool_fn *create_dma_pool;
nvm_destroy_dma_pool_fn *destroy_dma_pool;
--
2.7.4

2019-07-31 11:01:41

by Hans Holmberg

Subject: [PATCH 3/4] lightnvm: pblk: use kvmalloc for metadata

There is no reason now not to use kvmalloc, so
so replace the internal metadata allocation scheme.

Signed-off-by: Hans Holmberg <[email protected]>
---
drivers/lightnvm/pblk-core.c | 3 +--
drivers/lightnvm/pblk-gc.c | 19 ++++++++-----------
drivers/lightnvm/pblk-init.c | 38 ++++++++++----------------------------
drivers/lightnvm/pblk.h | 23 -----------------------
4 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index a58d3c84a3f2..b413bafe93fd 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1839,8 +1839,7 @@ static void pblk_save_lba_list(struct pblk *pblk, struct pblk_line *line)
struct pblk_w_err_gc *w_err_gc = line->w_err_gc;
struct pblk_emeta *emeta = line->emeta;

- w_err_gc->lba_list = pblk_malloc(lba_list_size,
- l_mg->emeta_alloc_type, GFP_KERNEL);
+ w_err_gc->lba_list = kvmalloc(lba_list_size, GFP_KERNEL);
memcpy(w_err_gc->lba_list, emeta_to_lbas(pblk, emeta->buf),
lba_list_size);
}
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 63ee205b41c4..2581eebcfc41 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -132,14 +132,12 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
struct pblk_line *line)
{
struct line_emeta *emeta_buf;
- struct pblk_line_mgmt *l_mg = &pblk->l_mg;
struct pblk_line_meta *lm = &pblk->lm;
unsigned int lba_list_size = lm->emeta_len[2];
__le64 *lba_list;
int ret;

- emeta_buf = pblk_malloc(lm->emeta_len[0],
- l_mg->emeta_alloc_type, GFP_KERNEL);
+ emeta_buf = kvmalloc(lm->emeta_len[0], GFP_KERNEL);
if (!emeta_buf)
return NULL;

@@ -147,7 +145,7 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
if (ret) {
pblk_err(pblk, "line %d read emeta failed (%d)\n",
line->id, ret);
- pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
+ kvfree(emeta_buf);
return NULL;
}

@@ -161,16 +159,16 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
if (ret) {
pblk_err(pblk, "inconsistent emeta (line %d)\n",
line->id);
- pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
+ kvfree(emeta_buf);
return NULL;
}

- lba_list = pblk_malloc(lba_list_size,
- l_mg->emeta_alloc_type, GFP_KERNEL);
+ lba_list = kvmalloc(lba_list_size, GFP_KERNEL);
+
if (lba_list)
memcpy(lba_list, emeta_to_lbas(pblk, emeta_buf), lba_list_size);

- pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
+ kvfree(emeta_buf);

return lba_list;
}
@@ -181,7 +179,6 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
ws);
struct pblk *pblk = line_ws->pblk;
struct pblk_line *line = line_ws->line;
- struct pblk_line_mgmt *l_mg = &pblk->l_mg;
struct pblk_line_meta *lm = &pblk->lm;
struct nvm_tgt_dev *dev = pblk->dev;
struct nvm_geo *geo = &dev->geo;
@@ -272,7 +269,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
goto next_rq;

out:
- pblk_mfree(lba_list, l_mg->emeta_alloc_type);
+ kvfree(lba_list);
kfree(line_ws);
kfree(invalid_bitmap);

@@ -286,7 +283,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
fail_free_gc_rq:
kfree(gc_rq);
fail_free_lba_list:
- pblk_mfree(lba_list, l_mg->emeta_alloc_type);
+ kvfree(lba_list);
fail_free_invalid_bitmap:
kfree(invalid_bitmap);
fail_free_ws:
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index b351c7f002de..9a967a2e83dd 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -543,7 +543,7 @@ static void pblk_line_mg_free(struct pblk *pblk)

for (i = 0; i < PBLK_DATA_LINES; i++) {
kfree(l_mg->sline_meta[i]);
- pblk_mfree(l_mg->eline_meta[i]->buf, l_mg->emeta_alloc_type);
+ kvfree(l_mg->eline_meta[i]->buf);
kfree(l_mg->eline_meta[i]);
}

@@ -560,7 +560,7 @@ static void pblk_line_meta_free(struct pblk_line_mgmt *l_mg,
kfree(line->erase_bitmap);
kfree(line->chks);

- pblk_mfree(w_err_gc->lba_list, l_mg->emeta_alloc_type);
+ kvfree(w_err_gc->lba_list);
kfree(w_err_gc);
}

@@ -890,29 +890,14 @@ static int pblk_line_mg_init(struct pblk *pblk)
if (!emeta)
goto fail_free_emeta;

- if (lm->emeta_len[0] > KMALLOC_MAX_CACHE_SIZE) {
- l_mg->emeta_alloc_type = PBLK_VMALLOC_META;
-
- emeta->buf = vmalloc(lm->emeta_len[0]);
- if (!emeta->buf) {
- kfree(emeta);
- goto fail_free_emeta;
- }
-
- emeta->nr_entries = lm->emeta_sec[0];
- l_mg->eline_meta[i] = emeta;
- } else {
- l_mg->emeta_alloc_type = PBLK_KMALLOC_META;
-
- emeta->buf = kmalloc(lm->emeta_len[0], GFP_KERNEL);
- if (!emeta->buf) {
- kfree(emeta);
- goto fail_free_emeta;
- }
-
- emeta->nr_entries = lm->emeta_sec[0];
- l_mg->eline_meta[i] = emeta;
+ emeta->buf = kvmalloc(lm->emeta_len[0], GFP_KERNEL);
+ if (!emeta->buf) {
+ kfree(emeta);
+ goto fail_free_emeta;
}
+
+ emeta->nr_entries = lm->emeta_sec[0];
+ l_mg->eline_meta[i] = emeta;
}

for (i = 0; i < l_mg->nr_lines; i++)
@@ -926,10 +911,7 @@ static int pblk_line_mg_init(struct pblk *pblk)

fail_free_emeta:
while (--i >= 0) {
- if (l_mg->emeta_alloc_type == PBLK_VMALLOC_META)
- vfree(l_mg->eline_meta[i]->buf);
- else
- kfree(l_mg->eline_meta[i]->buf);
+ kvfree(l_mg->eline_meta[i]->buf);
kfree(l_mg->eline_meta[i]);
}

diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index d515d3409a74..86ffa875bfe1 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -482,11 +482,6 @@ struct pblk_line {
#define PBLK_DATA_LINES 4

enum {
- PBLK_KMALLOC_META = 1,
- PBLK_VMALLOC_META = 2,
-};
-
-enum {
PBLK_EMETA_TYPE_HEADER = 1, /* struct line_emeta first sector */
PBLK_EMETA_TYPE_LLBA = 2, /* lba list - type: __le64 */
PBLK_EMETA_TYPE_VSC = 3, /* vsc list - type: __le32 */
@@ -521,9 +516,6 @@ struct pblk_line_mgmt {

__le32 *vsc_list; /* Valid sector counts for all lines */

- /* Metadata allocation type: VMALLOC | KMALLOC */
- int emeta_alloc_type;
-
/* Pre-allocated metadata for data lines */
struct pblk_smeta *sline_meta[PBLK_DATA_LINES];
struct pblk_emeta *eline_meta[PBLK_DATA_LINES];
@@ -934,21 +926,6 @@ void pblk_rl_werr_line_out(struct pblk_rl *rl);
int pblk_sysfs_init(struct gendisk *tdisk);
void pblk_sysfs_exit(struct gendisk *tdisk);

-static inline void *pblk_malloc(size_t size, int type, gfp_t flags)
-{
- if (type == PBLK_KMALLOC_META)
- return kmalloc(size, flags);
- return vmalloc(size);
-}
-
-static inline void pblk_mfree(void *ptr, int type)
-{
- if (type == PBLK_KMALLOC_META)
- kfree(ptr);
- else
- vfree(ptr);
-}
-
static inline struct nvm_rq *nvm_rq_from_c_ctx(void *c_ctx)
{
return c_ctx - sizeof(struct nvm_rq);
--
2.7.4

2019-07-31 12:07:36

by Hans Holmberg

Subject: [PATCH 4/4] block: stop exporting bio_map_kern

Now that there no module users left of bio_map_kern, stop
exporting the symbol.

Signed-off-by: Hans Holmberg <[email protected]>
---
block/bio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index 299a0e7651ec..96ca0b4e73bb 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1521,7 +1521,6 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len,
bio->bi_end_io = bio_map_kern_endio;
return bio;
}
-EXPORT_SYMBOL(bio_map_kern);

static void bio_copy_kern_endio(struct bio *bio)
{
--
2.7.4

2019-07-31 15:12:12

by Javier González

Subject: Re: [PATCH 3/4] lightnvm: pblk: use kvmalloc for metadata

> On 31 Jul 2019, at 11.41, Hans Holmberg <[email protected]> wrote:
>
> There is no reason now not to use kvmalloc, so
> so replace the internal metadata allocation scheme.

2 x so

>
> Signed-off-by: Hans Holmberg <[email protected]>
> ---
> drivers/lightnvm/pblk-core.c | 3 +--
> drivers/lightnvm/pblk-gc.c | 19 ++++++++-----------
> drivers/lightnvm/pblk-init.c | 38 ++++++++++----------------------------
> drivers/lightnvm/pblk.h | 23 -----------------------
> 4 files changed, 19 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index a58d3c84a3f2..b413bafe93fd 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1839,8 +1839,7 @@ static void pblk_save_lba_list(struct pblk *pblk, struct pblk_line *line)
> struct pblk_w_err_gc *w_err_gc = line->w_err_gc;
> struct pblk_emeta *emeta = line->emeta;
>
> - w_err_gc->lba_list = pblk_malloc(lba_list_size,
> - l_mg->emeta_alloc_type, GFP_KERNEL);
> + w_err_gc->lba_list = kvmalloc(lba_list_size, GFP_KERNEL);
> memcpy(w_err_gc->lba_list, emeta_to_lbas(pblk, emeta->buf),
> lba_list_size);
> }
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 63ee205b41c4..2581eebcfc41 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -132,14 +132,12 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
> struct pblk_line *line)
> {
> struct line_emeta *emeta_buf;
> - struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct pblk_line_meta *lm = &pblk->lm;
> unsigned int lba_list_size = lm->emeta_len[2];
> __le64 *lba_list;
> int ret;
>
> - emeta_buf = pblk_malloc(lm->emeta_len[0],
> - l_mg->emeta_alloc_type, GFP_KERNEL);
> + emeta_buf = kvmalloc(lm->emeta_len[0], GFP_KERNEL);
> if (!emeta_buf)
> return NULL;
>
> @@ -147,7 +145,7 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
> if (ret) {
> pblk_err(pblk, "line %d read emeta failed (%d)\n",
> line->id, ret);
> - pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
> + kvfree(emeta_buf);
> return NULL;
> }
>
> @@ -161,16 +159,16 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk,
> if (ret) {
> pblk_err(pblk, "inconsistent emeta (line %d)\n",
> line->id);
> - pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
> + kvfree(emeta_buf);
> return NULL;
> }
>
> - lba_list = pblk_malloc(lba_list_size,
> - l_mg->emeta_alloc_type, GFP_KERNEL);
> + lba_list = kvmalloc(lba_list_size, GFP_KERNEL);
> +
> if (lba_list)
> memcpy(lba_list, emeta_to_lbas(pblk, emeta_buf), lba_list_size);
>
> - pblk_mfree(emeta_buf, l_mg->emeta_alloc_type);
> + kvfree(emeta_buf);
>
> return lba_list;
> }
> @@ -181,7 +179,6 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> ws);
> struct pblk *pblk = line_ws->pblk;
> struct pblk_line *line = line_ws->line;
> - struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct pblk_line_meta *lm = &pblk->lm;
> struct nvm_tgt_dev *dev = pblk->dev;
> struct nvm_geo *geo = &dev->geo;
> @@ -272,7 +269,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> goto next_rq;
>
> out:
> - pblk_mfree(lba_list, l_mg->emeta_alloc_type);
> + kvfree(lba_list);
> kfree(line_ws);
> kfree(invalid_bitmap);
>
> @@ -286,7 +283,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> fail_free_gc_rq:
> kfree(gc_rq);
> fail_free_lba_list:
> - pblk_mfree(lba_list, l_mg->emeta_alloc_type);
> + kvfree(lba_list);
> fail_free_invalid_bitmap:
> kfree(invalid_bitmap);
> fail_free_ws:
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index b351c7f002de..9a967a2e83dd 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -543,7 +543,7 @@ static void pblk_line_mg_free(struct pblk *pblk)
>
> for (i = 0; i < PBLK_DATA_LINES; i++) {
> kfree(l_mg->sline_meta[i]);
> - pblk_mfree(l_mg->eline_meta[i]->buf, l_mg->emeta_alloc_type);
> + kvfree(l_mg->eline_meta[i]->buf);
> kfree(l_mg->eline_meta[i]);
> }
>
> @@ -560,7 +560,7 @@ static void pblk_line_meta_free(struct pblk_line_mgmt *l_mg,
> kfree(line->erase_bitmap);
> kfree(line->chks);
>
> - pblk_mfree(w_err_gc->lba_list, l_mg->emeta_alloc_type);
> + kvfree(w_err_gc->lba_list);
> kfree(w_err_gc);
> }
>
> @@ -890,29 +890,14 @@ static int pblk_line_mg_init(struct pblk *pblk)
> if (!emeta)
> goto fail_free_emeta;
>
> - if (lm->emeta_len[0] > KMALLOC_MAX_CACHE_SIZE) {
> - l_mg->emeta_alloc_type = PBLK_VMALLOC_META;
> -
> - emeta->buf = vmalloc(lm->emeta_len[0]);
> - if (!emeta->buf) {
> - kfree(emeta);
> - goto fail_free_emeta;
> - }
> -
> - emeta->nr_entries = lm->emeta_sec[0];
> - l_mg->eline_meta[i] = emeta;
> - } else {
> - l_mg->emeta_alloc_type = PBLK_KMALLOC_META;
> -
> - emeta->buf = kmalloc(lm->emeta_len[0], GFP_KERNEL);
> - if (!emeta->buf) {
> - kfree(emeta);
> - goto fail_free_emeta;
> - }
> -
> - emeta->nr_entries = lm->emeta_sec[0];
> - l_mg->eline_meta[i] = emeta;
> + emeta->buf = kvmalloc(lm->emeta_len[0], GFP_KERNEL);
> + if (!emeta->buf) {
> + kfree(emeta);
> + goto fail_free_emeta;
> }
> +
> + emeta->nr_entries = lm->emeta_sec[0];
> + l_mg->eline_meta[i] = emeta;
> }
>
> for (i = 0; i < l_mg->nr_lines; i++)
> @@ -926,10 +911,7 @@ static int pblk_line_mg_init(struct pblk *pblk)
>
> fail_free_emeta:
> while (--i >= 0) {
> - if (l_mg->emeta_alloc_type == PBLK_VMALLOC_META)
> - vfree(l_mg->eline_meta[i]->buf);
> - else
> - kfree(l_mg->eline_meta[i]->buf);
> + kvfree(l_mg->eline_meta[i]->buf);
> kfree(l_mg->eline_meta[i]);
> }
>
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index d515d3409a74..86ffa875bfe1 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -482,11 +482,6 @@ struct pblk_line {
> #define PBLK_DATA_LINES 4
>
> enum {
> - PBLK_KMALLOC_META = 1,
> - PBLK_VMALLOC_META = 2,
> -};
> -
> -enum {
> PBLK_EMETA_TYPE_HEADER = 1, /* struct line_emeta first sector */
> PBLK_EMETA_TYPE_LLBA = 2, /* lba list - type: __le64 */
> PBLK_EMETA_TYPE_VSC = 3, /* vsc list - type: __le32 */
> @@ -521,9 +516,6 @@ struct pblk_line_mgmt {
>
> __le32 *vsc_list; /* Valid sector counts for all lines */
>
> - /* Metadata allocation type: VMALLOC | KMALLOC */
> - int emeta_alloc_type;
> -
> /* Pre-allocated metadata for data lines */
> struct pblk_smeta *sline_meta[PBLK_DATA_LINES];
> struct pblk_emeta *eline_meta[PBLK_DATA_LINES];
> @@ -934,21 +926,6 @@ void pblk_rl_werr_line_out(struct pblk_rl *rl);
> int pblk_sysfs_init(struct gendisk *tdisk);
> void pblk_sysfs_exit(struct gendisk *tdisk);
>
> -static inline void *pblk_malloc(size_t size, int type, gfp_t flags)
> -{
> - if (type == PBLK_KMALLOC_META)
> - return kmalloc(size, flags);
> - return vmalloc(size);
> -}
> -
> -static inline void pblk_mfree(void *ptr, int type)
> -{
> - if (type == PBLK_KMALLOC_META)
> - kfree(ptr);
> - else
> - vfree(ptr);
> -}
> -
> static inline struct nvm_rq *nvm_rq_from_c_ctx(void *c_ctx)
> {
> return c_ctx - sizeof(struct nvm_rq);
> --
> 2.7.4

Looks good to me.

Reviewed-by: Javier González <[email protected]>

Attachments:

2019-07-31 15:12:36

by Javier González

Subject: Re: [PATCH 1/4] lightnvm: remove nvm_submit_io_sync_fn

> On 31 Jul 2019, at 11.41, Hans Holmberg <[email protected]> wrote:
>
> Move the redundant sync handling interface and wait for a completion
> in the lightnvm core instead.
>
> Signed-off-by: Hans Holmberg <[email protected]>
> ---
> drivers/lightnvm/core.c | 35 +++++++++++++++++++++++++++++------
> drivers/nvme/host/lightnvm.c | 29 -----------------------------
> include/linux/lightnvm.h | 2 --
> 3 files changed, 29 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index a600934fdd9c..01d098fb96ac 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -752,12 +752,36 @@ int nvm_submit_io(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
> }
> EXPORT_SYMBOL(nvm_submit_io);
>
> +static void nvm_sync_end_io(struct nvm_rq *rqd)
> +{
> + struct completion *waiting = rqd->private;
> +
> + complete(waiting);
> +}
> +
> +static int nvm_submit_io_wait(struct nvm_dev *dev, struct nvm_rq *rqd)
> +{
> + DECLARE_COMPLETION_ONSTACK(wait);
> + int ret = 0;
> +
> + rqd->end_io = nvm_sync_end_io;
> + rqd->private = &wait;
> +
> + ret = dev->ops->submit_io(dev, rqd);
> + if (ret)
> + return ret;
> +
> + wait_for_completion_io(&wait);
> +
> + return 0;
> +}
> +
> int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
> {
> struct nvm_dev *dev = tgt_dev->parent;
> int ret;
>
> - if (!dev->ops->submit_io_sync)
> + if (!dev->ops->submit_io)
> return -ENODEV;
>
> nvm_rq_tgt_to_dev(tgt_dev, rqd);
> @@ -765,9 +789,7 @@ int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
> rqd->dev = tgt_dev;
> rqd->flags = nvm_set_flags(&tgt_dev->geo, rqd);
>
> - /* In case of error, fail with right address format */
> - ret = dev->ops->submit_io_sync(dev, rqd);
> - nvm_rq_dev_to_tgt(tgt_dev, rqd);
> + ret = nvm_submit_io_wait(dev, rqd);
>
> return ret;
> }
> @@ -788,12 +810,13 @@ EXPORT_SYMBOL(nvm_end_io);
>
> static int nvm_submit_io_sync_raw(struct nvm_dev *dev, struct nvm_rq *rqd)
> {
> - if (!dev->ops->submit_io_sync)
> + if (!dev->ops->submit_io)
> return -ENODEV;
>
> + rqd->dev = NULL;
> rqd->flags = nvm_set_flags(&dev->geo, rqd);
>
> - return dev->ops->submit_io_sync(dev, rqd);
> + return nvm_submit_io_wait(dev, rqd);
> }
>
> static int nvm_bb_chunk_sense(struct nvm_dev *dev, struct ppa_addr ppa)
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index ba009d4c9dfa..d6f121452d5d 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -690,34 +690,6 @@ static int nvme_nvm_submit_io(struct nvm_dev *dev, struct nvm_rq *rqd)
> return 0;
> }
>
> -static int nvme_nvm_submit_io_sync(struct nvm_dev *dev, struct nvm_rq *rqd)
> -{
> - struct request_queue *q = dev->q;
> - struct request *rq;
> - struct nvme_nvm_command cmd;
> - int ret = 0;
> -
> - memset(&cmd, 0, sizeof(struct nvme_nvm_command));
> -
> - rq = nvme_nvm_alloc_request(q, rqd, &cmd);
> - if (IS_ERR(rq))
> - return PTR_ERR(rq);
> -
> - /* I/Os can fail and the error is signaled through rqd. Callers must
> - * handle the error accordingly.
> - */
> - blk_execute_rq(q, NULL, rq, 0);
> - if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
> - ret = -EINTR;
> -
> - rqd->ppa_status = le64_to_cpu(nvme_req(rq)->result.u64);
> - rqd->error = nvme_req(rq)->status;
> -
> - blk_mq_free_request(rq);
> -
> - return ret;
> -}
> -
> static void *nvme_nvm_create_dma_pool(struct nvm_dev *nvmdev, char *name,
> int size)
> {
> @@ -754,7 +726,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
> .get_chk_meta = nvme_nvm_get_chk_meta,
>
> .submit_io = nvme_nvm_submit_io,
> - .submit_io_sync = nvme_nvm_submit_io_sync,
>
> .create_dma_pool = nvme_nvm_create_dma_pool,
> .destroy_dma_pool = nvme_nvm_destroy_dma_pool,
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 4d0d5655c7b2..8891647b24b1 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -89,7 +89,6 @@ typedef int (nvm_op_set_bb_fn)(struct nvm_dev *, struct ppa_addr *, int, int);
> typedef int (nvm_get_chk_meta_fn)(struct nvm_dev *, sector_t, int,
> struct nvm_chk_meta *);
> typedef int (nvm_submit_io_fn)(struct nvm_dev *, struct nvm_rq *);
> -typedef int (nvm_submit_io_sync_fn)(struct nvm_dev *, struct nvm_rq *);
> typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
> typedef void (nvm_destroy_dma_pool_fn)(void *);
> typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
> @@ -104,7 +103,6 @@ struct nvm_dev_ops {
> nvm_get_chk_meta_fn *get_chk_meta;
>
> nvm_submit_io_fn *submit_io;
> - nvm_submit_io_sync_fn *submit_io_sync;
>
> nvm_create_dma_pool_fn *create_dma_pool;
> nvm_destroy_dma_pool_fn *destroy_dma_pool;
> --
> 2.7.4

Looks good to me. Thanks Hans.

Reviewed-by: Javier González <[email protected]>

Attachments:

2019-07-31 17:53:03

by Javier González

Subject: Re: [PATCH 4/4] block: stop exporting bio_map_kern

> On 31 Jul 2019, at 11.41, Hans Holmberg <[email protected]> wrote:
>
> Now that there no module users left of bio_map_kern, stop
> exporting the symbol.
>
> Signed-off-by: Hans Holmberg <[email protected]>
> ---
> block/bio.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 299a0e7651ec..96ca0b4e73bb 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1521,7 +1521,6 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len,
> bio->bi_end_io = bio_map_kern_endio;
> return bio;
> }
> -EXPORT_SYMBOL(bio_map_kern);
>
> static void bio_copy_kern_endio(struct bio *bio)
> {
> --
> 2.7.4

Haven’t realized we were the only users at this point. Nice cleanup.

Reviewed-by: Javier González <[email protected]>

Attachments:

2019-08-06 07:08:24

by Christoph Hellwig

Subject: Re: [PATCH 1/4] lightnvm: remove nvm_submit_io_sync_fn

On Wed, Jul 31, 2019 at 11:41:33AM +0200, Hans Holmberg wrote:
> Move the redundant sync handling interface and wait for a completion
> in the lightnvm core instead.
>
> Signed-off-by: Hans Holmberg <[email protected]>

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2019-08-06 07:09:14

by Christoph Hellwig

Subject: Re: [PATCH 3/4] lightnvm: pblk: use kvmalloc for metadata

On Wed, Jul 31, 2019 at 11:41:35AM +0200, Hans Holmberg wrote:
> There is no reason now not to use kvmalloc, so
> so replace the internal metadata allocation scheme.

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2019-08-06 07:09:31

by Christoph Hellwig

Subject: Re: [PATCH 4/4] block: stop exporting bio_map_kern

On Wed, Jul 31, 2019 at 11:41:36AM +0200, Hans Holmberg wrote:
> Now that there no module users left of bio_map_kern, stop
> exporting the symbol.
>
> Signed-off-by: Hans Holmberg <[email protected]>

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2019-08-06 13:41:48

by Matias Bjørling

Subject: Re: [PATCH 0/4] lnvm/pblk mapping cleanups

On 7/31/19 11:41 AM, Hans Holmberg wrote:
> This series cleans up the metadata allocation/mapping in lnvm/pblk
> by moving over to kvmalloc for metadata and moving metadata mapping
> down to the lower lever driver where blk_rq_map_kern can be used.
>
> Hans Holmberg (4):
> lightnvm: remove nvm_submit_io_sync_fn
> lightnvm: move metadata mapping to lower level driver
> lightnvm: pblk: use kvmalloc for metadata
> block: stop exporting bio_map_kern
>
> block/bio.c | 1 -
> drivers/lightnvm/core.c | 43 ++++++++++++---
> drivers/lightnvm/pblk-core.c | 116 +++++----------------------------------
> drivers/lightnvm/pblk-gc.c | 19 +++----
> drivers/lightnvm/pblk-init.c | 38 ++++---------
> drivers/lightnvm/pblk-read.c | 22 +-------
> drivers/lightnvm/pblk-recovery.c | 39 ++-----------
> drivers/lightnvm/pblk-write.c | 20 +------
> drivers/lightnvm/pblk.h | 31 +----------
> drivers/nvme/host/lightnvm.c | 45 +++++----------
> include/linux/lightnvm.h | 8 +--
> 11 files changed, 96 insertions(+), 286 deletions(-)
>

Hi Jens,

Would you like me to pick up this serie, and send it through the
lightnvm pull request, or would you like to pick it up?

Thank you!
Matias

2019-08-06 14:18:57

by Jens Axboe

Subject: Re: [PATCH 0/4] lnvm/pblk mapping cleanups

On 8/6/19 6:40 AM, Matias Bjørling wrote:
> On 7/31/19 11:41 AM, Hans Holmberg wrote:
>> This series cleans up the metadata allocation/mapping in lnvm/pblk
>> by moving over to kvmalloc for metadata and moving metadata mapping
>> down to the lower lever driver where blk_rq_map_kern can be used.
>>
>> Hans Holmberg (4):
>> lightnvm: remove nvm_submit_io_sync_fn
>> lightnvm: move metadata mapping to lower level driver
>> lightnvm: pblk: use kvmalloc for metadata
>> block: stop exporting bio_map_kern
>>
>> block/bio.c | 1 -
>> drivers/lightnvm/core.c | 43 ++++++++++++---
>> drivers/lightnvm/pblk-core.c | 116 +++++----------------------------------
>> drivers/lightnvm/pblk-gc.c | 19 +++----
>> drivers/lightnvm/pblk-init.c | 38 ++++---------
>> drivers/lightnvm/pblk-read.c | 22 +-------
>> drivers/lightnvm/pblk-recovery.c | 39 ++-----------
>> drivers/lightnvm/pblk-write.c | 20 +------
>> drivers/lightnvm/pblk.h | 31 +----------
>> drivers/nvme/host/lightnvm.c | 45 +++++----------
>> include/linux/lightnvm.h | 8 +--
>> 11 files changed, 96 insertions(+), 286 deletions(-)
>>
>
> Hi Jens,
>
> Would you like me to pick up this serie, and send it through the
> lightnvm pull request, or would you like to pick it up?

I can apply it directly for 5.4.

--
Jens Axboe