Re: [PATCHv6 3/3] block: fix potential deadlock while running nr_hw_queue update

Hannes Reinecke
2025-06-30T06:24:16Z

On 6/30/25 07:21, Nilay Shroff wrote:
> Move scheduler tags (sched_tags) allocation and deallocation outside
> both the ->elevator_lock and ->freeze_lock when updating nr_hw_queues.
> This change breaks the dependency chain from the percpu allocator lock
> to the elevator lock, helping to prevent potential deadlocks, as
> observed in the reported lockdep splat[1].
>
> This commit introduces batch allocation and deallocation helpers for
> sched_tags, which are now used from within __blk_mq_update_nr_hw_queues
> routine while iterating through the tagset.
>
> With this change, all sched_tags memory management is handled entirely
> outside the ->elevator_lock and the ->freeze_lock context, thereby
> eliminating the lock dependency that could otherwise manifest during
> nr_hw_queues updates.
>
> [1] https://lore.kernel.org/all/0659ea8d-a463-47c8-9180-43c719e106eb@linux.ibm.com/
>
> Reported-by: Stefan Haberland <sth@linux.ibm.com>
> Closes: https://lore.kernel.org/all/0659ea8d-a463-47c8-9180-43c719e106eb@linux.ibm.com/
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> block/blk-mq-sched.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
> block/blk-mq-sched.h | 4 +++
> block/blk-mq.c | 11 +++++++-
> block/blk.h | 2 +-
> block/elevator.c | 4 +--
> 5 files changed, 80 insertions(+), 4 deletions(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 2d6d1ebdd8fb..da802df34a8c 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -427,6 +427,30 @@ void blk_mq_free_sched_tags(struct elevator_tags *et,
> kfree(et);
> }
>
> +void blk_mq_free_sched_tags_batch(struct xarray *et_table,
> + struct blk_mq_tag_set *set)
> +{
> + struct request_queue *q;
> + struct elevator_tags *et;
> +
> + lockdep_assert_held_write(&set->update_nr_hwq_lock);
> +
> + list_for_each_entry(q, &set->tag_list, tag_set_list) {
> + /*
> + * Accessing q->elevator without holding q->elevator_lock is
> + * safe because we're holding here set->update_nr_hwq_lock in
> + * the writer context. So, scheduler update/switch code (which
> + * acquires the same lock but in the reader context) can't run
> + * concurrently.
> + */
> + if (q->elevator) {
> + et = xa_load(et_table, q->id);
> + if (et)
> + blk_mq_free_sched_tags(et, set);
> + }
> + }
> +}
> +
Odd. Do we allow empty sched_tags?
Why isn't this an error (or at least a warning)?

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich