Re: [PATCH 2/3] block: introduce alloc_sched_data and free_sched_data elevator methods

[Ming Lei <ming.lei@redhat.com>]

On Thu, Oct 23, 2025 at 11:27:26AM +0530, Nilay Shroff wrote:
> 
> 
> On 10/22/25 10:09 AM, Ming Lei wrote:
> > On Thu, Oct 16, 2025 at 11:00:48AM +0530, Nilay Shroff wrote:
> >> The recent lockdep splat [1] highlights a potential deadlock risk
> >> involving ->elevator_lock and ->freeze_lock dependencies on -pcpu_alloc_
> >> mutex. The trace shows that the issue occurs when the Kyber scheduler
> >> allocates dynamic memory for its elevator data during initialization.
> >>
> >> To address this, introduce two new elevator operation callbacks:
> >> ->alloc_sched_data and ->free_sched_data.
> > 
> > This way looks good.
> > 
> >>
> >> When an elevator implements these methods, they are invoked during
> >> scheduler switch before acquiring ->freeze_lock and ->elevator_lock.
> >> This allows safe allocation and deallocation of per-elevator data
> > 
> > This per-elevator data should be very similar with `struct elevator_tags`
> > from block layer viewpoint: both have same lifetime, and follow same
> > allocation constraint(per-cpu lock).
> > 
> > Can we abstract elevator data structure to cover both? Then I guess the
> > code should be more readable & maintainable, what do you think of this way?
> > 
> > One easiest way could be to add 'void *data' into `struct elevator_tags`,
> > just the naming of `elevator_tags` is not generic enough, but might not
> > a big deal.
> > 
> Hmm, good point! I'd rather suggest if we could instead rename 
> struct elevator_tags to struct elevator_resources and then
> add void *data field to it. Something like this:
> 
> struct elevator_tags {
> 	unsigned int nr_hw_queues;
> 	unsigned int nr_requests;
> 	struct blk_mq_tags *tags[];
>         void *data;

'data' can't follow `tags[]`.

> };
> 
> What do you think?

It is good. The patch may be split into two:

- add data to `struct elevator_tags` for covering the lockdep issue

- renaming

Then it will become easier for review.


Thanks
Ming