Re: [PATCH 03/11] blk-mq: rename blk_mq_update_tag_set_depth()

[Hannes Reinecke <hare@suse.de>]

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@suse.de>
>> ---
>>   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@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer