Re: [PATCH] blk-mq: fix lockdep warning in __blk_mq_update_nr_hw_queues

[Ming Lei <ming.lei@redhat.com>]

On Fri, Aug 15, 2025 at 05:34:23PM +0800, Yu Kuai wrote:
> Hi,
> 
> 在 2025/08/15 17:15, Yu Kuai 写道:
> > Will it be simpler if we move blk_mq_freeze_queue_nomemsave() into
> > blk_mq_elv_switch_none(), after elevator is succeed switching to none
> > then freeze the queue.
> > 
> > Later in blk_mq_elv_switch_back we'll know if xa_load() return valid
> > elevator_type, related queue is already freezed.
> 
> Like following:
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index e9f037a25fe3..3640fae5707b 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -5010,7 +5010,13 @@ static int blk_mq_elv_switch_none(struct
> request_queue *q,
>                 __elevator_get(q->elevator->type);
> 
>                 elevator_set_none(q);
> +       } else {
> +               ret = xa_insert(elv_tbl, q->id, xa_mk_value(1), GFP_KERNEL);
> +               if (WARN_ON_ONCE(ret))
> +                       return ret;
>         }
> +
> +       blk_mq_freeze_queue_nomemsave(q);
>         return ret;
>  }
> 
> @@ -5045,9 +5051,6 @@ static void __blk_mq_update_nr_hw_queues(struct
> blk_mq_tag_set *set,
>                 blk_mq_sysfs_unregister_hctxs(q);
>         }
> 
> -       list_for_each_entry(q, &set->tag_list, tag_set_list)
> -               blk_mq_freeze_queue_nomemsave(q);
> -
>         /*
>          * Switch IO scheduler to 'none', cleaning up the data associated
>          * with the previous scheduler. We will switch back once we are done
> diff --git a/block/elevator.c b/block/elevator.c
> index e2ebfbf107b3..9400ea9ec024 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -715,16 +715,21 @@ void elv_update_nr_hw_queues(struct request_queue *q,
> struct elevator_type *e,
> 
>         WARN_ON_ONCE(q->mq_freeze_depth == 0);
> 
> -       if (e && !blk_queue_dying(q) && blk_queue_registered(q)) {
> -               ctx.name = e->elevator_name;
> -               ctx.et = t;
> -
> -               mutex_lock(&q->elevator_lock);
> -               /* force to reattach elevator after nr_hw_queue is updated
> */
> -               ret = elevator_switch(q, &ctx);
> -               mutex_unlock(&q->elevator_lock);
> +       if (e) {
> +               if (!xa_is_value(e) && !blk_queue_dying(q) &&
> +                   blk_queue_registered(q)) {
> +                       ctx.name = e->elevator_name;
> +                       ctx.et = t;
> +
> +                       mutex_lock(&q->elevator_lock);
> +                       /* force to reattach elevator after nr_hw_queue is
> updated */
> +                       ret = elevator_switch(q, &ctx);
> +                       mutex_unlock(&q->elevator_lock);
> +               }
> +
> +               blk_mq_unfreeze_queue_nomemrestore(q);
>         }
> -       blk_mq_unfreeze_queue_nomemrestore(q);
> +

I feel it doesn't become simpler, :-(

However we still can avoid the change in elv_update_nr_hw_queues() by moving
freeze/unfree queue to blk_mq_elv_switch_back(), which looks more readable.



Thanks, 
Ming