lkml.org 
[lkml]   [2016]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/7] blk-mq-sched: add framework for MQ capable IO schedulers
From
Date
On 12/08/2016 09:13 PM, Jens Axboe wrote:
> +static inline void blk_mq_sched_put_request(struct request *rq)
> +{
> + struct request_queue *q = rq->q;
> + struct elevator_queue *e = q->elevator;
> +
> + if (e && e->type->mq_ops.put_request)
> + e->type->mq_ops.put_request(rq);
> + else
> + blk_mq_free_request(rq);
> +}

blk_mq_free_request() always triggers a call of blk_queue_exit().
dd_put_request() only triggers a call of blk_queue_exit() if it is not a
shadow request. Is that on purpose?

> +static inline struct request *
> +blk_mq_sched_get_request(struct request_queue *q, unsigned int op,
> + struct blk_mq_alloc_data *data)
> +{
> + struct elevator_queue *e = q->elevator;
> + struct blk_mq_hw_ctx *hctx;
> + struct blk_mq_ctx *ctx;
> + struct request *rq;
> +
> + blk_queue_enter_live(q);
> + ctx = blk_mq_get_ctx(q);
> + hctx = blk_mq_map_queue(q, ctx->cpu);
> +
> + blk_mq_set_alloc_data(data, q, 0, ctx, hctx);
> +
> + if (e && e->type->mq_ops.get_request)
> + rq = e->type->mq_ops.get_request(q, op, data);
> + else
> + rq = __blk_mq_alloc_request(data, op);
> +
> + if (rq)
> + data->hctx->queued++;
> +
> + return rq;
> +
> +}

Some but not all callers of blk_mq_sched_get_request() call
blk_queue_exit() if this function returns NULL. Please consider to move
the blk_queue_exit() call from the blk_mq_alloc_request() error path
into this function. I think that will make it a lot easier to verify
whether or not the blk_queue_enter() / blk_queue_exit() calls are
balanced properly.

Additionally, since blk_queue_enter() / blk_queue_exit() calls by
blk_mq_sched_get_request() and blk_mq_sched_put_request() must be
balanced and since the latter function only calls blk_queue_exit() for
non-shadow requests, shouldn't blk_mq_sched_get_request() call
blk_queue_enter_live() only if __blk_mq_alloc_request() is called?

Thanks,

Bart.

\
 
 \ /
  Last update: 2016-12-13 15:46    [W:0.277 / U:2.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site