On 12/3/19 3:30 PM, John Garry wrote: > On 02/12/2019 15:39, Hannes Reinecke wrote: >> 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. > > We will probably need to split this patch into two, as we're doing more > than just renaming. > > The prep patches could be picked up earlier if we're lucky. > > Thanks, > John > >> >> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> >> --- >> block/blk-mq-tag.c | 18 ++++++++++-------- >> block/blk-mq.c | 8 ++++---- >> 2 files changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index d7aa23c82dbf..f5009587e1b5 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -440,24 +440,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, >> @@ -478,7 +476,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); >> + tags = NULL; >> + } >> + return tags; >> } >> void blk_mq_free_tags(struct blk_mq_tags *tags) >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 6b39cf0efdcd..91950d3e436a 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -2581,8 +2581,8 @@ static void queue_set_hctx_shared(struct >> request_queue *q, bool shared) >> } >> } >> -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, >> - bool shared) >> +static void blk_mq_update_tag_set_shared(struct blk_mq_tag_set *set, >> + bool shared) >> { >> struct request_queue *q; >> @@ -2605,7 +2605,7 @@ static void blk_mq_del_queue_tag_set(struct >> request_queue *q) >> /* just transitioned to unshared */ >> set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED; >> /* update existing queue */ >> - blk_mq_update_tag_set_depth(set, false); >> + blk_mq_update_tag_set_shared(set, false); >> } >> mutex_unlock(&set->tag_list_lock); >> INIT_LIST_HEAD(&q->tag_set_list); >> @@ -2623,7 +2623,7 @@ static void blk_mq_add_queue_tag_set(struct >> blk_mq_tag_set *set, >> !(set->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { >> set->flags |= BLK_MQ_F_TAG_QUEUE_SHARED; >> /* update existing queue */ >> - blk_mq_update_tag_set_depth(set, true); >> + blk_mq_update_tag_set_shared(set, true); >> } >> if (set->flags & BLK_MQ_F_TAG_QUEUE_SHARED) >> queue_set_hctx_shared(q, true); >> > Hmm. That shouldn't have happened; I'm sure I've had it in two patches originally. Oh well. But I'd rather wait for feedback to my shared scheduler tag patch; guess that'll meet with some raise eyebrows ... Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@xxxxxxx +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer