[PATCH 1/3] blk-mq: fix tags leak when shrink nr_hw_queues

[PATCH 1/3] blk-mq: fix tags leak when shrink nr_hw_queues

[chengming.zhou]

From: Chengming Zhou <zhouchengming@bytedance.com>

Although we don't need to realloc set->tags[] when shrink nr_hw_queues,
we need to free them. Or these tags will be leaked.

How to reproduce:
1. mount -t configfs configfs /mnt
2. modprobe null_blk nr_devices=0 submit_queues=8
3. mkdir /mnt/nullb/nullb0
4. echo 1 > /mnt/nullb/nullb0/power
5. echo 4 > /mnt/nullb/nullb0/submit_queues
6. rmdir /mnt/nullb/nullb0

In step 4, will alloc 9 tags (8 submit queues and 1 poll queue), then
in step 5, new_nr_hw_queues = 5 (4 submit queues and 1 poll queue).
At last in step 6, only these 5 tags are freed, the other 4 tags leaked.

Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 block/blk-mq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index f14b8669ac69..8af29a3b1400 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4386,9 +4386,13 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 				       int new_nr_hw_queues)
 {
 	struct blk_mq_tags **new_tags;
+	int i;
 
-	if (set->nr_hw_queues >= new_nr_hw_queues)
+	if (set->nr_hw_queues >= new_nr_hw_queues) {
+		for (i = new_nr_hw_queues; i < set->nr_hw_queues; i++)
+			__blk_mq_free_map_and_rqs(set, i);
 		goto done;
+	}
 
 	new_tags = kcalloc_node(new_nr_hw_queues, sizeof(struct blk_mq_tags *),
 				GFP_KERNEL, set->numa_node);
-- 
2.41.0

[PATCH 2/3] blk-mq: delete redundant tagset map update when fallback

[chengming.zhou]

From: Chengming Zhou <zhouchengming@bytedance.com>

When we increase nr_hw_queues fail, the fallback path will use
blk_mq_update_queue_map() to clear and update all maps.
Obviously, this line of update of HCTX_TYPE_DEFAULT only is not
needed, so delete it.

Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 block/blk-mq.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8af29a3b1400..8d02bafff331 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4737,7 +4737,6 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 				__blk_mq_free_map_and_rqs(set, i);
 
 			set->nr_hw_queues = prev_nr_hw_queues;
-			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
 			goto fallback;
 		}
 		blk_mq_map_swqueue(q);
-- 
2.41.0

Re: [PATCH 2/3] blk-mq: delete redundant tagset map update when fallback

[Ming Lei]

On Mon, Aug 21, 2023 at 05:56:01PM +0800, chengming.zhou@linux.dev wrote:
> From: Chengming Zhou <zhouchengming@bytedance.com>
> 
> When we increase nr_hw_queues fail, the fallback path will use
> blk_mq_update_queue_map() to clear and update all maps.
> Obviously, this line of update of HCTX_TYPE_DEFAULT only is not
> needed, so delete it.
> 
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

[PATCH 3/3] blk-mq: prealloc tags when increase tagset nr_hw_queues

[chengming.zhou]

From: Chengming Zhou <zhouchengming@bytedance.com>

Just like blk_mq_alloc_tag_set(), it's better to prepare all tags before
using to map to queue ctxs in blk_mq_map_swqueue(), which now have to
consider empty set->tags[].

The good point is that we can fallback easily if increasing nr_hw_queues
fail, instead of just mapping to hctx[0] when fail in blk_mq_map_swqueue().

And the fallback path already has tags free & clean handling, so all
is good.

Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 block/blk-mq.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8d02bafff331..84adb3657b30 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4404,6 +4404,16 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 		       sizeof(*set->tags));
 	kfree(set->tags);
 	set->tags = new_tags;
+
+	for (i = set->nr_hw_queues; i < new_nr_hw_queues; i++) {
+		if (!__blk_mq_alloc_map_and_rqs(set, i)) {
+			while (--i >= set->nr_hw_queues)
+				__blk_mq_free_map_and_rqs(set, i);
+			return -ENOMEM;
+		}
+		cond_resched();
+	}
+
 done:
 	set->nr_hw_queues = new_nr_hw_queues;
 	return 0;
-- 
2.41.0

Re: [PATCH 3/3] blk-mq: prealloc tags when increase tagset nr_hw_queues

[Ming Lei]

On Mon, Aug 21, 2023 at 05:56:02PM +0800, chengming.zhou@linux.dev wrote:
> From: Chengming Zhou <zhouchengming@bytedance.com>
> 
> Just like blk_mq_alloc_tag_set(), it's better to prepare all tags before
> using to map to queue ctxs in blk_mq_map_swqueue(), which now have to
> consider empty set->tags[].
> 
> The good point is that we can fallback easily if increasing nr_hw_queues
> fail, instead of just mapping to hctx[0] when fail in blk_mq_map_swqueue().
> 
> And the fallback path already has tags free & clean handling, so all
> is good.
> 
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [PATCH 1/3] blk-mq: fix tags leak when shrink nr_hw_queues

[Ming Lei]

On Mon, Aug 21, 2023 at 05:56:00PM +0800, chengming.zhou@linux.dev wrote:
> From: Chengming Zhou <zhouchengming@bytedance.com>
> 
> Although we don't need to realloc set->tags[] when shrink nr_hw_queues,
> we need to free them. Or these tags will be leaked.
> 
> How to reproduce:
> 1. mount -t configfs configfs /mnt
> 2. modprobe null_blk nr_devices=0 submit_queues=8
> 3. mkdir /mnt/nullb/nullb0
> 4. echo 1 > /mnt/nullb/nullb0/power
> 5. echo 4 > /mnt/nullb/nullb0/submit_queues
> 6. rmdir /mnt/nullb/nullb0
> 
> In step 4, will alloc 9 tags (8 submit queues and 1 poll queue), then
> in step 5, new_nr_hw_queues = 5 (4 submit queues and 1 poll queue).
> At last in step 6, only these 5 tags are freed, the other 4 tags leaked.
> 
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [PATCH 1/3] blk-mq: fix tags leak when shrink nr_hw_queues

[Jens Axboe]

On Mon, 21 Aug 2023 17:56:00 +0800, chengming.zhou@linux.dev wrote:
> Although we don't need to realloc set->tags[] when shrink nr_hw_queues,
> we need to free them. Or these tags will be leaked.
> 
> How to reproduce:
> 1. mount -t configfs configfs /mnt
> 2. modprobe null_blk nr_devices=0 submit_queues=8
> 3. mkdir /mnt/nullb/nullb0
> 4. echo 1 > /mnt/nullb/nullb0/power
> 5. echo 4 > /mnt/nullb/nullb0/submit_queues
> 6. rmdir /mnt/nullb/nullb0
> 
> [...]

Applied, thanks!

[1/3] blk-mq: fix tags leak when shrink nr_hw_queues
      commit: e1dd7bc93029024af5688253b0c05181d6e01f8e
[2/3] blk-mq: delete redundant tagset map update when fallback
      commit: 2bc4d7a355a4d617452eaf1b21d6d261194b3667
[3/3] blk-mq: prealloc tags when increase tagset nr_hw_queues
      commit: 7222657e51b5626d10154b3e48ad441c33b5da96

Best regards,
-- 
Jens Axboe