* [PATCH 2/3] nvme-core: delete the dependency on blk status
@ 2020-08-12 8:18 Chao Leng
2020-08-12 15:10 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Chao Leng @ 2020-08-12 8:18 UTC (permalink / raw)
To: linux-nvme, linux-block; +Cc: kbusch, axboe, hch, sagi, lengchao
nvme should not depend on blk status, just need check nvme status.
Just need do translating nvme status to blk status for returning error.
Signed-off-by: Chao Leng <lengchao@huawei.com>
---
drivers/nvme/host/core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 52d19a4d3bc8..246988091c05 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -278,7 +278,7 @@ static void nvme_retry_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
- blk_status_t status = nvme_error_status(nvme_req(req)->status);
+ blk_status_t status;
trace_nvme_complete_rq(req);
@@ -287,7 +287,8 @@ void nvme_complete_rq(struct request *req)
if (nvme_req(req)->ctrl->kas)
nvme_req(req)->ctrl->comp_seen = true;
- if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
+ if (unlikely(nvme_req(req)->status != NVME_SC_SUCCESS &&
+ nvme_req_needs_retry(req))) {
if (nvme_req_path_error(req)) {
if (req->cmd_flags & REQ_NVME_MPATH) {
nvme_failover_req(req);
@@ -299,6 +300,7 @@ void nvme_complete_rq(struct request *req)
}
}
+ status = nvme_error_status(nvme_req(req)->status);
nvme_trace_bio_complete(req, status);
blk_mq_end_request(req, status);
}
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] nvme-core: delete the dependency on blk status
2020-08-12 8:18 [PATCH 2/3] nvme-core: delete the dependency on blk status Chao Leng
@ 2020-08-12 15:10 ` Christoph Hellwig
2020-08-12 20:28 ` Mike Snitzer
2020-08-13 3:53 ` Chao Leng
0 siblings, 2 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-08-12 15:10 UTC (permalink / raw)
To: Chao Leng; +Cc: linux-nvme, linux-block, kbusch, axboe, hch, sagi
On Wed, Aug 12, 2020 at 04:18:44PM +0800, Chao Leng wrote:
> nvme should not depend on blk status, just need check nvme status.
> Just need do translating nvme status to blk status for returning error.
While this doesn't look wrong it also doesn't save us a single
instruction and actually adds more lines of code. Do you have a good
reason for this change?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] nvme-core: delete the dependency on blk status
2020-08-12 15:10 ` Christoph Hellwig
@ 2020-08-12 20:28 ` Mike Snitzer
2020-08-13 15:47 ` Christoph Hellwig
2020-08-13 3:53 ` Chao Leng
1 sibling, 1 reply; 5+ messages in thread
From: Mike Snitzer @ 2020-08-12 20:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chao Leng, axboe, sagi, linux-nvme, linux-block, kbusch
On Wed, Aug 12 2020 at 11:10am -0400,
Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Aug 12, 2020 at 04:18:44PM +0800, Chao Leng wrote:
> > nvme should not depend on blk status, just need check nvme status.
> > Just need do translating nvme status to blk status for returning error.
>
> While this doesn't look wrong it also doesn't save us a single
> instruction and actually adds more lines of code. Do you have a good
> reason for this change?
It certainly saves nvme_error_status(nvme_req(req)->status if
nvme_req_needs_retry().
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] nvme-core: delete the dependency on blk status
2020-08-12 20:28 ` Mike Snitzer
@ 2020-08-13 15:47 ` Christoph Hellwig
0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-08-13 15:47 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Chao Leng, axboe, sagi, linux-nvme,
linux-block, kbusch
On Wed, Aug 12, 2020 at 04:28:30PM -0400, Mike Snitzer wrote:
> On Wed, Aug 12 2020 at 11:10am -0400,
> Christoph Hellwig <hch@lst.de> wrote:
>
> > On Wed, Aug 12, 2020 at 04:18:44PM +0800, Chao Leng wrote:
> > > nvme should not depend on blk status, just need check nvme status.
> > > Just need do translating nvme status to blk status for returning error.
> >
> > While this doesn't look wrong it also doesn't save us a single
> > instruction and actually adds more lines of code. Do you have a good
> > reason for this change?
>
> It certainly saves nvme_error_status(nvme_req(req)->status if
> nvme_req_needs_retry().
Yeah, but the retry path isn't exactly the fast path, is it?
Not that I really care too much about this one..
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] nvme-core: delete the dependency on blk status
2020-08-12 15:10 ` Christoph Hellwig
2020-08-12 20:28 ` Mike Snitzer
@ 2020-08-13 3:53 ` Chao Leng
1 sibling, 0 replies; 5+ messages in thread
From: Chao Leng @ 2020-08-13 3:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, axboe, sagi
On 2020/8/12 23:10, Christoph Hellwig wrote:
> On Wed, Aug 12, 2020 at 04:18:44PM +0800, Chao Leng wrote:
>> nvme should not depend on blk status, just need check nvme status.
>> Just need do translating nvme status to blk status for returning error.
>
> While this doesn't look wrong it also doesn't save us a single
> instruction and actually adds more lines of code. Do you have a good
> reason for this change?
If need retry, save nvme_error_status. Of course, it doesn't matter.
There's no logical change, just want nvme do not care blk status when
check retry.
It is ok to change or not.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-13 15:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12 8:18 [PATCH 2/3] nvme-core: delete the dependency on blk status Chao Leng
2020-08-12 15:10 ` Christoph Hellwig
2020-08-12 20:28 ` Mike Snitzer
2020-08-13 15:47 ` Christoph Hellwig
2020-08-13 3:53 ` Chao Leng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).