On 11/06/2020 09:26, John Garry wrote:
On 11/06/2020 03:57, Ming Lei wrote:
On Thu, Jun 11, 2020 at 01:29:09AM +0800, John Garry wrote:
From: Hannes Reinecke <hare@xxxxxxx>
The function does not set the depth, but rather transitions from
shared to non-shared queues and vice versa.
So rename it to blk_mq_update_tag_set_shared() to better reflect
its purpose.
It is fine to rename it for me, however:
This patch claims to rename blk_mq_update_tag_set_shared(), but also
change blk_mq_init_bitmap_tags's signature.
I was going to update the commit message here, but forgot again...
So suggest to split this patch into two or add comment log on changing
blk_mq_init_bitmap_tags().
I think I'll just split into 2x commits.
Hi Hannes,
Do you have any issue with splitting the undocumented changes into
another patch as so:
-------------------->8---------------------
From db3f8ec1295efbf53273ffc137d348857cbd411e Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@xxxxxxx>
Date: Tue, 23 Jun 2020 12:07:33 +0100
Subject: [PATCH] blk-mq: Free tags in blk_mq_init_tags() upon error
Since the tags are allocated in blk_mq_init_tags() it's better practice
to free in that same function upon error, rather than a callee which is
to init the bitmap tags - blk_mq_init_tags().
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
[jpg: split from an earlier patch with a new commit message, minor mod
to return NULL directly for error]
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 1085dc9848f3..b8972014cd90 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -487,24 +487,22 @@ static int bt_alloc(struct sbitmap_queue *bt,
unsigned int depth,
node);
}
-static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct blk_mq_tags
*tags,
- int node, int alloc_policy)
+static int blk_mq_init_bitmap_tags(struct blk_mq_tags *tags,
+ int node, int alloc_policy)
{
unsigned int depth = tags->nr_tags - tags->nr_reserved_tags;
bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR;
if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node))
- goto free_tags;
+ return -ENOMEM;
if (bt_alloc(&tags->breserved_tags, tags->nr_reserved_tags, round_robin,
node))
goto free_bitmap_tags;
- return tags;
+ return 0;
free_bitmap_tags:
sbitmap_queue_free(&tags->bitmap_tags);
-free_tags:
- kfree(tags);
- return NULL;
+ return -ENOMEM;
}
struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
@@ -525,7 +523,11 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int
total_tags,
tags->nr_tags = total_tags;
tags->nr_reserved_tags = reserved_tags;
- return blk_mq_init_bitmap_tags(tags, node, alloc_policy);
+ if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) {
+ kfree(tags);
+ return NULL;
+ }
+ return tags;
}
void blk_mq_free_tags(struct blk_mq_tags *tags)
--------------------8<---------------------
Thanks
