Re: [PATCH V2 2/2] nvme: retry commands based on ACRE result

[Minwoo Im <minwoo.im.dev@gmail.com>]

Hello Keith,

On 21-01-13 19:11:02, Keith Busch wrote:
> On Wed, Jan 13, 2021 at 11:35:38PM +0900, Minwoo Im wrote:
> > @@ -317,6 +316,13 @@ static inline enum nvme_disposition nvme_decide_disposition(struct request *req)
> >  			return COMPLETE;
> >  	}
> >  
> > +	if (nvme_req(req)->ctrl->acre) {
> > +		if (!nvme_is_path_error(nvme_req(req)->status) &&
> > +				!blk_queue_dying(req->q))
> > +			return RETRY;
> > +	} else if (blk_noretry_request(req))
> > +		return COMPLETE;
> > +
> >  	return RETRY;
> >  }
> 
> This is returning RETRY for path related errors when blk_notry_request()
> is true. That will deadlock initialization on a command timeout. The
> check should be something like this:
> 
> 	if (nvme_req(req)->ctrl->acre &&
> 	    !nvme_is_path_error(nvme_req(req)->status) &&
> 	    !blk_queue_dying(req->q))
> 		return RETRY;
> 	else if (blk_noretry_request(req))
> 		return COMPLETE;
> 
> 

Thank you for chatching this!  As Chao pointed it out, I will prepare
the next version of this patch.  I really appreciate it.

Thanks!

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme