Re: [PATCH 4/6] elevator: factor elevator lock out of dispatch_request method

[Damien Le Moal <dlemoal@kernel.org>]

On 7/23/25 11:51 AM, Yu Kuai wrote:
>> If you apply this patch, stop here without applying the following patches, and
>> test the changes up to this point, things will break since there is no locking
>> during dispatch.
> 
> Do you missed the following change in this patch? Dispatch do switch to
> the new lock, I don't get it why there is no locking.

My bad. Yes, I completely missed it. Sorry for the noise.

> @@ -113,7 +114,12 @@ static int __blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx
> *hctx)
>          if (budget_token < 0)
>              break;
> 
> +        if (sq_sched)
> +            spin_lock_irq(&e->lock);
>          rq = e->type->ops.dispatch_request(hctx);
> +        if (sq_sched)
> +            spin_unlock_irq(&e->lock);
> +
>          if (!rq) {
>              blk_mq_put_dispatch_budget(q, budget_token);
>              /*
>>
>> So you need to organize the patches so that you first have the elevator level
>> common locking in place and then have one patch for bfq and one patch for
>> mq-deadline that switch to using that new lock. Hence the suggestion to reverse
>> the order of your changes: change the block layer first, then have bfq and
>> mq-deadline use that new locking.
> 
> I think I understand what you mean, just to be sure.
> 
> 1. patch 5 in this set
> 2. patch to introduce high level lock, and grab it during dispatch in block layer.
> 3. changes in ioc
> 4. changes in bfq
> 5. changes in deadline
> 6. patch 6 in this set.

What about something like this:
1) Introduce the elevator common/generic lock (first part of patch 1 + middle
of patch 4 squashed together)
2) Convert deadline to use elevator generic lock (second part of patch 1 + end
of patch 4)
3) Convert bfq to use elevator generic lock (patch 3 + beginning of patch 4)
4) Patch 6

As for the ioc changes, they do not seem directly related to the elevator lock
changes, but since the code may conflict, maybe bring them as prep patches at
the beginning (0).


-- 
Damien Le Moal
Western Digital Research