| Message ID | 20210201034940.18891-2-lengchao@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | avoid double request completion and IO error | expand |
On 1/31/21 8:49 PM, Chao Leng wrote: > nvme drivers need to set the state of request to MQ_RQ_COMPLETE when > directly complete request in queue_rq. > So add blk_mq_set_request_complete. This is a bit iffy, but looks ok for the intended use case. We just have to be careful NOT to add frivolous users of it...
On Thu, Feb 04, 2021 at 08:27:46AM -0700, Jens Axboe wrote: > On 1/31/21 8:49 PM, Chao Leng wrote: > > nvme drivers need to set the state of request to MQ_RQ_COMPLETE when > > directly complete request in queue_rq. > > So add blk_mq_set_request_complete. > > This is a bit iffy, but looks ok for the intended use case. We just have > to be careful NOT to add frivolous users of it... Yes, that is my main issue with it. The current use case looks fine, but I suspect every time someone else uses it it's probly going to be as misuse. I've applied this to nvme-5.12 with the rest of the patches, it should be on its way to you soon.
On 2/4/21 8:30 AM, Christoph Hellwig wrote: > On Thu, Feb 04, 2021 at 08:27:46AM -0700, Jens Axboe wrote: >> On 1/31/21 8:49 PM, Chao Leng wrote: >>> nvme drivers need to set the state of request to MQ_RQ_COMPLETE when >>> directly complete request in queue_rq. >>> So add blk_mq_set_request_complete. >> >> This is a bit iffy, but looks ok for the intended use case. We just have >> to be careful NOT to add frivolous users of it... > > Yes, that is my main issue with it. The current use case looks fine, > but I suspect every time someone else uses it it's probly going to be > as misuse. I've applied this to nvme-5.12 with the rest of the patches, > it should be on its way to you soon. Might benefit from a big fat comment on why you probably shouldn't be using it...
On Thu, Feb 04, 2021 at 08:32:17AM -0700, Jens Axboe wrote: > On 2/4/21 8:30 AM, Christoph Hellwig wrote: > > On Thu, Feb 04, 2021 at 08:27:46AM -0700, Jens Axboe wrote: > >> On 1/31/21 8:49 PM, Chao Leng wrote: > >>> nvme drivers need to set the state of request to MQ_RQ_COMPLETE when > >>> directly complete request in queue_rq. > >>> So add blk_mq_set_request_complete. > >> > >> This is a bit iffy, but looks ok for the intended use case. We just have > >> to be careful NOT to add frivolous users of it... > > > > Yes, that is my main issue with it. The current use case looks fine, > > but I suspect every time someone else uses it it's probly going to be > > as misuse. I've applied this to nvme-5.12 with the rest of the patches, > > it should be on its way to you soon. > > Might benefit from a big fat comment on why you probably shouldn't > be using it... I actually reworded the comment a bit, this is the current version: http://git.infradead.org/nvme.git/commitdiff/bdd2a4a61460a341c1f8462e5caf63195e960623
On 2/4/21 8:36 AM, Christoph Hellwig wrote: > On Thu, Feb 04, 2021 at 08:32:17AM -0700, Jens Axboe wrote: >> On 2/4/21 8:30 AM, Christoph Hellwig wrote: >>> On Thu, Feb 04, 2021 at 08:27:46AM -0700, Jens Axboe wrote: >>>> On 1/31/21 8:49 PM, Chao Leng wrote: >>>>> nvme drivers need to set the state of request to MQ_RQ_COMPLETE when >>>>> directly complete request in queue_rq. >>>>> So add blk_mq_set_request_complete. >>>> >>>> This is a bit iffy, but looks ok for the intended use case. We just have >>>> to be careful NOT to add frivolous users of it... >>> >>> Yes, that is my main issue with it. The current use case looks fine, >>> but I suspect every time someone else uses it it's probly going to be >>> as misuse. I've applied this to nvme-5.12 with the rest of the patches, >>> it should be on its way to you soon. >> >> Might benefit from a big fat comment on why you probably shouldn't >> be using it... > > I actually reworded the comment a bit, this is the current version: > > http://git.infradead.org/nvme.git/commitdiff/bdd2a4a61460a341c1f8462e5caf63195e960623 Alright looks fine, as it clearly states it should be used from ->queue_rq().
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index 47b021952ac7..38c632ce2270 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -494,6 +494,15 @@ static inline int blk_mq_request_completed(struct request *rq) return blk_mq_rq_state(rq) == MQ_RQ_COMPLETE; } +/* + * If nvme complete request directly, need to set the state to complete + * to avoid canceling the request again. + */ +static inline void blk_mq_set_request_complete(struct request *rq) +{ + WRITE_ONCE(rq->state, MQ_RQ_COMPLETE); +} + void blk_mq_start_request(struct request *rq); void blk_mq_end_request(struct request *rq, blk_status_t error); void __blk_mq_end_request(struct request *rq, blk_status_t error);
nvme drivers need to set the state of request to MQ_RQ_COMPLETE when directly complete request in queue_rq. So add blk_mq_set_request_complete. Signed-off-by: Chao Leng <lengchao@huawei.com> --- include/linux/blk-mq.h | 9 +++++++++ 1 file changed, 9 insertions(+)