Re: [PATCH 07/18] lightnvm: pblk: wait for inflight IOs in recovery

["Matias Bjørling" <mb@lightnvm.io>]

On 3/14/19 9:04 AM, Igor Konopko wrote:
> This patch changes the behaviour of recovery padding in order to
> support a case, when some IOs were already submitted to the drive and
> some next one are not submitted due to error returned.
> 
> Currently in case of errors we simply exit the pad function without
> waiting for inflight IOs, which leads to panic on inflight IOs
> completion.
> 
> After the changes we always wait for all the inflight IOs before
> exiting the function.
> 
> Also, since NVMe has an internal timeout per IO, there is no need to
> introduce additonal one here.
> 
> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
> ---
>   drivers/lightnvm/pblk-recovery.c | 32 +++++++++++++-------------------
>   1 file changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index ba1691d..73d5ead 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -200,7 +200,7 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   	rq_ppas = pblk_calc_secs(pblk, left_ppas, 0, false);
>   	if (rq_ppas < pblk->min_write_pgs) {
>   		pblk_err(pblk, "corrupted pad line %d\n", line->id);
> -		goto fail_free_pad;
> +		goto fail_complete;
>   	}
>   
>   	rq_len = rq_ppas * geo->csecs;
> @@ -209,7 +209,7 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   						PBLK_VMALLOC_META, GFP_KERNEL);
>   	if (IS_ERR(bio)) {
>   		ret = PTR_ERR(bio);
> -		goto fail_free_pad;
> +		goto fail_complete;
>   	}
>   
>   	bio->bi_iter.bi_sector = 0; /* internal bio */
> @@ -218,8 +218,11 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   	rqd = pblk_alloc_rqd(pblk, PBLK_WRITE_INT);
>   
>   	ret = pblk_alloc_rqd_meta(pblk, rqd);
> -	if (ret)
> -		goto fail_free_rqd;
> +	if (ret) {
> +		pblk_free_rqd(pblk, rqd, PBLK_WRITE_INT);
> +		bio_put(bio);
> +		goto fail_complete;
> +	}
>   
>   	rqd->bio = bio;
>   	rqd->opcode = NVM_OP_PWRITE;
> @@ -266,7 +269,10 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   	if (ret) {
>   		pblk_err(pblk, "I/O submission failed: %d\n", ret);
>   		pblk_up_chunk(pblk, rqd->ppa_list[0]);
> -		goto fail_free_rqd;
> +		kref_put(&pad_rq->ref, pblk_recov_complete);
> +		pblk_free_rqd(pblk, rqd, PBLK_WRITE_INT);
> +		bio_put(bio);
> +		goto fail_complete;
>   	}
>   
>   	left_line_ppas -= rq_ppas;
> @@ -274,13 +280,9 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   	if (left_ppas && left_line_ppas)
>   		goto next_pad_rq;
>   
> +fail_complete:
>   	kref_put(&pad_rq->ref, pblk_recov_complete);
> -
> -	if (!wait_for_completion_io_timeout(&pad_rq->wait,
> -				msecs_to_jiffies(PBLK_COMMAND_TIMEOUT_MS))) {
> -		pblk_err(pblk, "pad write timed out\n");
> -		ret = -ETIME;
> -	}
> +	wait_for_completion(&pad_rq->wait);
>   
>   	if (!pblk_line_is_full(line))
>   		pblk_err(pblk, "corrupted padded line: %d\n", line->id);
> @@ -289,14 +291,6 @@ static int pblk_recov_pad_line(struct pblk *pblk, struct pblk_line *line,
>   free_rq:
>   	kfree(pad_rq);
>   	return ret;
> -
> -fail_free_rqd:
> -	pblk_free_rqd(pblk, rqd, PBLK_WRITE_INT);
> -	bio_put(bio);
> -fail_free_pad:
> -	kfree(pad_rq);
> -	vfree(data);
> -	return ret;
>   }
>   
>   static int pblk_pad_distance(struct pblk *pblk, struct pblk_line *line)
> 

Hi Igor,

Can you split this patch in two. One that removes the 
wait_for_completion_io_timeout (and constant), and another that makes 
sure it waits until all inflight IOs are completed?