Re: [PATCH v1 07/13] thermal: gov_power_allocator: Use trip pointers instead of trip indices

[Rafael J. Wysocki]

On Wed, Sep 27, 2023 at 5:46 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:
>
> On 27/09/2023 17:27, Rafael J. Wysocki wrote:
> > On Wed, Sep 27, 2023 at 5:10 PM Daniel Lezcano
> > <daniel.lezcano@xxxxxxxxxx> wrote:
> >>
> >> On 21/09/2023 19:55, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >>>
> >>> Eliminate the __thermal_zone_get_trip() usage that adds unnecessary
> >>> overhead (due to pointless bounds checking and copying of trip point
> >>> data) from the power allocator thermal governor and generally make it
> >>> use trip pointers instead of trip indices where applicable.
> >>
> >> Actually the __thermal_zone_get_trip() change was done on purpose to
> >> replace the 'throttle' callback index parameter by the trip pointer and
> >> removing those call to __thermal_zone_get_trip() while the code was
> >> using the trip pointer.
> >>
> >> IMO, the changes should focus on changing the trip_index parameter by
> >> the trip pointer directly in the throttle ops.
> >
> > So you would like .throttle() to take a trip pointer argument instead
> > of an index?
> >
> > The difficulty here is that the user space governor needs to expose
> > the index to user space anyway, so it would need to find it if it gets
> > a trip pointer instead.
> >
> > Not a big deal I suppose, but a bit of extra overhead.
> >
> > Also it is easier to switch the governors over to using trip pointers
> > internally and then change the .throttle() argument on top of that.
> >
> >> The pointer can be
> >> retrieved in the handle_thermal_trip() function and passed around for
> >> the rest of the actions on this trip point
> >
> > Right, except for the user space governor which needs a trip index.
> > And the indices are used for tracing too.
>
> Given the userspace governor is going obsolete and the notifications are
> for the userspace, which is slow, we can retrieve the index from the
> throttling ops

OK

Given that patches [01-05/13] are not controversial, I'll respon the
governor patches into a separate series on top of them.

I would much appreciate it if you could take a look at patch [10/13]
and the remaining ACPi thermal patches in this series [11-13/13].
They don't depend on the governor changes.