Re: [PATCH V6 1/8] blk-mq: assign rq->tag in blk_mq_get_driver_tag

[Christoph Hellwig <hch@xxxxxx>]

On Tue, Apr 07, 2020 at 05:28:54PM +0800, Ming Lei wrote:
> @@ -472,14 +462,18 @@ static void __blk_mq_free_request(struct request *rq)
>  	struct request_queue *q = rq->q;
>  	struct blk_mq_ctx *ctx = rq->mq_ctx;
>  	struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
> -	const int sched_tag = rq->internal_tag;
> +	const int tag = rq->internal_tag;
> +	bool has_sched = !!hctx->sched_tags;
>  
>  	blk_pm_mark_last_busy(rq);
>  	rq->mq_hctx = NULL;
> +	if (!has_sched)
> +		blk_mq_put_tag(hctx->tags, ctx, tag);
> +	else if (rq->tag >= 0)
>  		blk_mq_put_tag(hctx->tags, ctx, rq->tag);
> +
> +	if (has_sched)
> +		blk_mq_put_tag(hctx->sched_tags, ctx, tag);

This looks weird to me.  Why not simply:

	if (hctx->sched_tags) {
		if (rq->tag >= 0)
			blk_mq_put_tag(hctx->tags, ctx, rq->tag);
		blk_mq_put_tag(hctx->sched_tags, ctx, rq->internal_tag);
	} else {
		blk_mq_put_tag(hctx->tags, ctx, rq->internal_tag);
	}


> @@ -1037,14 +1031,21 @@ bool blk_mq_get_driver_tag(struct request *rq)

FYI, it seems like blk_mq_get_driver_tag can be marked static.

Otherwise this looks pretty sensible to me.