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
