Skip to contentSkip to conversation

blk-mq: fix tags leak when shrink nr_hw_queues

PATCH Applied [email protected] 3 participants 7 messages Aug 21, 2023 – Aug 22, 2023 Block layer

lore
Share & tools
lore
#0 lore raw

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

From: Chengming Zhou <[email protected]>

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.

---
 block/blk-mq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
block/blk-mq.c +5 -1
index f14b8669ac69..8af29a3b1400 100644
@@ -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);
Signature · 1 line
2.41.0
#1 lore raw

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

From: Chengming Zhou <[email protected]>

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.

---
 block/blk-mq.c | 1 -
 1 file changed, 1 deletion(-)
block/blk-mq.c +0 -1
index 8af29a3b1400..8d02bafff331 100644
@@ -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);
Signature · 1 line
2.41.0
Reviewed-by on v1 2/3
#2 lore raw

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

Quoted 8 lines · On Mon, Aug 21, 2023 at 05:56:01PM +0800, [email protected] wrote:

From: Chengming Zhou <[email protected]>

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.

Signature · 1 line
Ming
#3 lore raw

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

In reply to [email protected]

From: Chengming Zhou <[email protected]>

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.

---
 block/blk-mq.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
block/blk-mq.c +10 -0
index 8d02bafff331..84adb3657b30 100644
@@ -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;
Signature · 1 line
2.41.0
Reviewed-by on v1 3/3
#4 lore raw

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

Quoted 6 lines · On Mon, Aug 21, 2023 at 05:56:02PM +0800, [email protected] wrote:

From: Chengming Zhou <[email protected]>

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[].

Quoted 7 lines

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.

Signature · 1 line
Ming
Reviewed-by on v1 1/3
#5 lore raw
In reply to [email protected]
Quoted 13 lines · On Mon, Aug 21, 2023 at 05:56:00PM +0800, [email protected] wrote:

From: Chengming Zhou <[email protected]>

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

Quoted 5 lines

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.

Signature · 1 line
Ming
maintainer Applied
#6 lore raw
In reply to [email protected]
Quoted 11 lines · On Mon, 21 Aug 2023 17:56:00 +0800, [email protected] 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

Quoted 1 line

[...]

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,

Signature · 1 line
Jens Axboe