[PATCH v2 0/8] blk-mq: Minor cleanups

[PATCH v2 0/8] blk-mq: Minor cleanups

[Alexander Gordeev]

Hello,

The series is against:

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

Thanks!

CC: linux-block@vger.kernel.org

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(-)

-- 
1.8.3.1

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c         | 2 --
 include/linux/blkdev.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 0be5577..c96a168 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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))
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c47c358..0f1bf55 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -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;
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[Alexander Gordeev]

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

CC: linux-block@vger.kernel.org
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c96a168..92c2519 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 92c2519..e3e9b23 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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])
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e3e9b23..eed6e348 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
 }
 
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 37 +++++++++++--------------------------
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index eed6e348..15c03c2 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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 */
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[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: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 15c03c2..4b07901 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
-- 
1.8.3.1

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

[Sagi Grimberg]

> @@ -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];

Didn't you mean hctx[j]?

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

[Alexander Gordeev]

On Thu, Oct 06, 2016 at 12:47:26AM +0300, Sagi Grimberg wrote:
> 
> >@@ -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];
> 
> Didn't you mean hctx[j]?

Surely, I did.
Thanks!

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

[Sagi Grimberg]

On 06/10/16 11:25, Alexander Gordeev wrote:
> On Thu, Oct 06, 2016 at 12:47:26AM +0300, Sagi Grimberg wrote:
>>
>>> @@ -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];
>>
>> Didn't you mean hctx[j]?
>
> Surely, I did.
> Thanks!

Maybe it would be cleaner to do:

	while (--i >= 0) {
		hctx = hctxs[i];
		...
	}

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

[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: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 15c03c2..d2c11fc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
-- 
1.8.3.1

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq-sysfs.c | 5 +++++
 block/blk-mq.c       | 2 +-
 block/blk-mq.h       | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 01fb455..11846d0 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -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;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4b07901..78ee5af 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index e5d2524..3d53818 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -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);
 
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

[Alexander Gordeev]

CC: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq-sysfs.c | 5 +++++
 block/blk-mq.c       | 2 +-
 block/blk-mq.h       | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 01fb455..11846d0 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -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;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d2c11fc..53f8663 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index e5d2524..3d53818 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -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);
 
-- 
1.8.3.1

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

[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: linux-block@vger.kernel.org
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 block/blk-mq.c | 51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 78ee5af..03654af 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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);
-- 
1.8.3.1

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

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>