Thread (6 messages) 6 messages, 4 authors, 2020-06-22

Re: [PATCH 2/2] block: only return started requests from blk_mq_tag_to_rq()

From: Jens Axboe <axboe@kernel.dk>
Date: 2020-06-19 18:38:14
Also in: linux-scsi

On 6/19/20 8:09 AM, Hannes Reinecke wrote:
On 6/19/20 4:01 PM, Hannes Reinecke wrote:
quoted
blk_mq_tag_to_rq() is used from within the driver to map a tag
to a request. As such it should only return requests which are
already started (ie passed to the driver); otherwise the driver
might trip over requests which it has never seen and random
crashes will occur.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
  block/blk-mq.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4f57d27bfa73..f02d18113f9e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -815,9 +815,13 @@ EXPORT_SYMBOL(blk_mq_delay_kick_requeue_list);
  
  struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)
  {
+	struct request *rq;
+
  	if (tag < tags->nr_tags) {
  		prefetch(tags->rqs[tag]);
-		return tags->rqs[tag];
+		rq = tags->rqs[tag];
+		if (blk_mq_request_started(rq))
+			return rq;
  	}
  
  	return NULL;
This becomes particularly obnoxious for SCSI drivers using 
scsi_host_find_tag() for cleaning up stale commands (ie drivers like 
qla4xxx, fnic, and snic).
All other drivers use it from the completion routine, so one can expect 
a valid (and started) tag here. So for those it shouldn't matter.

But still, if there are objections I could look at fixing it within the 
SCSI stack; although that would most likely mean I'll have to implement 
the above patch as an additional function.
The helper does exactly what it should, return a request associated
with a tag. Either add the logic to the caller, or provide a new
helper that does what you need. I'd be inclined to just add it to
the caller that needs it.

-- 
Jens Axboe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help