> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmb...@owltronix.com> wrote: > > From: Hans Holmberg <hans.holmb...@cnexlabs.com> > > 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.holmb...@cnexlabs.com>
> ---
> 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 <jav...@cnexlabs.com>
signature.asc
Description: Message signed with OpenPGP
