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

[Hannes Reinecke <hare@suse.de>]

On 6/19/20 4:01 PM, Hannes Reinecke wrote:
> 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.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer