Re: [PATCH] block: fix lock dependency between percpu alloc lock and elevator lock

[Nilay Shroff <nilay@linux.ibm.com>]

On 5/21/25 9:38 AM, Ming Lei wrote:
> Hi Nilay,
> 
> On Tue, May 20, 2025 at 04:03:49PM +0530, Nilay Shroff wrote:

> 
> Not dig into this implementation, will look into later.
> 
> I guess it should work by extending elv_change_ctx.
> 
> However we have other elevator_queue lifetime issue, that is why
> ->elevator_lock is used almost everywhere.
> 
> Another solution is to move all `sched_data` into 'struct elevator_queue':
> 
> I feel it may be simpler in concept:
> 
> - sched data and elevator queue share same lifetime
> 
> - kobject_put(&eq->kobj) is already called without holding ->elevator_lock &
>   queue isn't freezed
> 
> - replace unnecessary ->elevator_lock by blk_get_elevator()/blk_put_elevator()
> 
> But it needs some cleanup/refactor on scheduler interface.
> 
> What do you think about the above way?

IMO moving scheduler specific data/tags into 'struct elevator_queue' is
a good idea. So instead of extending elv_change_ctx for storing/restoring
sched_tags (which this propose patch implements), we may now store the
sched_tags inside elevator_queue. And as you mentioned, the lifetime of 
elevator_queue and sched_tags are similar, we can release the sched_tags
along with releasing the elevator kobject (which anyways happens outside
of ->elevator_lock and ->freeze_lock). Also elevator queue alloc code 
can be moved out of init_sched before we acquire ->elevator_lock and 
->freeze_lock. And, as Christoph mentioned in another email, the changes
might be invasive but it seems doable. 

I will send another patch with the above changes. 

Thanks,
--Nilay