Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Minwoo Im <minwoo.im.dev@gmail.com>
To: Keith Busch <kbusch@kernel.org>
Cc: Jens Axboe <axboe@fb.com>, Chao Leng <lengchao@huawei.com>,
	Christoph Hellwig <hch@lst.de>,
	linux-nvme@lists.infradead.org, Sagi Grimberg <sagi@grimberg.me>
Subject: Re: [PATCH V2 2/2] nvme: retry commands based on ACRE result
Date: Thu, 14 Jan 2021 12:28:11 +0900	[thread overview]
Message-ID: <20210114032811.GB5817@localhost.localdomain> (raw)
In-Reply-To: <20210114031102.GA1509941@dhcp-10-100-145-180.wdc.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

      reply	other threads:[~2021-01-14  3:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-13 14:35 [PATCH V2 0/2] nvme: fixes for command retry Minwoo Im
2021-01-13 14:35 ` [PATCH V2 1/2] nvme: check all retry delay times in Identify Controller Minwoo Im
2021-01-13 14:35 ` [PATCH V2 2/2] nvme: retry commands based on ACRE result Minwoo Im
2021-01-13 22:20   ` Sagi Grimberg
2021-01-13 22:43     ` Minwoo Im
2021-01-14  2:47   ` Chao Leng
2021-01-14  3:27     ` Minwoo Im
2021-01-14  3:11   ` Keith Busch
2021-01-14  3:28     ` Minwoo Im [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210114032811.GB5817@localhost.localdomain \
    --to=minwoo.im.dev@gmail.com \
    --cc=axboe@fb.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=lengchao@huawei.com \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox