| Message ID | 20200516153430.294324-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] blk-mq: move the call to blk_queue_enter_live out of blk_mq_get_request | expand |
On 2020-05-16 08:34, Christoph Hellwig wrote: > No need for two queue references. Also reduce the q_usage_counter > critical section to just the actual request allocation. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/blk-mq.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index d96d3931f33e6..69e58cc4244c0 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -439,26 +439,20 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, > if (hctx_idx >= q->nr_hw_queues) > return ERR_PTR(-EIO); > > - ret = blk_queue_enter(q, flags); > - if (ret) > - return ERR_PTR(ret); > - > /* > * Check if the hardware context is actually mapped to anything. > * If not tell the caller that it should skip this queue. > */ > alloc_data.hctx = q->queue_hw_ctx[hctx_idx]; > - if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) { > - blk_queue_exit(q); > + if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) > return ERR_PTR(-EXDEV); > - } > cpu = cpumask_first_and(alloc_data.hctx->cpumask, cpu_online_mask); > alloc_data.ctx = __blk_mq_get_ctx(q, cpu); > > - blk_queue_enter_live(q); > + ret = blk_queue_enter(q, flags); > + if (ret) > + return ERR_PTR(ret); > rq = blk_mq_get_request(q, NULL, &alloc_data); > - blk_queue_exit(q); > - > if (!rq) { > blk_queue_exit(q); > return ERR_PTR(-EWOULDBLOCK); This change looks wrong to me. blk_mq_update_nr_hw_queues() modifies q->queue_hw_ctx so q_usage_counter must be incremented before that pointer is dereferenced. Bart.
On Sat, May 16, 2020 at 09:28:22AM -0700, Bart Van Assche wrote: > This change looks wrong to me. blk_mq_update_nr_hw_queues() modifies > q->queue_hw_ctx so q_usage_counter must be incremented before that > pointer is dereferenced. True, I'll drop that part.
diff --git a/block/blk-mq.c b/block/blk-mq.c index d96d3931f33e6..69e58cc4244c0 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -439,26 +439,20 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, if (hctx_idx >= q->nr_hw_queues) return ERR_PTR(-EIO); - ret = blk_queue_enter(q, flags); - if (ret) - return ERR_PTR(ret); - /* * Check if the hardware context is actually mapped to anything. * If not tell the caller that it should skip this queue. */ alloc_data.hctx = q->queue_hw_ctx[hctx_idx]; - if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) { - blk_queue_exit(q); + if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) return ERR_PTR(-EXDEV); - } cpu = cpumask_first_and(alloc_data.hctx->cpumask, cpu_online_mask); alloc_data.ctx = __blk_mq_get_ctx(q, cpu); - blk_queue_enter_live(q); + ret = blk_queue_enter(q, flags); + if (ret) + return ERR_PTR(ret); rq = blk_mq_get_request(q, NULL, &alloc_data); - blk_queue_exit(q); - if (!rq) { blk_queue_exit(q); return ERR_PTR(-EWOULDBLOCK);
No need for two queue references. Also reduce the q_usage_counter critical section to just the actual request allocation. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-mq.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)