On Mon, Jul 22, 2019 at 09:51:27AM -0700, Bart Van Assche wrote:
> On 7/19/19 8:06 PM, Ming Lei wrote:
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index b038ec680e84..fc38d95c557f 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -502,6 +502,9 @@ void blk_mq_free_request(struct request *rq)
> > struct blk_mq_ctx *ctx = rq->mq_ctx;
> > struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
> > + if (q->mq_ops->cleanup_rq)
> > + q->mq_ops->cleanup_rq(rq);
> > +
> > if (rq->rq_flags & RQF_ELVPRIV) {
> > if (e && e->type->ops.finish_request)
> > e->type->ops.finish_request(rq);
>
> I'm concerned about the performance impact of this change. How about not
Not see any performance impact in my test, and q->mq_ops should be in
data cache at that time.
> introducing .cleanup_rq() and adding a call to
> scsi_mq_uninit_cmd() in scsi_queue_rq() just before that function returns
> BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE?
The problem is that only dm-rq needs to free the request private data
when BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE is returned. If we do that
unconditionally, performance impact might be visible.
Thanks,
Ming
