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

[Matias BjÃrling]

On 1/30/19 9:18 AM, 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;

Thanks Hans and Zhoujie. I've applied it for 5.1.