blk-mq: dynamic h/w context count

PATCH New Keith Busch 4 participants 7 messages Dec 18, 2015 – Feb 9, 2016

Subsystems (2)

Block layer NVMe

lore

Share & tools

lore

Keith Busch

author

2015-12-18 00:08 UTC #0 lore raw

PATCH 1/2 blk-mq: dynamic h/w context count

The hardware's provided queue count may change at runtime with resource provisioning. This patch allows a block driver to alter the number of h/w queues available when its resource count changes.

The main part is a new blk-mq API to request a new number of h/w queues for a given live tag set. The new API freezes all queues using that set, then adjusts the allocated count prior to remapping these to CPUs.

The bulk of the rest just shifts where h/w contexts and all their artifacts are allocated and freed.

The number of max h/w contexts is capped to the number of possible cpus since there is no use for more than that. As such, all pre-allocated memory for pointers need to account for the max possible rather than the initial number of queues.

A side effect of this is that the blk-mq will proceed successfully as long as it can allocate at least one h/w context. Previously it would fail request queue initialization if less than the requested number was allocated.

Signed-off-by Keith Busch <keith.busch at intel.com>

---
 block/blk-mq-sysfs.c   |   9 +--
 block/blk-mq.c         | 173 +++++++++++++++++++++++++++++--------------------
 block/blk-mq.h         |   1 +
 include/linux/blk-mq.h |   2 +
 4 files changed, 112 insertions(+), 73 deletions(-)

block/blk-mq-sysfs.c +5 -4

index 1cf1878..431fdda 100644
@@ -408,17 +408,18 @@ void blk_mq_unregister_disk(struct gendisk *disk)
 	blk_mq_enable_hotplug();
 }
 
+void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
+{
+	kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
+}
+
 static void blk_mq_sysfs_init(struct request_queue *q)
 {
-	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_ctx *ctx;
 	int i;
 
 	kobject_init(&q->mq_kobj, &blk_mq_ktype);
 
-	queue_for_each_hw_ctx(q, hctx, i)
-		kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
-
 	queue_for_each_ctx(q, ctx, i)
 		kobject_init(&ctx->kobj, &blk_mq_ctx_ktype);
 }

block/blk-mq.c +104 ~~-69~~

243 lines not on this page · raw message

block/blk-mq.h +1 -0

index eaede8e..9087b11 100644
@@ -57,6 +57,7 @@ extern int blk_mq_hw_queue_to_node(unsigned int *map, unsigned int);
  */
 extern int blk_mq_sysfs_register(struct request_queue *q);
 extern void blk_mq_sysfs_unregister(struct request_queue *q);
+extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
 
 extern void blk_mq_rq_timed_out(struct request *req, bool reserved);

include/linux/blk-mq.h +2 -0

index 7fc9296..15a73d4 100644
@@ -244,6 +244,8 @@ void blk_mq_freeze_queue(struct request_queue *q);
 void blk_mq_unfreeze_queue(struct request_queue *q);
 void blk_mq_freeze_queue_start(struct request_queue *q);
 
+void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
+
 /*
  * Driver command data is immediately after the request. So subtract request
  * size to get back to the original request, add request size to get the PDU.

Signature · 1 line

2.6.2.307.g37023ba

Keith Busch

author

2015-12-18 00:08 UTC #1 lore raw

PATCH 2/2 NVMe: Fix possible queue use after freed

This notifies blk-mq when the tag set contains a different number of queues prior to freeing unused ones that the request queue points to.

Signed-off-by Keith Busch <keith.busch at intel.com>

---
 drivers/nvme/host/pci.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/nvme/host/pci.c +9 -5

index b82bbea..b03631c 100644
@@ -1392,7 +1392,7 @@ static int nvme_kthread(void *data)
 
 static int nvme_create_io_queues(struct nvme_dev *dev)
 {
-	unsigned i;
+	unsigned i, max;
 	int ret = 0;
 
 	for (i = dev->queue_count; i <= dev->max_qid; i++) {

@@ -1402,7 +1402,8 @@ static int nvme_create_io_queues(struct nvme_dev *dev)
 		}
 	}
 
-	for (i = dev->online_queues; i <= dev->queue_count - 1; i++) {
+	max = min(dev->max_qid, dev->queue_count - 1);
+	for (i = dev->online_queues; i <= max; i++) {
 		ret = nvme_create_queue(dev->queues[i], i);
 		if (ret) {
 			nvme_free_queues(dev, i);

@@ -1559,9 +1560,6 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 		adminq->cq_vector = -1;
 		goto free_queues;
 	}
-
-	/* Free previously allocated queues that are no longer usable */
-	nvme_free_queues(dev, nr_io_queues + 1);
 	return nvme_create_io_queues(dev);
 
  free_queues:

@@ -1617,7 +1615,13 @@ static int nvme_dev_add(struct nvme_dev *dev)
 		if (blk_mq_alloc_tag_set(&dev->tagset))
 			return 0;
 		dev->ctrl.tagset = &dev->tagset;
+	} else {
+		blk_mq_update_nr_hw_queues(&dev->tagset, dev->online_queues - 1);
+
+		/* Free previously allocated queues that are no longer usable */
+		nvme_free_queues(dev, dev->online_queues);
 	}
+
 	queue_work(nvme_workq, &dev->scan_work);
 	return 0;
 }

Signature · 1 line

2.6.2.307.g37023ba

Christoph Hellwig

2015-12-24 14:23 UTC #2 lore raw

PATCH 2/2 NVMe: Fix possible queue use after freed

Looks good:

Reviewed-by Christoph Hellwig <hch at lst.de>

Jon Derrick

2016-02-08 19:28 UTC #3 lore raw

PATCH 2/2 NVMe: Fix possible queue use after freed

In reply to Keith Busch

Hi Jens,

This set was acked a while back by Christoph and reviewed by me. I just tested it again and it still works well against 4.5-rc3.

Is there any chance we can get this in for 4.6?

Cheers

Jens Axboe

2016-02-09 19:39 UTC #4 lore raw

PATCH 2/2 NVMe: Fix possible queue use after freed

Quoted 6 lines · Jon Derrick

Hi Jens,
This set was acked a while back by Christoph and reviewed by me. I just tested it again and it still works well against 4.5-rc3.
Is there any chance we can get this in for 4.6?

Yes, it will go into 4.6.

Signature · 1 line

Jens Axboe

Christoph Hellwig

2015-12-24 14:22 UTC #5 lore raw

PATCH 1/2 blk-mq: dynamic h/w context count

In reply to Keith Busch

Looks reasonable to me:

Reviewed-by Christoph Hellwig <hch at lst.de>

Jon Derrick

2016-01-15 18:22 UTC #6 lore raw

PATCH 1/2 blk-mq: dynamic h/w context count

In reply to Keith Busch

Quoted 16 lines · Keith Busch

The hardware's provided queue count may change at runtime with resource provisioning. This patch allows a block driver to alter the number of h/w queues available when its resource count changes.
The main part is a new blk-mq API to request a new number of h/w queues for a given live tag set. The new API freezes all queues using that set, then adjusts the allocated count prior to remapping these to CPUs.
The bulk of the rest just shifts where h/w contexts and all their artifacts are allocated and freed.
The number of max h/w contexts is capped to the number of possible cpus since there is no use for more than that. As such, all pre-allocated memory for pointers need to account for the max possible rather than the initial number of queues.

Quoted 7 lines

A side effect of this is that the blk-mq will proceed successfully as long as it can allocate at least one h/w context. Previously it would fail request queue initialization if less than the requested number was allocated.
Signed-off-by Keith Busch <keith.busch at intel.com>
---

Looks good, works well,
Tested-by: Jon Derrick <jonathan.derrick at intel.com>