| Message ID | 20210105071936.25097-3-lengchao@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | avoid repeated request completion and IO error | expand |
Hello, On 21-01-05 15:19:32, Chao Leng wrote: > When a request is queued failed, if the fail status is not > BLK_STS_RESOURCE, BLK_STS_DEV_RESOURCE, BLK_STS_ZONE_RESOURCE, > the request is need to complete with nvme_complete_rq in queue_rq. > So introduce nvme_try_complete_failed_req. > The request is needed to complete with NVME_SC_HOST_PATH_ERROR in > nvmf_fail_nonready_command and queue_rq. > So introduce nvme_complete_failed_req. > For details, see the subsequent patches. > > Signed-off-by: Chao Leng <lengchao@huawei.com> > --- > drivers/nvme/host/nvme.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h > index bfcedfa4b057..1a0bddb9158f 100644 > --- a/drivers/nvme/host/nvme.h > +++ b/drivers/nvme/host/nvme.h > @@ -649,6 +649,24 @@ void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx); > extern const struct attribute_group *nvme_ns_id_attr_groups[]; > extern const struct block_device_operations nvme_ns_head_ops; > > +static inline void nvme_complete_failed_req(struct request *req) > +{ > + nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR; > + blk_mq_set_request_complete(req); > + nvme_complete_rq(req); > +} > + > +static inline blk_status_t nvme_try_complete_failed_req(struct request *req, > + blk_status_t ret) > +{ > + if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE || > + ret == BLK_STS_ZONE_RESOURCE) > + return ret; If it has nothing to do with various conditions, can we have this if to switch just like the other function in the same file does: switch (ret) { case BLK_STS_RESOURCE: case BLK_STS_DEV_RESOURCE: case BLK_STS_ZONE_RESOURCE: return ret; default: nvme_complete_failed_req(req); return BLK_STS_OK; } > + > + nvme_complete_failed_req(req); > + return BLK_STS_OK; > +} > + Can we have these two functions along side with nvme_try_complete_req() by moving declaration of nvme_coplete_rq() a little bit up ? > #ifdef CONFIG_NVME_MULTIPATH > static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl) > { > -- > 2.16.4 > Thanks,
On 2021/1/6 3:11, Minwoo Im wrote: > Hello, > > On 21-01-05 15:19:32, Chao Leng wrote: >> When a request is queued failed, if the fail status is not >> BLK_STS_RESOURCE, BLK_STS_DEV_RESOURCE, BLK_STS_ZONE_RESOURCE, >> the request is need to complete with nvme_complete_rq in queue_rq. >> So introduce nvme_try_complete_failed_req. >> The request is needed to complete with NVME_SC_HOST_PATH_ERROR in >> nvmf_fail_nonready_command and queue_rq. >> So introduce nvme_complete_failed_req. >> For details, see the subsequent patches. >> >> Signed-off-by: Chao Leng <lengchao@huawei.com> >> --- >> drivers/nvme/host/nvme.h | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h >> index bfcedfa4b057..1a0bddb9158f 100644 >> --- a/drivers/nvme/host/nvme.h >> +++ b/drivers/nvme/host/nvme.h >> @@ -649,6 +649,24 @@ void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx); >> extern const struct attribute_group *nvme_ns_id_attr_groups[]; >> extern const struct block_device_operations nvme_ns_head_ops; >> >> +static inline void nvme_complete_failed_req(struct request *req) >> +{ >> + nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR; >> + blk_mq_set_request_complete(req); >> + nvme_complete_rq(req); >> +} >> + >> +static inline blk_status_t nvme_try_complete_failed_req(struct request *req, >> + blk_status_t ret) >> +{ >> + if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE || >> + ret == BLK_STS_ZONE_RESOURCE) >> + return ret; > > If it has nothing to do with various conditions, can we have this if > to switch just like the other function in the same file does: ok. > > switch (ret) { > case BLK_STS_RESOURCE: > case BLK_STS_DEV_RESOURCE: > case BLK_STS_ZONE_RESOURCE: > return ret; > default: > nvme_complete_failed_req(req); > return BLK_STS_OK; > } > >> + >> + nvme_complete_failed_req(req); >> + return BLK_STS_OK; >> +} >> + > > Can we have these two functions along side with nvme_try_complete_req() > by moving declaration of nvme_coplete_rq() a little bit up ? This may cause the function declaration disordered. > >> #ifdef CONFIG_NVME_MULTIPATH >> static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl) >> { >> -- >> 2.16.4 >> > > Thanks, > . >
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index bfcedfa4b057..1a0bddb9158f 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -649,6 +649,24 @@ void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx); extern const struct attribute_group *nvme_ns_id_attr_groups[]; extern const struct block_device_operations nvme_ns_head_ops; +static inline void nvme_complete_failed_req(struct request *req) +{ + nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR; + blk_mq_set_request_complete(req); + nvme_complete_rq(req); +} + +static inline blk_status_t nvme_try_complete_failed_req(struct request *req, + blk_status_t ret) +{ + if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE || + ret == BLK_STS_ZONE_RESOURCE) + return ret; + + nvme_complete_failed_req(req); + return BLK_STS_OK; +} + #ifdef CONFIG_NVME_MULTIPATH static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl) {
When a request is queued failed, if the fail status is not BLK_STS_RESOURCE, BLK_STS_DEV_RESOURCE, BLK_STS_ZONE_RESOURCE, the request is need to complete with nvme_complete_rq in queue_rq. So introduce nvme_try_complete_failed_req. The request is needed to complete with NVME_SC_HOST_PATH_ERROR in nvmf_fail_nonready_command and queue_rq. So introduce nvme_complete_failed_req. For details, see the subsequent patches. Signed-off-by: Chao Leng <lengchao@huawei.com> --- drivers/nvme/host/nvme.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)