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@xxxxxxxxxx>
>>>
>>> No functional changes are intended, make code cleaner and prepare to fix
>>> the grow case in following patches.
>>>
>>> Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>
>>> ---
>>> 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
