Re: [PATCH 5/9] lightnvm: pblk: consider bad sectors in emeta during recovery

From: Javier GonzÃlez
Date: Tue Oct 03 2017 - 06:38:34 EST


> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@xxxxxxxxxxxxx> wrote:
>
> From: Hans Holmberg <hans.holmberg@xxxxxxxxxxxx>
>
> When recovering lines we need to consider that bad blocks in a line
> affects the emeta area size.
>
> Previously it was assumed that the emeta area would grow by the
> number of sectors per page * number of bad blocks in the line.
>
> This assumtion not correct - the number of "extra" pages that are
> consumed could be both smaller (depending on emeta size) and bigger
> (depending on the placement of the bad blocks).

This assumption is not correct >> missing _is_

>
> Fix this by calculating the emeta start by iterating backwards
> through the line, skipping ppas that map to bad blocks.
>
> Also fix the data types used for ppa indices/counts in
> pblk_recov_l2p_from_emeta - we should use u64.
>
> Signed-off-by: Hans Holmberg <hans.holmberg@xxxxxxxxxxxx>
> ---
> drivers/lightnvm/pblk-recovery.c | 44 +++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 74b3b86..b5a2275 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -133,16 +133,16 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> struct pblk_emeta *emeta = line->emeta;
> struct line_emeta *emeta_buf = emeta->buf;
> __le64 *lba_list;
> - int data_start, data_end;
> - int nr_valid_lbas, nr_lbas = 0;
> - int i;
> + u64 data_start, data_end;
> + u64 nr_valid_lbas, nr_lbas = 0;
> + u64 i;
>
> lba_list = pblk_recov_get_lba_list(pblk, emeta_buf);
> if (!lba_list)
> return 1;
>
> data_start = pblk_line_smeta_start(pblk, line) + lm->smeta_sec;
> - data_end = lm->sec_per_line - lm->emeta_sec[0];
> + data_end = line->emeta_ssec;
> nr_valid_lbas = le64_to_cpu(emeta_buf->nr_valid_lbas);
>
> for (i = data_start; i < data_end; i++) {
> @@ -172,8 +172,8 @@ static int pblk_recov_l2p_from_emeta(struct pblk *pblk, struct pblk_line *line)
> }
>
> if (nr_valid_lbas != nr_lbas)
> - pr_err("pblk: line %d - inconsistent lba list(%llu/%d)\n",
> - line->id, emeta_buf->nr_valid_lbas, nr_lbas);
> + pr_err("pblk: line %d - inconsistent lba list(%llu/%llu)\n",
> + line->id, nr_valid_lbas, nr_lbas);
>
> line->left_msecs = 0;
>
> @@ -827,11 +827,33 @@ static void pblk_recov_line_add_ordered(struct list_head *head,
> __list_add(&line->list, t->list.prev, &t->list);
> }
>
> -struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
> +static u64 pblk_line_emeta_start(struct pblk *pblk, struct pblk_line *line)
> {
> struct nvm_tgt_dev *dev = pblk->dev;
> struct nvm_geo *geo = &dev->geo;
> struct pblk_line_meta *lm = &pblk->lm;
> + unsigned int emeta_secs;
> + u64 emeta_start;
> + struct ppa_addr ppa;
> + int pos;
> +
> + emeta_secs = lm->emeta_sec[0];
> + emeta_start = lm->sec_per_line;
> +
> + while (emeta_secs) {
> + emeta_start--;
> + ppa = addr_to_pblk_ppa(pblk, emeta_start, line->id);
> + pos = pblk_ppa_to_pos(geo, ppa);
> + if (!test_bit(pos, line->blk_bitmap))
> + emeta_secs--;
> + }
> +
> + return emeta_start;
> +}
> +
> +struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
> +{
> + struct pblk_line_meta *lm = &pblk->lm;
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct pblk_line *line, *tline, *data_line = NULL;
> struct pblk_smeta *smeta;
> @@ -930,15 +952,9 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
>
> /* Verify closed blocks and recover this portion of L2P table*/
> list_for_each_entry_safe(line, tline, &recov_list, list) {
> - int off, nr_bb;
> -
> recovered_lines++;
> - /* Calculate where emeta starts based on the line bb */
> - off = lm->sec_per_line - lm->emeta_sec[0];
> - nr_bb = bitmap_weight(line->blk_bitmap, lm->blk_per_line);
> - off -= nr_bb * geo->sec_per_pl;
>
> - line->emeta_ssec = off;
> + line->emeta_ssec = pblk_line_emeta_start(pblk, line);
> line->emeta = emeta;
> memset(line->emeta->buf, 0, lm->emeta_len[0]);
>
> --
> 2.7.4

Apart from the commit message, it looks good.

Reviewed-by: Javier GonzÃlez <javier@xxxxxxxxxxxx>

Attachment: signature.asc
Description: Message signed with OpenPGP