On Tue, Jul 27, 2021 at 05:32:53PM +0800, John Garry wrote: > If the blk_mq_sched_alloc_tags() -> blk_mq_alloc_rqs() call fails, then we > call blk_mq_sched_free_tags() -> blk_mq_free_rqs(). > > It is incorrect to do so, as any rqs would have already been freed in the > blk_mq_alloc_rqs() call. > > Fix by calling blk_mq_free_rq_map() only directly. > > Fixes: 6917ff0b5bd41 ("blk-mq-sched: refactor scheduler initialization") > Signed-off-by: John Garry <john.garry@xxxxxxxxxx> Not sure it is one fix, because blk_mq_free_rqs() does nothing when ->static_rqs[] isn't filled, so 'Fixes' tag isn't needed, IMO. > > diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c > index c838d81ac058..0f006cabfd91 100644 > --- a/block/blk-mq-sched.c > +++ b/block/blk-mq-sched.c > @@ -515,17 +515,6 @@ void blk_mq_sched_insert_requests(struct blk_mq_hw_ctx *hctx, > percpu_ref_put(&q->q_usage_counter); > } > > -static void blk_mq_sched_free_tags(struct blk_mq_tag_set *set, > - struct blk_mq_hw_ctx *hctx, > - unsigned int hctx_idx) > -{ > - if (hctx->sched_tags) { > - blk_mq_free_rqs(set, hctx->sched_tags, hctx_idx); > - blk_mq_free_rq_map(hctx->sched_tags, set->flags); > - hctx->sched_tags = NULL; > - } > -} > - > static int blk_mq_sched_alloc_tags(struct request_queue *q, > struct blk_mq_hw_ctx *hctx, > unsigned int hctx_idx) > @@ -539,8 +528,10 @@ static int blk_mq_sched_alloc_tags(struct request_queue *q, > return -ENOMEM; > > ret = blk_mq_alloc_rqs(set, hctx->sched_tags, hctx_idx, q->nr_requests); > - if (ret) > - blk_mq_sched_free_tags(set, hctx, hctx_idx); > + if (ret) { > + blk_mq_free_rq_map(hctx->sched_tags, set->flags); > + hctx->sched_tags = NULL; > + } The patch itself looks fine: Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx> Thanks, Ming