On Fri, Apr 04, 2025 at 11:10:37AM +0200, Christoph Hellwig wrote:
> On Thu, Apr 03, 2025 at 06:54:02PM +0800, Ming Lei wrote:
> > Fixes the following lockdep warning:
>
> Please spell the actual dependency out here, links are not permanent
> and also not readable for any offline reading of the commit logs.
>
> > +static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
> > + struct request_queue *q, bool lock)
> > +{
> > + if (lock) {
>
> bool lock(ed) arguments are an anti-pattern, and regularly get Linus
> screaming at you (in this case even for the right reason :))
>
> > + /* protect against switching io scheduler */
> > + mutex_lock(&q->elevator_lock);
> > + __blk_mq_realloc_hw_ctxs(set, q);
> > + mutex_unlock(&q->elevator_lock);
> > + } else {
> > + __blk_mq_realloc_hw_ctxs(set, q);
> > + }
>
> I think the problem here is again that because of all the other
> dependencies elevator_lock really needs to be per-set instead of
> per-queue which will allows us to have much saner locking hierarchies.
per-set lock is required in error handling code path, anywhere it is used
with freezing together can be one deadlock risk if hardware error happens.
Or can you explain the idea in details?
Thanks,
Ming
