Re: [PATCH V2] lightnvm: pblk: extend line wp balance check

From: Javier GonzÃlez
Date: Wed Jan 30 2019 - 04:38:56 EST


> On 30 Jan 2019, at 09.18, hans@xxxxxxxxxxxxx wrote:
>
> From: Hans Holmberg <hans.holmberg@xxxxxxxxxxxx>
>
> pblk stripes writes of minimal write size across all non-offline chunks
> in a line, which means that the maximum write pointer delta should not
> exceed the minimal write size.
>
> Extend the line write pointer balance check to cover this case, and
> ignore the offline chunk wps.
>
> This will render us a warning during recovery if something unexpected
> has happened to the chunk write pointers (i.e. powerloss, a spurious
> chunk reset, ..).
>
> Reported-by: Zhoujie Wu <zjwu@xxxxxxxxxxx>
> Tested-by: Zhoujie Wu <zjwu@xxxxxxxxxxx>
> Signed-off-by: Hans Holmberg <hans.holmberg@xxxxxxxxxxxx>
> ---
>
> Changes since V1:
>
> * Squashed with Zhoujie's:
> "lightnvm: pblk: ignore bad block wp for pblk_line_wp_is_unbalanced"
> * Clarified commit message based on Javier's comments.
>
>
> drivers/lightnvm/pblk-recovery.c | 56 ++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 6761d2afa4d0..d86f580036d3 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -302,35 +302,55 @@ static int pblk_pad_distance(struct pblk *pblk, struct pblk_line *line)
> return (distance > line->left_msecs) ? line->left_msecs : distance;
> }
>
> -static int pblk_line_wp_is_unbalanced(struct pblk *pblk,
> - struct pblk_line *line)
> +/* Return a chunk belonging to a line by stripe(write order) index */
> +static struct nvm_chk_meta *pblk_get_stripe_chunk(struct pblk *pblk,
> + struct pblk_line *line,
> + int index)
> {
> struct nvm_tgt_dev *dev = pblk->dev;
> struct nvm_geo *geo = &dev->geo;
> - struct pblk_line_meta *lm = &pblk->lm;
> struct pblk_lun *rlun;
> - struct nvm_chk_meta *chunk;
> struct ppa_addr ppa;
> - u64 line_wp;
> - int pos, i;
> + int pos;
>
> - rlun = &pblk->luns[0];
> + rlun = &pblk->luns[index];
> ppa = rlun->bppa;
> pos = pblk_ppa_to_pos(geo, ppa);
> - chunk = &line->chks[pos];
>
> - line_wp = chunk->wp;
> + return &line->chks[pos];
> +}
>
> - for (i = 1; i < lm->blk_per_line; i++) {
> - rlun = &pblk->luns[i];
> - ppa = rlun->bppa;
> - pos = pblk_ppa_to_pos(geo, ppa);
> - chunk = &line->chks[pos];
> +static int pblk_line_wps_are_unbalanced(struct pblk *pblk,
> + struct pblk_line *line)
> +{
> + struct pblk_line_meta *lm = &pblk->lm;
> + int blk_in_line = lm->blk_per_line;
> + struct nvm_chk_meta *chunk;
> + u64 max_wp, min_wp;
> + int i;
> +
> + i = find_first_zero_bit(line->blk_bitmap, blk_in_line);
>
> - if (chunk->wp > line_wp)
> + /* If there is one or zero good chunks in the line,
> + * the write pointers can't be unbalanced.
> + */
> + if (i >= (blk_in_line - 1))
> + return 0;
> +
> + chunk = pblk_get_stripe_chunk(pblk, line, i);
> + max_wp = chunk->wp;
> + if (max_wp > pblk->max_write_pgs)
> + min_wp = max_wp - pblk->max_write_pgs;
> + else
> + min_wp = 0;
> +
> + i = find_next_zero_bit(line->blk_bitmap, blk_in_line, i + 1);
> + while (i < blk_in_line) {
> + chunk = pblk_get_stripe_chunk(pblk, line, i);
> + if (chunk->wp > max_wp || chunk->wp < min_wp)
> return 1;
> - else if (chunk->wp < line_wp)
> - line_wp = chunk->wp;
> +
> + i = find_next_zero_bit(line->blk_bitmap, blk_in_line, i + 1);
> }
>
> return 0;
> @@ -356,7 +376,7 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
> int ret;
> u64 left_ppas = pblk_sec_in_open_line(pblk, line) - lm->smeta_sec;
>
> - if (pblk_line_wp_is_unbalanced(pblk, line))
> + if (pblk_line_wps_are_unbalanced(pblk, line))
> pblk_warn(pblk, "recovering unbalanced line (%d)\n", line->id);
>
> ppa_list = p.ppa_list;
> --
> 2.17.1

Looks good to me.

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

Attachment: signature.asc
Description: Message signed with OpenPGP