Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH V2] lightnvm: pblk: extend line wp balance check
To:     hans@owltronix.com
Cc:     javier@javigon.com, Zhoujie Wu <zjwu@marvell.com>,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        Hans Holmberg <hans.holmberg@cnexlabs.com>
References: <20190130081857.6779-1-hans@owltronix.com>
From:   =?UTF-8?Q?Matias_Bj=c3=b8rling?= <mb@lightnvm.io>
Message-ID: <b653e741-27ec-1a79-0375-3d5f375cbbe4@lightnvm.io>
Date:   Wed, 30 Jan 2019 14:33:19 +0100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0)
 Gecko/20100101 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <20190130081857.6779-1-hans@owltronix.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 1/30/19 9:18 AM, hans@owltronix.com wrote:
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> 
> 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@marvell.com>
> Tested-by: Zhoujie Wu <zjwu@marvell.com>
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> 
> 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.