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

[Javier GonzÃlez]

> 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