On Tue, Apr 07, 2020 at 07:14:05PM +0200, Christoph Hellwig wrote:
> 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);
> }
Nice!
>
>
> > @@ -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.
Indeed, just forgot to do that.
Thanks,
Ming
