> On 16 Jun 2018, at 00.27, Igor Konopko <igor.j.konopko@xxxxxxxxx> wrote:
>
> In current pblk implementation, l2p mapping for not closed lines
> is always stored only in OOB metadata and recovered from it.
>
> Such a solution does not provide data integrity when drives does
> not have such a OOB metadata space.
>
> The goal of this patch is to add support for so called packed
> metadata, which store l2p mapping for open lines in last sector
> of every write unit.
>
> Signed-off-by: Igor Konopko <igor.j.konopko@xxxxxxxxx>
> ---
> drivers/lightnvm/pblk-core.c | 52 ++++++++++++++++++++++++++++++++++++----
> drivers/lightnvm/pblk-init.c | 37 ++++++++++++++++++++++++++--
> drivers/lightnvm/pblk-rb.c | 3 +++
> drivers/lightnvm/pblk-recovery.c | 25 +++++++++++++++----
> drivers/lightnvm/pblk-sysfs.c | 7 ++++++
> drivers/lightnvm/pblk-write.c | 14 +++++++----
> drivers/lightnvm/pblk.h | 5 +++-
> 7 files changed, 128 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index c092ee93a18d..375c6430612e 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -340,7 +340,7 @@ void pblk_write_should_kick(struct pblk *pblk)
> {
> unsigned int secs_avail = pblk_rb_read_count(&pblk->rwb);
>
> - if (secs_avail >= pblk->min_write_pgs)
> + if (secs_avail >= pblk->min_write_pgs_data)
> pblk_write_kick(pblk);
> }
>
> @@ -371,7 +371,9 @@ struct list_head *pblk_line_gc_list(struct pblk *pblk, struct pblk_line *line)
> struct pblk_line_meta *lm = &pblk->lm;
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct list_head *move_list = NULL;
> - int vsc = le32_to_cpu(*line->vsc);
> + int packed_meta = (le32_to_cpu(*line->vsc) / pblk->min_write_pgs_data)
> + * (pblk->min_write_pgs - pblk->min_write_pgs_data);
> + int vsc = le32_to_cpu(*line->vsc) + packed_meta;
>
> lockdep_assert_held(&line->lock);
>
> @@ -540,12 +542,15 @@ struct bio *pblk_bio_map_addr(struct pblk *pblk, void *data,
> }
>
> int pblk_calc_secs(struct pblk *pblk, unsigned long secs_avail,
> - unsigned long secs_to_flush)
> + unsigned long secs_to_flush, bool skip_meta)
> {
> int max = pblk->sec_per_write;
> int min = pblk->min_write_pgs;
> int secs_to_sync = 0;
>
> + if (skip_meta)
> + min = max = pblk->min_write_pgs_data;
> +
> if (secs_avail >= max)
> secs_to_sync = max;
> else if (secs_avail >= min)
> @@ -663,7 +668,7 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line,
> next_rq:
> memset(&rqd, 0, sizeof(struct nvm_rq));
>
> - rq_ppas = pblk_calc_secs(pblk, left_ppas, 0);
> + rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
> rq_len = rq_ppas * geo->csecs;
>
> bio = pblk_bio_map_addr(pblk, emeta_buf, rq_ppas, rq_len,
> @@ -2091,3 +2096,42 @@ void pblk_lookup_l2p_rand(struct pblk *pblk, struct ppa_addr *ppas,
> }
> spin_unlock(&pblk->trans_lock);
> }
> +
> +void pblk_set_packed_meta(struct pblk *pblk, struct nvm_rq *rqd)
> +{
> + void *meta_list = rqd->meta_list;
> + void *page;
> + int i = 0;
> +
> + if (pblk_is_oob_meta_supported(pblk))
> + return;
> +
> + /* We need to zero out metadata corresponding to packed meta page */
> + pblk_get_meta_at(pblk, meta_list, rqd->nr_ppas - 1)->lba = ADDR_EMPTY;
> +
> + page = page_to_virt(rqd->bio->bi_io_vec[rqd->bio->bi_vcnt - 1].bv_page);
> + /* We need to fill last page of request (packed metadata)
> + * with data from oob meta buffer.
> + */
> + for (; i < rqd->nr_ppas; i++)
> + memcpy(page + (i * sizeof(struct pblk_sec_meta)),
> + pblk_get_meta_at(pblk, meta_list, i),
> + sizeof(struct pblk_sec_meta));
> +}
> +
> +void pblk_get_packed_meta(struct pblk *pblk, struct nvm_rq *rqd)
> +{
> + void *meta_list = rqd->meta_list;
> + void *page;
> + int i = 0;
> +
> + if (pblk_is_oob_meta_supported(pblk))
> + return;
> +
> + page = page_to_virt(rqd->bio->bi_io_vec[rqd->bio->bi_vcnt - 1].bv_page);
> + /* We need to fill oob meta buffer with data from packed metadata */
> + for (; i < rqd->nr_ppas; i++)
> + memcpy(pblk_get_meta_at(pblk, meta_list, i),
> + page + (i * sizeof(struct pblk_sec_meta)),
> + sizeof(struct pblk_sec_meta));
> +}
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index f05112230a52..5eb641da46ed 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -372,8 +372,40 @@ static int pblk_core_init(struct pblk *pblk)
> pblk->min_write_pgs = geo->ws_opt * (geo->csecs / PAGE_SIZE);
> max_write_ppas = pblk->min_write_pgs * geo->all_luns;
> pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA);
> + pblk->min_write_pgs_data = pblk->min_write_pgs;
> pblk_set_sec_per_write(pblk, pblk->min_write_pgs);
>
> + if (!pblk_is_oob_meta_supported(pblk)) {
> + /* For drives which does not have OOB metadata feature
> + * in order to support recovery feature we need to use
> + * so called packed metadata. Packed metada will store
> + * the same information as OOB metadata (l2p table mapping,
> + * but in the form of the single page at the end of
> + * every write request.
> + */
> + if (pblk->min_write_pgs
> + * sizeof(struct pblk_sec_meta) > PAGE_SIZE) {
> + /* We want to keep all the packed metadata on single
> + * page per write requests. So we need to ensure that
> + * it will fit.
> + *
> + * This is more like sanity check, since there is
> + * no device with such a big minimal write size
> + * (above 1 metabytes).
> + */
> + pr_err("pblk: Not supported min write size\n");
> + return -EINVAL;
> + }
> + /* For packed meta approach we do some simplification.
> + * On read path we always issue requests which size
> + * equal to max_write_pgs, with all pages filled with
> + * user payload except of last one page which will be
> + * filled with packed metadata.
> + */
> + pblk->max_write_pgs = pblk->min_write_pgs;
> + pblk->min_write_pgs_data = pblk->min_write_pgs - 1;
> + }
> +
> if (pblk->max_write_pgs > PBLK_MAX_REQ_ADDRS) {
> pr_err("pblk: vector list too big(%u > %u)\n",
> pblk->max_write_pgs, PBLK_MAX_REQ_ADDRS);
> @@ -668,7 +700,7 @@ static void pblk_set_provision(struct pblk *pblk, long nr_free_blks)
> struct pblk_line_meta *lm = &pblk->lm;
> struct nvm_geo *geo = &dev->geo;
> sector_t provisioned;
> - int sec_meta, blk_meta;
> + int sec_meta, blk_meta, clba;
>
> if (geo->op == NVM_TARGET_DEFAULT_OP)
> pblk->op = PBLK_DEFAULT_OP;
> @@ -691,7 +723,8 @@ static void pblk_set_provision(struct pblk *pblk, long nr_free_blks)
> sec_meta = (lm->smeta_sec + lm->emeta_sec[0]) * l_mg->nr_free_lines;
> blk_meta = DIV_ROUND_UP(sec_meta, geo->clba);
>
> - pblk->capacity = (provisioned - blk_meta) * geo->clba;
> + clba = (geo->clba / pblk->min_write_pgs) * pblk->min_write_pgs_data;
> + pblk->capacity = (provisioned - blk_meta) * clba;
>
> atomic_set(&pblk->rl.free_blocks, nr_free_blks);
> atomic_set(&pblk->rl.free_user_blocks, nr_free_blks);
> diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
> index a81a97e8ea6d..081e73e7978f 100644
> --- a/drivers/lightnvm/pblk-rb.c
> +++ b/drivers/lightnvm/pblk-rb.c
> @@ -528,6 +528,9 @@ unsigned int pblk_rb_read_to_bio(struct pblk_rb *rb, struct nvm_rq *rqd,
> to_read = count;
> }
>
> + /* Add space for packed metadata if in use*/
> + pad += (pblk->min_write_pgs - pblk->min_write_pgs_data);
> +
> c_ctx->sentry = pos;
> c_ctx->nr_valid = to_read;
> c_ctx->nr_padded = pad;
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index f5853fc77a0c..0fab18fe30d9 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -138,7 +138,7 @@ static int pblk_recov_read_oob(struct pblk *pblk, struct pblk_line *line,
> next_read_rq:
> memset(rqd, 0, pblk_g_rq_size);
>
> - rq_ppas = pblk_calc_secs(pblk, left_ppas, 0);
> + rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
> if (!rq_ppas)
> rq_ppas = pblk->min_write_pgs;
> rq_len = rq_ppas * geo->csecs;
> @@ -198,6 +198,7 @@ static int pblk_recov_read_oob(struct pblk *pblk, struct pblk_line *line,
> return -EINTR;
> }
>
> + pblk_get_packed_meta(pblk, rqd);
> for (i = 0; i < rqd->nr_ppas; i++) {
> u64 lba = le64_to_cpu(pblk_get_meta_at(pblk,
> meta_list, i)->lba);
> @@ -272,7 +273,7 @@ static int pblk_recov_pad_oob(struct pblk *pblk, struct pblk_line *line,
> kref_init(&pad_rq->ref);
>
> next_pad_rq:
> - rq_ppas = pblk_calc_secs(pblk, left_ppas, 0);
> + rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
> if (rq_ppas < pblk->min_write_pgs) {
> pr_err("pblk: corrupted pad line %d\n", line->id);
> goto fail_free_pad;
> @@ -418,7 +419,7 @@ static int pblk_recov_scan_all_oob(struct pblk *pblk, struct pblk_line *line,
> next_rq:
> memset(rqd, 0, pblk_g_rq_size);
>
> - rq_ppas = pblk_calc_secs(pblk, left_ppas, 0);
> + rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
> if (!rq_ppas)
> rq_ppas = pblk->min_write_pgs;
> rq_len = rq_ppas * geo->csecs;
> @@ -475,6 +476,7 @@ static int pblk_recov_scan_all_oob(struct pblk *pblk, struct pblk_line *line,
> */
> if (!rec_round++ && !rqd->error) {
> rec_round = 0;
> + pblk_get_packed_meta(pblk, rqd);
> for (i = 0; i < rqd->nr_ppas; i++, r_ptr++) {
> u64 lba = le64_to_cpu(pblk_get_meta_at(pblk,
> meta_list, i)->lba);
> @@ -492,6 +494,12 @@ static int pblk_recov_scan_all_oob(struct pblk *pblk, struct pblk_line *line,
> int ret;
>
> bit = find_first_bit((void *)&rqd->ppa_status, rqd->nr_ppas);
> + if (!pblk_is_oob_meta_supported(pblk) && bit > 0) {
> + /* This case should not happen since we always read in
> + * the same unit here as we wrote in writer thread.
> + */
> + pr_err("pblk: Inconsistent packed metadata read\n");
> + }
> nr_error_bits = rqd->nr_ppas - bit;
>
> /* Roll back failed sectors */
> @@ -550,7 +558,7 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
> next_rq:
> memset(rqd, 0, pblk_g_rq_size);
>
> - rq_ppas = pblk_calc_secs(pblk, left_ppas, 0);
> + rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
> if (!rq_ppas)
> rq_ppas = pblk->min_write_pgs;
> rq_len = rq_ppas * geo->csecs;
> @@ -608,6 +616,14 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
> int nr_error_bits, bit;
>
> bit = find_first_bit((void *)&rqd->ppa_status, rqd->nr_ppas);
> + if (!pblk_is_oob_meta_supported(pblk)) {
> + /* For packed metadata we do not handle partially
> + * written requests here, since metadata is always
> + * in last page on the requests.
> + */
> + bit = 0;
> + *done = 0;
> + }
> nr_error_bits = rqd->nr_ppas - bit;
>
> /* Roll back failed sectors */
> @@ -622,6 +638,7 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
> *done = 0;
> }
>
> + pblk_get_packed_meta(pblk, rqd);
> for (i = 0; i < rqd->nr_ppas; i++) {
> u64 lba = le64_to_cpu(pblk_get_meta_at(pblk,
> meta_list, i)->lba);
> diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c
> index b0e5e93a9d5f..aa7b4164ce9e 100644
> --- a/drivers/lightnvm/pblk-sysfs.c
> +++ b/drivers/lightnvm/pblk-sysfs.c
> @@ -473,6 +473,13 @@ static ssize_t pblk_sysfs_set_sec_per_write(struct pblk *pblk,
> if (kstrtouint(page, 0, &sec_per_write))
> return -EINVAL;
>
> + if (!pblk_is_oob_meta_supported(pblk)) {
> + /* For packed metadata case it is
> + * not allowed to change sec_per_write.
> + */
> + return -EINVAL;
> + }
> +
> if (sec_per_write < pblk->min_write_pgs
> || sec_per_write > pblk->max_write_pgs
> || sec_per_write % pblk->min_write_pgs != 0)
> diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> index 6552db35f916..bb45c7e6c375 100644
> --- a/drivers/lightnvm/pblk-write.c
> +++ b/drivers/lightnvm/pblk-write.c
> @@ -354,7 +354,7 @@ static int pblk_calc_secs_to_sync(struct pblk *pblk, unsigned int secs_avail,
> {
> int secs_to_sync;
>
> - secs_to_sync = pblk_calc_secs(pblk, secs_avail, secs_to_flush);
> + secs_to_sync = pblk_calc_secs(pblk, secs_avail, secs_to_flush, true);
>
> #ifdef CONFIG_NVM_PBLK_DEBUG
> if ((!secs_to_sync && secs_to_flush)
> @@ -522,6 +522,11 @@ static int pblk_submit_io_set(struct pblk *pblk, struct nvm_rq *rqd)
> return NVM_IO_ERR;
> }
>
> + /* This is the first place when we have write requests mapped
> + * and we can fill packed metadata with l2p mappings.
> + */
> + pblk_set_packed_meta(pblk, rqd);
> +
> meta_line = pblk_should_submit_meta_io(pblk, rqd);
>
> /* Submit data write for current data line */
> @@ -572,7 +577,7 @@ static int pblk_submit_write(struct pblk *pblk)
> struct bio *bio;
> struct nvm_rq *rqd;
> unsigned int secs_avail, secs_to_sync, secs_to_com;
> - unsigned int secs_to_flush;
> + unsigned int secs_to_flush, packed_meta_pgs;
> unsigned long pos;
> unsigned int resubmit;
>
> @@ -608,7 +613,7 @@ static int pblk_submit_write(struct pblk *pblk)
> return 1;
>
> secs_to_flush = pblk_rb_flush_point_count(&pblk->rwb);
> - if (!secs_to_flush && secs_avail < pblk->min_write_pgs)
> + if (!secs_to_flush && secs_avail < pblk->min_write_pgs_data)
> return 1;
>
> secs_to_sync = pblk_calc_secs_to_sync(pblk, secs_avail,
> @@ -623,7 +628,8 @@ static int pblk_submit_write(struct pblk *pblk)
> pos = pblk_rb_read_commit(&pblk->rwb, secs_to_com);
> }
>
> - bio = bio_alloc(GFP_KERNEL, secs_to_sync);
> + packed_meta_pgs = (pblk->min_write_pgs - pblk->min_write_pgs_data);
> + bio = bio_alloc(GFP_KERNEL, secs_to_sync + packed_meta_pgs);
>
> bio->bi_iter.bi_sector = 0; /* internal bio */
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index 4c61ede5b207..c95ecd8bcf79 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -605,6 +605,7 @@ struct pblk {
> int state; /* pblk line state */
>
> int min_write_pgs; /* Minimum amount of pages required by controller */
> + int min_write_pgs_data; /* Minimum amount of payload pages */
> int max_write_pgs; /* Maximum amount of pages supported by controller */
>
> sector_t capacity; /* Device capacity when bad blocks are subtracted */
> @@ -798,7 +799,7 @@ void pblk_dealloc_page(struct pblk *pblk, struct pblk_line *line, int nr_secs);
> u64 pblk_alloc_page(struct pblk *pblk, struct pblk_line *line, int nr_secs);
> u64 __pblk_alloc_page(struct pblk *pblk, struct pblk_line *line, int nr_secs);
> int pblk_calc_secs(struct pblk *pblk, unsigned long secs_avail,
> - unsigned long secs_to_flush);
> + unsigned long secs_to_flush, bool skip_meta);
> 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);
> @@ -823,6 +824,8 @@ void pblk_lookup_l2p_rand(struct pblk *pblk, struct ppa_addr *ppas,
> u64 *lba_list, int nr_secs);
> void pblk_lookup_l2p_seq(struct pblk *pblk, struct ppa_addr *ppas,
> sector_t blba, int nr_secs);
> +void pblk_set_packed_meta(struct pblk *pblk, struct nvm_rq *rqd);
> +void pblk_get_packed_meta(struct pblk *pblk, struct nvm_rq *rqd);
>
> /*
> * pblk user I/O write path
> --
> 2.14.3
The functionality is good. A couple of comments though:
- Have you considered the case when the device reports ws_min = ws_opt
= 4096? Maybe checks preventing this case would be a good idea.
- Have you checked for any conflicts in the write recovery path? You
would need to deal with more corner cases (e.g., a write failure in
the metadata page would require re-sending always all other sectors).
- There are also corner cases on scan recovery: what if the metadata
page cannot be read? In this case, data loss (at a drive level) will
happen, but you will need to guarantee that reading garbage will not
corrupt other data. In the OOB area case this is not a problem
because data and metadata are always good/not good simultaneously.
- I think it would be good to update the WA counters, since there is a
significant write and space amplification in using (1 / ws_opt)
pages for metadata.
Javier
Attachment:
signature.asc
Description: Message signed with OpenPGP
