mbox series

[v1,0/6] thermal: core: Pass trip pointers to governor .throttle() callbacks

Message ID 13365827.uLZWGnKmhe@kreacher
Headers show
Series thermal: core: Pass trip pointers to governor .throttle() callbacks | expand

Message

Rafael J. Wysocki Oct. 6, 2023, 5:38 p.m. UTC
Hi All,

While working on https://lore.kernel.org/linux-pm/4846448.GXAFRqVoOG@kreacher/
I started to change thermal governors so as to reduce the usage of trip
indices in them.  At that time, I was concerned that they could not be
replaced with trip pointers overall, because they were needed in certain
situations (tracing, debug messages, userspace governor) and computing them
whenever they were needed would be extra overhead with relatively weak
justification.  In the meantime, however, I realized that for a given trip
pointer, it is actually trivial to compute the corresponding index: it is
sufficient to subtract the start of the trips[] table in the thermal zone
containing the trip from that trip pointer for this purpose.  Patch [1/6]
modifies thermal_zone_trip_id() in accordance with this observation.

Now that the cost of computing a trip index for a given trip pointer and
thermal zone is not a concern any more, the governors can be generally
switched over to using trip pointers for representing trips.  One of the
things they need to do sometimes, though, is to iterate over trips in a
given thermal zone (the core needs to do that too, of course) and
for_each_thermal_trip() is somewhat inconvenient for this purpose, because
it requires callback functions to be defined and in some cases new data
types need to be introduced just in order to use it.  For this reason,
patch [2/6] adds a macro for iterating over trip points in a given thermal
zone with the help of a trip pointer and changes for_each_thermal_trip() to
use that macro internally.

Patches [3-5/6] change individual governors to prepare them for using trip
pointers everywhere for representing trips, except for the trip argument of
the .throttle() callback and patch [6/6] finally changes the .throttle()
callback definition so that it takes a trip pointer as the argument
representing the trip.

Please refer to the individual patch changelogs for details.

Thanks!

Comments

Lukasz Luba Oct. 13, 2023, 8:12 a.m. UTC | #1
Hi Rafael,


On 10/6/23 18:38, Rafael J. Wysocki wrote:
> Hi All,
> 
> While working on https://lore.kernel.org/linux-pm/4846448.GXAFRqVoOG@kreacher/
> I started to change thermal governors so as to reduce the usage of trip
> indices in them.  At that time, I was concerned that they could not be
> replaced with trip pointers overall, because they were needed in certain
> situations (tracing, debug messages, userspace governor) and computing them
> whenever they were needed would be extra overhead with relatively weak
> justification.  In the meantime, however, I realized that for a given trip
> pointer, it is actually trivial to compute the corresponding index: it is
> sufficient to subtract the start of the trips[] table in the thermal zone
> containing the trip from that trip pointer for this purpose.  Patch [1/6]
> modifies thermal_zone_trip_id() in accordance with this observation.
> 
> Now that the cost of computing a trip index for a given trip pointer and
> thermal zone is not a concern any more, the governors can be generally
> switched over to using trip pointers for representing trips.  One of the
> things they need to do sometimes, though, is to iterate over trips in a
> given thermal zone (the core needs to do that too, of course) and
> for_each_thermal_trip() is somewhat inconvenient for this purpose, because
> it requires callback functions to be defined and in some cases new data
> types need to be introduced just in order to use it.  For this reason,
> patch [2/6] adds a macro for iterating over trip points in a given thermal
> zone with the help of a trip pointer and changes for_each_thermal_trip() to
> use that macro internally.
> 
> Patches [3-5/6] change individual governors to prepare them for using trip
> pointers everywhere for representing trips, except for the trip argument of
> the .throttle() callback and patch [6/6] finally changes the .throttle()
> callback definition so that it takes a trip pointer as the argument
> representing the trip.
> 
> Please refer to the individual patch changelogs for details.
> 
> Thanks!
> 
> 
> 

I have issues to apply this series, could you tell me the best
base branch from your tree?

I will give it a try on my boards and review.

Thanks,
Lukasz
Rafael J. Wysocki Oct. 13, 2023, 10:07 a.m. UTC | #2
Hi Lukasz,

On Fri, Oct 13, 2023 at 10:12 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Rafael,
>
>
> On 10/6/23 18:38, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > While working on https://lore.kernel.org/linux-pm/4846448.GXAFRqVoOG@kreacher/
> > I started to change thermal governors so as to reduce the usage of trip
> > indices in them.  At that time, I was concerned that they could not be
> > replaced with trip pointers overall, because they were needed in certain
> > situations (tracing, debug messages, userspace governor) and computing them
> > whenever they were needed would be extra overhead with relatively weak
> > justification.  In the meantime, however, I realized that for a given trip
> > pointer, it is actually trivial to compute the corresponding index: it is
> > sufficient to subtract the start of the trips[] table in the thermal zone
> > containing the trip from that trip pointer for this purpose.  Patch [1/6]
> > modifies thermal_zone_trip_id() in accordance with this observation.
> >
> > Now that the cost of computing a trip index for a given trip pointer and
> > thermal zone is not a concern any more, the governors can be generally
> > switched over to using trip pointers for representing trips.  One of the
> > things they need to do sometimes, though, is to iterate over trips in a
> > given thermal zone (the core needs to do that too, of course) and
> > for_each_thermal_trip() is somewhat inconvenient for this purpose, because
> > it requires callback functions to be defined and in some cases new data
> > types need to be introduced just in order to use it.  For this reason,
> > patch [2/6] adds a macro for iterating over trip points in a given thermal
> > zone with the help of a trip pointer and changes for_each_thermal_trip() to
> > use that macro internally.
> >
> > Patches [3-5/6] change individual governors to prepare them for using trip
> > pointers everywhere for representing trips, except for the trip argument of
> > the .throttle() callback and patch [6/6] finally changes the .throttle()
> > callback definition so that it takes a trip pointer as the argument
> > representing the trip.
> >
> > Please refer to the individual patch changelogs for details.
> >
> > Thanks!
> >
> >
> >
>
> I have issues to apply this series, could you tell me the best
> base branch from your tree?
>
> I will give it a try on my boards and review.

It is there in the thermal-core branch of linux-pm.git.

Thanks!