Re: [PATCH 2/2] blk-mq: simplify blk_mq_realloc_tag_set_tags

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Nov 09, 2022 / 11:08, Christoph Hellwig wrote:
> Use set->nr_hw_queues for the current number of tags, and remove the
> duplicate set->nr_hw_queues update in the caller.

Chaitanya found that blktests test cases block/029 and block/030 fail on
linux-block/for-next branch [1]. The test cases modify null_blk devices'
submit_queues number during test, and it caused the failures.

[1] https://lore.kernel.org/linux-block/5183bc2c-746b-5806-9ace-31a3a7000e6d@nvidia.com/

I bisected and confirmed that this patch is the trigger. As I noted below,
one of its hunks looks wrong for me.

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-mq.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 8c630dbdf107e..9fa0b9a1435f2 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4381,11 +4381,11 @@ static void blk_mq_update_queue_map(struct blk_mq_tag_set *set)
>  }
>  
>  static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
> -				  int cur_nr_hw_queues, int new_nr_hw_queues)
> +				       int new_nr_hw_queues)
>  {
>  	struct blk_mq_tags **new_tags;
>  
> -	if (cur_nr_hw_queues >= new_nr_hw_queues)
> +	if (set->nr_hw_queues >= new_nr_hw_queues)
>  		return 0;
>

When the condition of the if statement above is true, set->nr_hw_queues is no
longer updated with new_nr_hw_queues. In nullb_update_nr_hw_queues(), null_blk
calls blk_mq_update_nr_hw_queues() and refers set->nr_hw_queues. With unexpected
set->nr_hw_queues value, null_blk fails to update submit_queues number.

With a quick fix below, the blktests failures were avoided. Could you take look
and see if this is the right fix?

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a3a5fb4d4ef6..604e19be9648 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4384,8 +4384,10 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 {
        struct blk_mq_tags **new_tags;

-       if (set->nr_hw_queues >= new_nr_hw_queues)
+       if (set->nr_hw_queues >= new_nr_hw_queues) {
+               set->nr_hw_queues = new_nr_hw_queues;
                return 0;
+       }

        new_tags = kcalloc_node(new_nr_hw_queues, sizeof(struct blk_mq_tags *),
                                GFP_KERNEL, set->numa_node);

-- 
Shin'ichiro Kawasaki