Skip to contentSkip to conversation

blk-mq: Minor cleanups

PATCH In review Alexander Gordeev 2 participants 21 messages Sep 30, 2016 – Oct 9, 2016 Block layer

lore
Share & tools
lore

Alexander Gordeev

author
#0 lore raw

v2 cover blk-mq: Minor cleanups

Hello,

The series is against:

git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next

Thanks!

CC [email protected]

Alexander Gordeev (8):
block: Get rid of unused request_queue::nr_queues member
blk-mq: Remove a redundant assignment
blk-mq: Fix hardware context data node selection
blk-mq: Cleanup a loop exit condition
blk-mq: Cleanup blk_mq_hw_ctx::cpumask (de-)allocation
blk-mq: Rework blk_mq_realloc_hw_ctxs()
blk-mq: Pair blk_mq_hctx_kobj_init() with blk_mq_hctx_kobj_put()
blk-mq: Cleanup (de-)allocation of blk_mq_hw_ctx::ctxs

 block/blk-mq-sysfs.c   |   5 ++
 block/blk-mq.c         | 129 ++++++++++++++++++++++++-------------------------
 block/blk-mq.h         |   1 +
 include/linux/blkdev.h |   1 -
 4 files changed, 68 insertions(+), 68 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v2 1/8 block: Get rid of unused request_queue::nr_queues member

CC [email protected]
Reviewed-by Omar Sandoval <[email protected]>
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c         | 2 --
 include/linux/blkdev.h | 1 -
 2 files changed, 3 deletions(-)
block/blk-mq.c +0 -2
index 0be5577..c96a168 100644
@@ -1983,8 +1983,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	INIT_WORK(&q->timeout_work, blk_mq_timeout_work);
 	blk_queue_rq_timeout(q, set->timeout ? set->timeout : 30 * HZ);
 
-	q->nr_queues = nr_cpu_ids;
-
 	q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
 
 	if (!(set->flags & BLK_MQ_F_SG_MERGE))
include/linux/blkdev.h +0 -1
index c47c358..0f1bf55 100644
@@ -325,7 +325,6 @@ struct request_queue {
 
 	/* sw queues */
 	struct blk_mq_ctx __percpu	*queue_ctx;
-	unsigned int		nr_queues;
 
 	/* hw dispatch queues */
 	struct blk_mq_hw_ctx	**queue_hw_ctx;
Signature · 1 line
1.8.3.1
author
#3 lore raw

PATCH v2 2/8 blk-mq: Remove a redundant assignment

In reply to Alexander Gordeev

blk_mq_hw_ctx::queue_num is initialized in blk_mq_init_hctx() function.

CC [email protected]
Reviewed-by Omar Sandoval <[email protected]>
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 1 -
 1 file changed, 1 deletion(-)
block/blk-mq.c +0 -1
index c96a168..92c2519 100644
@@ -1928,7 +1928,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 
 		atomic_set(&hctxs[i]->nr_active, 0);
 		hctxs[i]->numa_node = node;
-		hctxs[i]->queue_num = i;
 
 		if (blk_mq_init_hctx(q, set, hctxs[i], i)) {
 			free_cpumask_var(hctxs[i]->cpumask);
Signature · 1 line
1.8.3.1
author
#5 lore raw

PATCH v2 3/8 blk-mq: Fix hardware context data node selection

In reply to Alexander Gordeev
CC [email protected]
Reviewed-by Omar Sandoval <[email protected]>
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
block/blk-mq.c +4 -5
index 92c2519..e3e9b23 100644
@@ -1633,13 +1633,9 @@ static int blk_mq_init_hctx(struct request_queue *q,
 		struct blk_mq_tag_set *set,
 		struct blk_mq_hw_ctx *hctx, unsigned hctx_idx)
 {
-	int node;
+	int node = hctx->numa_node;
 	unsigned flush_start_tag = set->queue_depth;
 
-	node = hctx->numa_node;
-	if (node == NUMA_NO_NODE)
-		node = hctx->numa_node = set->numa_node;
-
 	INIT_WORK(&hctx->run_work, blk_mq_run_work_fn);
 	INIT_DELAYED_WORK(&hctx->delay_work, blk_mq_delay_work_fn);
 	spin_lock_init(&hctx->lock);

@@ -1914,6 +1910,9 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 			continue;
 
 		node = blk_mq_hw_queue_to_node(q->mq_map, i);
+		if (node == NUMA_NO_NODE)
+			node = set->numa_node;
+
 		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
 					GFP_KERNEL, node);
 		if (!hctxs[i])
Signature · 1 line
1.8.3.1
author
#7 lore raw

PATCH v2 4/8 blk-mq: Cleanup a loop exit condition

In reply to Alexander Gordeev
CC [email protected]
Reviewed-by Omar Sandoval <[email protected]>
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
block/blk-mq.c +3 -6
index e3e9b23..eed6e348 100644
@@ -1607,16 +1607,13 @@ static void blk_mq_exit_hctx(struct request_queue *q,
 }
 
 static void blk_mq_exit_hw_queues(struct request_queue *q,
-		struct blk_mq_tag_set *set, int nr_queue)
+		struct blk_mq_tag_set *set)
 {
 	struct blk_mq_hw_ctx *hctx;
 	unsigned int i;
 
-	queue_for_each_hw_ctx(q, hctx, i) {
-		if (i == nr_queue)
-			break;
+	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_exit_hctx(q, set, hctx, i);
-	}
 }
 
 static void blk_mq_free_hw_queues(struct request_queue *q,

@@ -2039,7 +2036,7 @@ void blk_mq_free_queue(struct request_queue *q)
 
 	blk_mq_del_queue_tag_set(q);
 
-	blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
+	blk_mq_exit_hw_queues(q, set);
 	blk_mq_free_hw_queues(q, set);
 }
Signature · 1 line
1.8.3.1
author
#9 lore raw

PATCH v2 5/8 blk-mq: Cleanup blk_mq_hw_ctx::cpumask (de-)allocation

In reply to Alexander Gordeev
CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 37 +++++++++++--------------------------
 1 file changed, 11 insertions(+), 26 deletions(-)
block/blk-mq.c +11 -26
index eed6e348..15c03c2 100644
@@ -1604,6 +1604,8 @@ static void blk_mq_exit_hctx(struct request_queue *q,
 	blk_mq_remove_cpuhp(hctx);
 	blk_free_flush_queue(hctx->fq);
 	sbitmap_free(&hctx->ctx_map);
+
+	free_cpumask_var(hctx->cpumask);
 }
 
 static void blk_mq_exit_hw_queues(struct request_queue *q,

@@ -1616,27 +1618,21 @@ static void blk_mq_exit_hw_queues(struct request_queue *q,
 		blk_mq_exit_hctx(q, set, hctx, i);
 }
 
-static void blk_mq_free_hw_queues(struct request_queue *q,
-		struct blk_mq_tag_set *set)
-{
-	struct blk_mq_hw_ctx *hctx;
-	unsigned int i;
-
-	queue_for_each_hw_ctx(q, hctx, i)
-		free_cpumask_var(hctx->cpumask);
-}
-
 static int blk_mq_init_hctx(struct request_queue *q,
 		struct blk_mq_tag_set *set,
-		struct blk_mq_hw_ctx *hctx, unsigned hctx_idx)
+		struct blk_mq_hw_ctx *hctx, unsigned hctx_idx, int node)
 {
-	int node = hctx->numa_node;
 	unsigned flush_start_tag = set->queue_depth;
 
+	if (!zalloc_cpumask_var_node(&hctx->cpumask, GFP_KERNEL, node))
+		goto err_cpumask;
+
 	INIT_WORK(&hctx->run_work, blk_mq_run_work_fn);
 	INIT_DELAYED_WORK(&hctx->delay_work, blk_mq_delay_work_fn);
 	spin_lock_init(&hctx->lock);
 	INIT_LIST_HEAD(&hctx->dispatch);
+	atomic_set(&hctx->nr_active, 0);
+	hctx->numa_node = node;
 	hctx->queue = q;
 	hctx->queue_num = hctx_idx;
 	hctx->flags = set->flags & ~BLK_MQ_F_TAG_SHARED;

@@ -1687,6 +1683,8 @@ static int blk_mq_init_hctx(struct request_queue *q,
 	kfree(hctx->ctxs);
  unregister_cpu_notifier:
 	blk_mq_remove_cpuhp(hctx);
+	free_cpumask_var(hctx->cpumask);
+ err_cpumask:
 	return -1;
 }
 

@@ -1915,18 +1913,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (!hctxs[i])
 			break;
 
-		if (!zalloc_cpumask_var_node(&hctxs[i]->cpumask, GFP_KERNEL,
-						node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
-			break;
-		}
-
-		atomic_set(&hctxs[i]->nr_active, 0);
-		hctxs[i]->numa_node = node;
-
-		if (blk_mq_init_hctx(q, set, hctxs[i], i)) {
-			free_cpumask_var(hctxs[i]->cpumask);
+		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
 			kfree(hctxs[i]);
 			hctxs[i] = NULL;
 			break;

@@ -1942,7 +1929,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 				set->tags[j] = NULL;
 			}
 			blk_mq_exit_hctx(q, set, hctx, j);
-			free_cpumask_var(hctx->cpumask);
 			kobject_put(&hctx->kobj);
 			kfree(hctx->ctxs);
 			kfree(hctx);

@@ -2037,7 +2023,6 @@ void blk_mq_free_queue(struct request_queue *q)
 	blk_mq_del_queue_tag_set(q);
 
 	blk_mq_exit_hw_queues(q, set);
-	blk_mq_free_hw_queues(q, set);
 }
 
 /* Basically redo blk_mq_init_queue with queue frozen */
Signature · 1 line
1.8.3.1
author
#11 lore raw

PATCH v2 6/8 blk-mq: Rework blk_mq_realloc_hw_ctxs()

In reply to Alexander Gordeev

Rework blk_mq_realloc_hw_ctxs() so deallocation is done in order reverse to allocation and indentation is bit more easy to read.

CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)
block/blk-mq.c +22 -18
index 15c03c2..4b07901 100644
@@ -1895,6 +1895,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 						struct request_queue *q)
 {
 	int i, j;
+	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_hw_ctx **hctxs = q->queue_hw_ctx;
 
 	blk_mq_sysfs_unregister(q);

@@ -1908,33 +1909,36 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
-					GFP_KERNEL, node);
-		if (!hctxs[i])
+		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		if (!hctx)
 			break;
 
-		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
+		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
+			kfree(hctx);
 			break;
 		}
-		blk_mq_hctx_kobj_init(hctxs[i]);
+
+		blk_mq_hctx_kobj_init(hctx);
+		hctxs[i] = hctx;
 	}
 	for (j = i; j < q->nr_hw_queues; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		hctx = hctxs[i];
 
-		if (hctx) {
-			if (hctx->tags) {
-				blk_mq_free_rq_map(set, hctx->tags, j);
-				set->tags[j] = NULL;
-			}
-			blk_mq_exit_hctx(q, set, hctx, j);
-			kobject_put(&hctx->kobj);
-			kfree(hctx->ctxs);
-			kfree(hctx);
-			hctxs[j] = NULL;
+		if (!hctx)
+			continue;
 
+		hctxs[i] = NULL;
+		kobject_put(&hctx->kobj);
+
+		if (hctx->tags) {
+			blk_mq_free_rq_map(set, hctx->tags, j);
+			set->tags[j] = NULL;
 		}
+
+		blk_mq_exit_hctx(q, set, hctx, j);
+
+		kfree(hctx->ctxs);
+		kfree(hctx);
 	}
 	q->nr_hw_queues = i;
 	blk_mq_sysfs_register(q);
Signature · 1 line
1.8.3.1
#12 lore raw

Re: [PATCH v2 6/8] blk-mq: Rework blk_mq_realloc_hw_ctxs()

Quoted 17 lines
block/blk-mq.c +4 -6
@@ -1908,33 +1909,36 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
-					GFP_KERNEL, node);
-		if (!hctxs[i])
+		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		if (!hctx)
 			break;
 
-		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
+		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
+			kfree(hctx);

block/blk-mq.c static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set, · line 1925

Quoted 10 lines
block/blk-mq.c +4 -2
 			break;
 		}
-		blk_mq_hctx_kobj_init(hctxs[i]);
+
+		blk_mq_hctx_kobj_init(hctx);
+		hctxs[i] = hctx;
 	}
 	for (j = i; j < q->nr_hw_queues; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		hctx = hctxs[i];

Didn't you mean hctx[j]?

author
#13 lore raw

Re: [PATCH v2 6/8] blk-mq: Rework blk_mq_realloc_hw_ctxs()

Quoted 21 lines · Sagi Grimberg
Quoted 21 lines
block/blk-mq.c +5 -7
@@ -1908,33 +1909,36 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
-					GFP_KERNEL, node);
-		if (!hctxs[i])
+		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		if (!hctx)
 			break;
 
-		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
+		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
+			kfree(hctx);
 			break;
 		}
-		blk_mq_hctx_kobj_init(hctxs[i]);
+
Quoted 8 lines

block/blk-mq.c static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set, · line 1925

Quoted 6 lines
block/blk-mq.c +3 -1
+		blk_mq_hctx_kobj_init(hctx);
+		hctxs[i] = hctx;
 	}
 	for (j = i; j < q->nr_hw_queues; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		hctx = hctxs[i];

Didn't you mean hctx[j]?

Surely, I did.
Thanks!

#14 lore raw

Re: [PATCH v2 6/8] blk-mq: Rework blk_mq_realloc_hw_ctxs()

Quoted 31 lines · Alexander Gordeev
Quoted 21 lines · Sagi Grimberg
Quoted 21 lines
block/blk-mq.c +5 -7
@@ -1908,33 +1909,36 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
-					GFP_KERNEL, node);
-		if (!hctxs[i])
+		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		if (!hctx)
 			break;
 
-		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
+		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
+			kfree(hctx);
 			break;
 		}
-		blk_mq_hctx_kobj_init(hctxs[i]);
+
Quoted 8 lines

block/blk-mq.c static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set, · line 1925

Quoted 6 lines
block/blk-mq.c +3 -1
+		blk_mq_hctx_kobj_init(hctx);
+		hctxs[i] = hctx;
 	}
 	for (j = i; j < q->nr_hw_queues; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		hctx = hctxs[i];

Didn't you mean hctx[j]?

Quoted 2 lines

Surely, I did.
Thanks!

Maybe it would be cleaner to do:

	while (--i >= 0) {
		hctx = hctxs[i];
		...
	}
author
#15 lore raw

PATCH v3 6/8 blk-mq: Rework blk_mq_realloc_hw_ctxs()

In reply to Sagi Grimberg

Rework blk_mq_realloc_hw_ctxs() so deallocation is done in order reverse to allocation and indentation is bit more easy to read.

CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)
block/blk-mq.c +22 -18
index 15c03c2..d2c11fc 100644
@@ -1895,6 +1895,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 						struct request_queue *q)
 {
 	int i, j;
+	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_hw_ctx **hctxs = q->queue_hw_ctx;
 
 	blk_mq_sysfs_unregister(q);

@@ -1908,33 +1909,36 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
-					GFP_KERNEL, node);
-		if (!hctxs[i])
+		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		if (!hctx)
 			break;
 
-		if (blk_mq_init_hctx(q, set, hctxs[i], i, node)) {
-			kfree(hctxs[i]);
-			hctxs[i] = NULL;
+		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
+			kfree(hctx);
 			break;
 		}
-		blk_mq_hctx_kobj_init(hctxs[i]);
+
+		blk_mq_hctx_kobj_init(hctx);
+		hctxs[i] = hctx;
 	}
 	for (j = i; j < q->nr_hw_queues; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		hctx = hctxs[j];
 
-		if (hctx) {
-			if (hctx->tags) {
-				blk_mq_free_rq_map(set, hctx->tags, j);
-				set->tags[j] = NULL;
-			}
-			blk_mq_exit_hctx(q, set, hctx, j);
-			kobject_put(&hctx->kobj);
-			kfree(hctx->ctxs);
-			kfree(hctx);
-			hctxs[j] = NULL;
+		if (!hctx)
+			continue;
 
+		hctxs[j] = NULL;
+		kobject_put(&hctx->kobj);
+
+		if (hctx->tags) {
+			blk_mq_free_rq_map(set, hctx->tags, j);
+			set->tags[j] = NULL;
 		}
+
+		blk_mq_exit_hctx(q, set, hctx, j);
+
+		kfree(hctx->ctxs);
+		kfree(hctx);
 	}
 	q->nr_hw_queues = i;
 	blk_mq_sysfs_register(q);
Signature · 1 line
1.8.3.1
author
#16 lore raw

PATCH v2 7/8 blk-mq: Pair blk_mq_hctx_kobj_init() with blk_mq_hctx_kobj_put()

In reply to Alexander Gordeev
CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq-sysfs.c | 5 +++++
 block/blk-mq.c       | 2 +-
 block/blk-mq.h       | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)
block/blk-mq-sysfs.c +5 -0
index 01fb455..11846d0 100644
@@ -429,6 +429,11 @@ void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
 	kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
 }
 
+void blk_mq_hctx_kobj_put(struct blk_mq_hw_ctx *hctx)
+{
+	kobject_put(&hctx->kobj);
+}
+
 static void blk_mq_sysfs_init(struct request_queue *q)
 {
 	struct blk_mq_ctx *ctx;
block/blk-mq.c +1 -1
index 4b07901..78ee5af 100644
@@ -1928,7 +1928,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 			continue;
 
 		hctxs[i] = NULL;
-		kobject_put(&hctx->kobj);
+		blk_mq_hctx_kobj_put(hctx);
 
 		if (hctx->tags) {
 			blk_mq_free_rq_map(set, hctx->tags, j);
block/blk-mq.h +1 -0
index e5d2524..3d53818 100644
@@ -53,6 +53,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
 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_hctx_kobj_put(struct blk_mq_hw_ctx *hctx);
 
 extern void blk_mq_rq_timed_out(struct request *req, bool reserved);
Signature · 1 line
1.8.3.1
author
#18 lore raw

PATCH v3 7/8 blk-mq: Pair blk_mq_hctx_kobj_init() with blk_mq_hctx_kobj_put()

CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq-sysfs.c | 5 +++++
 block/blk-mq.c       | 2 +-
 block/blk-mq.h       | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)
block/blk-mq-sysfs.c +5 -0
index 01fb455..11846d0 100644
@@ -429,6 +429,11 @@ void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
 	kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
 }
 
+void blk_mq_hctx_kobj_put(struct blk_mq_hw_ctx *hctx)
+{
+	kobject_put(&hctx->kobj);
+}
+
 static void blk_mq_sysfs_init(struct request_queue *q)
 {
 	struct blk_mq_ctx *ctx;
block/blk-mq.c +1 -1
index d2c11fc..53f8663 100644
@@ -1928,7 +1928,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 			continue;
 
 		hctxs[j] = NULL;
-		kobject_put(&hctx->kobj);
+		blk_mq_hctx_kobj_put(hctx);
 
 		if (hctx->tags) {
 			blk_mq_free_rq_map(set, hctx->tags, j);
block/blk-mq.h +1 -0
index e5d2524..3d53818 100644
@@ -53,6 +53,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
 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_hctx_kobj_put(struct blk_mq_hw_ctx *hctx);
 
 extern void blk_mq_rq_timed_out(struct request *req, bool reserved);
Signature · 1 line
1.8.3.1
author
#19 lore raw

PATCH v2 8/8 blk-mq: Cleanup (de-)allocation of blk_mq_hw_ctx::ctxs

In reply to Alexander Gordeev

Handling of blk_mq_hw_ctx::ctxs field (de-)allocation is confusing due to special treatment of the field introduced in commit c3b4afca7023 ("blk-mq: free hctx->ctxs in queue's release handler")'. Make it bit more readable by binding hctx and hctx->ctxs (de-)allocation.

CC [email protected]
Signed-off-by Alexander Gordeev <[email protected]>
---
 block/blk-mq.c | 51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)
block/blk-mq.c +32 -19
index 78ee5af..03654af 100644
@@ -1641,18 +1641,9 @@ static int blk_mq_init_hctx(struct request_queue *q,
 
 	hctx->tags = set->tags[hctx_idx];
 
-	/*
-	 * Allocate space for all possible cpus to avoid allocation at
-	 * runtime
-	 */
-	hctx->ctxs = kmalloc_node(nr_cpu_ids * sizeof(void *),
-					GFP_KERNEL, node);
-	if (!hctx->ctxs)
-		goto unregister_cpu_notifier;
-
 	if (sbitmap_init_node(&hctx->ctx_map, nr_cpu_ids, ilog2(8), GFP_KERNEL,
 			      node))
-		goto free_ctxs;
+		goto unregister_cpu_notifier;
 
 	hctx->nr_ctx = 0;
 

@@ -1679,8 +1670,6 @@ static int blk_mq_init_hctx(struct request_queue *q,
 		set->ops->exit_hctx(hctx, hctx_idx);
  free_bitmap:
 	sbitmap_free(&hctx->ctx_map);
- free_ctxs:
-	kfree(hctx->ctxs);
  unregister_cpu_notifier:
 	blk_mq_remove_cpuhp(hctx);
 	free_cpumask_var(hctx->cpumask);

@@ -1848,6 +1837,33 @@ static void blk_mq_add_queue_tag_set(struct blk_mq_tag_set *set,
 	mutex_unlock(&set->tag_list_lock);
 }
 
+static struct blk_mq_hw_ctx *alloc_hctx(int node)
+{
+	struct blk_mq_hw_ctx *hctx = kzalloc_node(sizeof(*hctx),
+					GFP_KERNEL, node);
+	if (!hctx)
+		return NULL;
+
+	/*
+	 * Allocate space for all possible cpus to avoid allocation at
+	 * runtime
+	 */
+	hctx->ctxs = kmalloc_node(nr_cpu_ids * sizeof(void *),
+					GFP_KERNEL, node);
+	if (!hctx->ctxs) {
+		kfree(hctx);
+		return NULL;
+	}
+
+	return hctx;
+}
+
+static void free_hctx(struct blk_mq_hw_ctx *hctx)
+{
+	kfree(hctx->ctxs);
+	kfree(hctx);
+}
+
 /*
  * It is the actual release handler for mq, but we do it from
  * request queue's release handler for avoiding use-after-free

@@ -1863,8 +1879,7 @@ void blk_mq_release(struct request_queue *q)
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (!hctx)
 			continue;
-		kfree(hctx->ctxs);
-		kfree(hctx);
+		free_hctx(hctx);
 	}
 
 	q->mq_map = NULL;

@@ -1909,12 +1924,12 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		if (node == NUMA_NO_NODE)
 			node = set->numa_node;
 
-		hctx = kzalloc_node(sizeof(*hctx), GFP_KERNEL, node);
+		hctx = alloc_hctx(node);
 		if (!hctx)
 			break;
 
 		if (blk_mq_init_hctx(q, set, hctx, i, node)) {
-			kfree(hctx);
+			free_hctx(hctx);
 			break;
 		}
 

@@ -1936,9 +1951,7 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 		}
 
 		blk_mq_exit_hctx(q, set, hctx, j);
-
-		kfree(hctx->ctxs);
-		kfree(hctx);
+		free_hctx(hctx);
 	}
 	q->nr_hw_queues = i;
 	blk_mq_sysfs_register(q);
Signature · 1 line
1.8.3.1