Hi Hannes,
+
+void blk_mq_exit_shared_sbitmap(struct blk_mq_tag_set *tag_set)
+{
+ sbitmap_queue_free(&tag_set->__bitmap_tags);
+ sbitmap_queue_free(&tag_set->__breserved_tags);
+}
+
+struct blk_mq_tags *blk_mq_init_tags(struct blk_mq_tag_set *set,
+ unsigned int total_tags,
unsigned int reserved_tags,
int node, int alloc_policy)
{
@@ -480,6 +506,7 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int
total_tags,
void blk_mq_free_tags(struct blk_mq_tags *tags)
{
+ /* Do not use tags->{bitmap, breserved}_tags */
sbitmap_queue_free(&tags->__bitmap_tags);
sbitmap_queue_free(&tags->__breserved_tags);
kfree(tags);
Hmm. This is essentially the same as blk_mq_exit_shared_sbitmap().
It does the same thing, but takes different argument types: struct
blk_mq_tag_set * vs struct blk_mq_tags *
Please call into that function or clarify the relationship between both.
And modify the comment; it's not about the 'use' but rather the freeing
ok
of the structures, and ->bitmap/breserved just point to the __ variants.
Please note that I had to change blk_mq_init_tags() from v5 to now
continue to init the blk_mq_tags.__{bitmap, breserved}_tags always. The
reason is that the following would now be broken for scheduler tags when
BLK_MQ_F_TAG_HCTX_SHARED is set, ***:
struct blk_mq_tags *blk_mq_init_tags(struct blk_mq_tag_set *set, ...)
{
struct blk_mq_tags *tags;
...
tags->nr_tags = total_tags;
tags->nr_reserved_tags = reserved_tags;
if (blk_mq_is_sbitmap_shared(set)) ***
return tags;
if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) {
...
}
return tags;
}
Keeping the blk_mq_is_sbitmap_shared() check would mean that we never
call blk_mq_init_bitmap_tags() for scheduler tags for when
BLK_MQ_F_TAG_HCTX_SHARED is set.
As such, I removed the blk_mq_is_sbitmap_shared() check, and we still
init the blk_mq_tags bitmaps always. Hence we still need to free them.
And as I noted in the hctx_tags_bitmap_show() change (patch #5), we
could actually reuse blk_mq_tags.__bitmap_tags.sb in that function
instead of using a local temp sbitmap.
@@ -538,6 +565,11 @@ int blk_mq_tag_update_depth(struct blk_mq_hw_ctx
*hctx,
return 0;
}
+void blk_mq_tag_resize_shared_sbitmap(struct blk_mq_tag_set *set,
unsigned int size)
+{
+ sbitmap_queue_resize(&set->__bitmap_tags, size -
set->reserved_tags);
+}
+
[...]
return 0;
+out_free_mq_rq_maps:
+ for (i = 0; i < set->nr_hw_queues; i++)
+ blk_mq_free_rq_map(set->tags[i]);
out_free_mq_map:
for (i = 0; i < set->nr_maps; i++) {
kfree(set->map[i].mq_map);
@@ -3170,6 +3189,9 @@ void blk_mq_free_tag_set(struct blk_mq_tag_set
*set)
for (i = 0; i < set->nr_hw_queues; i++)
blk_mq_free_map_and_requests(set, i);
+ if (blk_mq_is_sbitmap_shared(set))
+ blk_mq_exit_shared_sbitmap(set);
+
for (j = 0; j < set->nr_maps; j++) {
kfree(set->map[j].mq_map);
set->map[j].mq_map = NULL;
@@ -3206,6 +3228,8 @@ int blk_mq_update_nr_requests(struct
request_queue *q, unsigned int nr)
if (!hctx->sched_tags) {
ret = blk_mq_tag_update_depth(hctx, &hctx->tags, nr,
false);
+ if (!ret && blk_mq_is_sbitmap_shared(set))
+ blk_mq_tag_resize_shared_sbitmap(set, nr);
} else {
ret = blk_mq_tag_update_depth(hctx, &hctx->sched_tags,
nr, true);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 10bfdfb494fa..dde2d29f0ce5 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -158,6 +158,11 @@ struct blk_mq_alloc_data {
struct blk_mq_hw_ctx *hctx;
};
+static inline bool blk_mq_is_sbitmap_shared(struct blk_mq_tag_set
*tag_set)
+{
+ return tag_set->flags & BLK_MQ_F_TAG_HCTX_SHARED;
+}
+
When is that set?
It set later for SCSI hosts in scsi_mq_setup_tags(), for when
.host_tagset is set.
And to my understanding it contingent on the ->bitmap pointer _not_
pointing to ->__bitmap.
Can we use this as a test and drop this flag?
Maybe, but we need some flag passed to blk_mq_alloc_set() to request to
use the shared sbitmap.
Thanks,
John
