From: Yu Kuai <yukuai3@huawei.com>
No functional changes are intended, make code cleaner and prepare to fix
the grow case in following patches.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
block/blk-mq.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1ff6370f7314..82fa81036115 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4931,21 +4931,25 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
blk_mq_tag_update_sched_shared_tags(q);
else
blk_mq_tag_resize_shared_tags(set, nr);
- } else {
+ } else if (!q->elevator) {
queue_for_each_hw_ctx(q, hctx, i) {
if (!hctx->tags)
continue;
- /*
- * If we're using an MQ scheduler, just update the
- * scheduler queue depth. This is similar to what the
- * old code would do.
- */
- if (hctx->sched_tags)
- ret = blk_mq_tag_update_depth(hctx,
- &hctx->sched_tags, nr);
- else
- ret = blk_mq_tag_update_depth(hctx,
- &hctx->tags, nr);
+ sbitmap_queue_resize(&hctx->tags->bitmap_tags,
+ nr - hctx->tags->nr_reserved_tags);
+ }
+ } else if (nr <= q->elevator->et->nr_requests) {
+ queue_for_each_hw_ctx(q, hctx, i) {
+ if (!hctx->sched_tags)
+ continue;
+ sbitmap_queue_resize(&hctx->sched_tags->bitmap_tags,
+ nr - hctx->sched_tags->nr_reserved_tags);
+ }
+ } else {
+ queue_for_each_hw_ctx(q, hctx, i) {
+ if (!hctx->sched_tags)
+ continue;
+ blk_mq_tag_update_depth(hctx, &hctx->sched_tags, nr);
if (ret)
goto out;
}
--
2.39.2
On 9/8/25 11:45 AM, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
>
> No functional changes are intended, make code cleaner and prepare to fix
> the grow case in following patches.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> block/blk-mq.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 1ff6370f7314..82fa81036115 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4931,21 +4931,25 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
> blk_mq_tag_update_sched_shared_tags(q);
> else
> blk_mq_tag_resize_shared_tags(set, nr);
> - } else {
> + } else if (!q->elevator) {
> queue_for_each_hw_ctx(q, hctx, i) {
> if (!hctx->tags)
> continue;
> - /*
> - * If we're using an MQ scheduler, just update the
> - * scheduler queue depth. This is similar to what the
> - * old code would do.
> - */
> - if (hctx->sched_tags)
> - ret = blk_mq_tag_update_depth(hctx,
> - &hctx->sched_tags, nr);
> - else
> - ret = blk_mq_tag_update_depth(hctx,
> - &hctx->tags, nr);
> + sbitmap_queue_resize(&hctx->tags->bitmap_tags,
> + nr - hctx->tags->nr_reserved_tags);
> + }
> + } else if (nr <= q->elevator->et->nr_requests) {
> + queue_for_each_hw_ctx(q, hctx, i) {
> + if (!hctx->sched_tags)
> + continue;
> + sbitmap_queue_resize(&hctx->sched_tags->bitmap_tags,
> + nr - hctx->sched_tags->nr_reserved_tags);
> + }
> + } else {
> + queue_for_each_hw_ctx(q, hctx, i) {
> + if (!hctx->sched_tags)
> + continue;
> + blk_mq_tag_update_depth(hctx, &hctx->sched_tags, nr);
> if (ret)
> goto out;
> }
The above code is good however can this be bit simplified?
It's a bit difficult to follow through all nesting and so
could it be simplified as below:
if (shared-tags) {
if (elevator)
// resize sched-shared-tags bitmap
else
// resize shared-tags bitmap
} else {
// non-shared tags
if (elevator) {
if (new-depth-is-less-than-the-current-depth)
// resize sched-tags bitmap
else
// handle sched tags grow
} else
// resize tags bitmap
}
Thanks,
--Nilay
Hi,
在 2025/9/9 20:18, Nilay Shroff 写道:
>
> On 9/8/25 11:45 AM, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> No functional changes are intended, make code cleaner and prepare to fix
>> the grow case in following patches.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>> block/blk-mq.c | 28 ++++++++++++++++------------
>> 1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 1ff6370f7314..82fa81036115 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -4931,21 +4931,25 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>> blk_mq_tag_update_sched_shared_tags(q);
>> else
>> blk_mq_tag_resize_shared_tags(set, nr);
>> - } else {
>> + } else if (!q->elevator) {
>> queue_for_each_hw_ctx(q, hctx, i) {
>> if (!hctx->tags)
>> continue;
>> - /*
>> - * If we're using an MQ scheduler, just update the
>> - * scheduler queue depth. This is similar to what the
>> - * old code would do.
>> - */
>> - if (hctx->sched_tags)
>> - ret = blk_mq_tag_update_depth(hctx,
>> - &hctx->sched_tags, nr);
>> - else
>> - ret = blk_mq_tag_update_depth(hctx,
>> - &hctx->tags, nr);
>> + sbitmap_queue_resize(&hctx->tags->bitmap_tags,
>> + nr - hctx->tags->nr_reserved_tags);
>> + }
>> + } else if (nr <= q->elevator->et->nr_requests) {
>> + queue_for_each_hw_ctx(q, hctx, i) {
>> + if (!hctx->sched_tags)
>> + continue;
>> + sbitmap_queue_resize(&hctx->sched_tags->bitmap_tags,
>> + nr - hctx->sched_tags->nr_reserved_tags);
>> + }
>> + } else {
>> + queue_for_each_hw_ctx(q, hctx, i) {
>> + if (!hctx->sched_tags)
>> + continue;
>> + blk_mq_tag_update_depth(hctx, &hctx->sched_tags, nr);
>> if (ret)
>> goto out;
>> }
> The above code is good however can this be bit simplified?
> It's a bit difficult to follow through all nesting and so
> could it be simplified as below:
>
> if (shared-tags) {
> if (elevator)
> // resize sched-shared-tags bitmap
> else
> // resize shared-tags bitmap
> } else {
> // non-shared tags
> if (elevator) {
> if (new-depth-is-less-than-the-current-depth)
> // resize sched-tags bitmap
> else
> // handle sched tags grow
> } else
> // resize tags bitmap
> }
AFAIK, if - else if chain should be better than nested if - else, right?
If you don't mind, I can add comments to each else if chain to make code cleaner:
if () {
/* shared tags */
...
} else if () {
/* non-shared tags and elevator is none */
...
} else if () {
/* non-shared tags and elevator is not none, nr_requests doesn't grow */
...
} else () {
/* non-shared tags and elevator is not none, nr_requests grow */
...
}
Thanks,
Kuai
>
> Thanks,
> --Nilay
>
>
On 9/9/25 10:09 PM, Yu Kuai wrote:
> Hi,
>
> 在 2025/9/9 20:18, Nilay Shroff 写道:
>>
>> On 9/8/25 11:45 AM, Yu Kuai wrote:
>>> From: Yu Kuai <yukuai3@huawei.com>
>>>
>>> No functional changes are intended, make code cleaner and prepare to fix
>>> the grow case in following patches.
>>>
>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>>> ---
>>> block/blk-mq.c | 28 ++++++++++++++++------------
>>> 1 file changed, 16 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>> index 1ff6370f7314..82fa81036115 100644
>>> --- a/block/blk-mq.c
>>> +++ b/block/blk-mq.c
>>> @@ -4931,21 +4931,25 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>>> blk_mq_tag_update_sched_shared_tags(q);
>>> else
>>> blk_mq_tag_resize_shared_tags(set, nr);
>>> - } else {
>>> + } else if (!q->elevator) {
>>> queue_for_each_hw_ctx(q, hctx, i) {
>>> if (!hctx->tags)
>>> continue;
>>> - /*
>>> - * If we're using an MQ scheduler, just update the
>>> - * scheduler queue depth. This is similar to what the
>>> - * old code would do.
>>> - */
>>> - if (hctx->sched_tags)
>>> - ret = blk_mq_tag_update_depth(hctx,
>>> - &hctx->sched_tags, nr);
>>> - else
>>> - ret = blk_mq_tag_update_depth(hctx,
>>> - &hctx->tags, nr);
>>> + sbitmap_queue_resize(&hctx->tags->bitmap_tags,
>>> + nr - hctx->tags->nr_reserved_tags);
>>> + }
>>> + } else if (nr <= q->elevator->et->nr_requests) {
>>> + queue_for_each_hw_ctx(q, hctx, i) {
>>> + if (!hctx->sched_tags)
>>> + continue;
>>> + sbitmap_queue_resize(&hctx->sched_tags->bitmap_tags,
>>> + nr - hctx->sched_tags->nr_reserved_tags);
>>> + }
>>> + } else {
>>> + queue_for_each_hw_ctx(q, hctx, i) {
>>> + if (!hctx->sched_tags)
>>> + continue;
>>> + blk_mq_tag_update_depth(hctx, &hctx->sched_tags, nr);
>>> if (ret)
>>> goto out;
>>> }
>> The above code is good however can this be bit simplified?
>> It's a bit difficult to follow through all nesting and so
>> could it be simplified as below:
>>
>> if (shared-tags) {
>> if (elevator)
>> // resize sched-shared-tags bitmap
>> else
>> // resize shared-tags bitmap
>> } else {
>> // non-shared tags
>> if (elevator) {
>> if (new-depth-is-less-than-the-current-depth)
>> // resize sched-tags bitmap
>> else
>> // handle sched tags grow
>> } else
>> // resize tags bitmap
>> }
>
> AFAIK, if - else if chain should be better than nested if - else, right?
>
> If you don't mind, I can add comments to each else if chain to make code cleaner:
>
> if () {
> /* shared tags */
> ...
> } else if () {
> /* non-shared tags and elevator is none */
> ...
> } else if () {
> /* non-shared tags and elevator is not none, nr_requests doesn't grow */
> ...
> } else () {
> /* non-shared tags and elevator is not none, nr_requests grow */
> ...
> }
>
Yeah, I am good with the proper comments as well so that it'd be easy
for anyone reviewing the code later to understand what those all nested
if-else conditions meant.
Thanks,
--Nilay
Hi,
在 2025/09/10 14:30, Nilay Shroff 写道:
>
>
> On 9/9/25 10:09 PM, Yu Kuai wrote:
>> Hi,
>>
>> 在 2025/9/9 20:18, Nilay Shroff 写道:
>>>
>>> On 9/8/25 11:45 AM, Yu Kuai wrote:
>>>> From: Yu Kuai <yukuai3@huawei.com>
>>>>
>>>> No functional changes are intended, make code cleaner and prepare to fix
>>>> the grow case in following patches.
>>>>
>>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>>>> ---
>>>> block/blk-mq.c | 28 ++++++++++++++++------------
>>>> 1 file changed, 16 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>>> index 1ff6370f7314..82fa81036115 100644
>>>> --- a/block/blk-mq.c
>>>> +++ b/block/blk-mq.c
>>>> @@ -4931,21 +4931,25 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>>>> blk_mq_tag_update_sched_shared_tags(q);
>>>> else
>>>> blk_mq_tag_resize_shared_tags(set, nr);
>>>> - } else {
>>>> + } else if (!q->elevator) {
>>>> queue_for_each_hw_ctx(q, hctx, i) {
>>>> if (!hctx->tags)
>>>> continue;
>>>> - /*
>>>> - * If we're using an MQ scheduler, just update the
>>>> - * scheduler queue depth. This is similar to what the
>>>> - * old code would do.
>>>> - */
>>>> - if (hctx->sched_tags)
>>>> - ret = blk_mq_tag_update_depth(hctx,
>>>> - &hctx->sched_tags, nr);
>>>> - else
>>>> - ret = blk_mq_tag_update_depth(hctx,
>>>> - &hctx->tags, nr);
>>>> + sbitmap_queue_resize(&hctx->tags->bitmap_tags,
>>>> + nr - hctx->tags->nr_reserved_tags);
>>>> + }
>>>> + } else if (nr <= q->elevator->et->nr_requests) {
>>>> + queue_for_each_hw_ctx(q, hctx, i) {
>>>> + if (!hctx->sched_tags)
>>>> + continue;
>>>> + sbitmap_queue_resize(&hctx->sched_tags->bitmap_tags,
>>>> + nr - hctx->sched_tags->nr_reserved_tags);
>>>> + }
>>>> + } else {
>>>> + queue_for_each_hw_ctx(q, hctx, i) {
>>>> + if (!hctx->sched_tags)
>>>> + continue;
>>>> + blk_mq_tag_update_depth(hctx, &hctx->sched_tags, nr);
>>>> if (ret)
>>>> goto out;
>>>> }
>>> The above code is good however can this be bit simplified?
>>> It's a bit difficult to follow through all nesting and so
>>> could it be simplified as below:
>>>
>>> if (shared-tags) {
>>> if (elevator)
>>> // resize sched-shared-tags bitmap
>>> else
>>> // resize shared-tags bitmap
>>> } else {
>>> // non-shared tags
>>> if (elevator) {
>>> if (new-depth-is-less-than-the-current-depth)
>>> // resize sched-tags bitmap
>>> else
>>> // handle sched tags grow
>>> } else
>>> // resize tags bitmap
>>> }
>>
>> AFAIK, if - else if chain should be better than nested if - else, right?
>>
>> If you don't mind, I can add comments to each else if chain to make code cleaner:
>>
>> if () {
>> /* shared tags */
>> ...
>> } else if () {
>> /* non-shared tags and elevator is none */
>> ...
>> } else if () {
>> /* non-shared tags and elevator is not none, nr_requests doesn't grow */
>> ...
>> } else () {
>> /* non-shared tags and elevator is not none, nr_requests grow */
>> ...
>> }
>>
> Yeah, I am good with the proper comments as well so that it'd be easy
> for anyone reviewing the code later to understand what those all nested
> if-else conditions meant.
>
Ok, I'll do that in the next version.
Thanks for the review!
Kuai
> Thanks,
> --Nilay
>
> .
>
© 2016 - 2025 Red Hat, Inc.