| Message ID | 2411572.NG923GbCHz@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | thermal: core: Redesign the governor interface | expand |
On 4/10/24 17:57, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The Fair Share governor tries very hard to be stateless and so it > calls get_trip_level() from fair_share_throttle() every time, even > though the number produced by this function for all of the trips > during a given thermal zone update is actually the same. Since > get_trip_level() walks all of the trips in the thermal zone every > time it is called, doing this may generate quite a bit of completely > useless overhead. > > For this reason, make the governor use the new .manage() callback > instead of .throttle() which allows it to call get_trip_level() just > once and use the value computed by it to handle all of the trips. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/gov_fair_share.c | 37 ++++++++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 11 deletions(-) > > Index: linux-pm/drivers/thermal/gov_fair_share.c > =================================================================== > --- linux-pm.orig/drivers/thermal/gov_fair_share.c > +++ linux-pm/drivers/thermal/gov_fair_share.c > @@ -53,6 +53,7 @@ static long get_target_state(struct ther > * fair_share_throttle - throttles devices associated with the given zone > * @tz: thermal_zone_device > * @trip: trip point > + * @trip_level: number of trips crossed by the zone temperature > * > * Throttling Logic: This uses three parameters to calculate the new > * throttle state of the cooling devices associated with the given zone. > @@ -61,22 +62,19 @@ static long get_target_state(struct ther > * P1. max_state: Maximum throttle state exposed by the cooling device. > * P2. percentage[i]/100: > * How 'effective' the 'i'th device is, in cooling the given zone. > - * P3. cur_trip_level/max_no_of_trips: > + * P3. trip_level/max_no_of_trips: > * This describes the extent to which the devices should be throttled. > * We do not want to throttle too much when we trip a lower temperature, > * whereas the throttling is at full swing if we trip critical levels. > - * (Heavily assumes the trip points are in ascending order) > * new_state of cooling device = P3 * P2 * P1 > */ > -static int fair_share_throttle(struct thermal_zone_device *tz, > - const struct thermal_trip *trip) > +static void fair_share_throttle(struct thermal_zone_device *tz, > + const struct thermal_trip *trip, > + int trip_level) > { > struct thermal_instance *instance; > int total_weight = 0; > int total_instance = 0; > - int cur_trip_level = get_trip_level(tz); > - > - lockdep_assert_held(&tz->lock); > > list_for_each_entry(instance, &tz->thermal_instances, tz_node) { > if (instance->trip != trip) > @@ -99,18 +97,35 @@ static int fair_share_throttle(struct th > percentage = (instance->weight * 100) / total_weight; > > instance->target = get_target_state(tz, cdev, percentage, > - cur_trip_level); > + trip_level); > > mutex_lock(&cdev->lock); > __thermal_cdev_update(cdev); > mutex_unlock(&cdev->lock); > } > +} > > - return 0; > +static void fair_share_manage(struct thermal_zone_device *tz) > +{ > + int trip_level = get_trip_level(tz); > + const struct thermal_trip_desc *td; > + > + lockdep_assert_held(&tz->lock); > + > + for_each_trip_desc(tz, td) { > + const struct thermal_trip *trip = &td->trip; > + > + if (trip->temperature == THERMAL_TEMP_INVALID || > + trip->type == THERMAL_TRIP_CRITICAL || > + trip->type == THERMAL_TRIP_HOT) > + continue; > + > + fair_share_throttle(tz, trip, trip_level); > + } > } > > static struct thermal_governor thermal_gov_fair_share = { > - .name = "fair_share", > - .throttle = fair_share_throttle, > + .name = "fair_share", > + .manage = fair_share_manage, > }; > THERMAL_GOVERNOR_DECLARE(thermal_gov_fair_share); > > > LGTM Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Index: linux-pm/drivers/thermal/gov_fair_share.c =================================================================== --- linux-pm.orig/drivers/thermal/gov_fair_share.c +++ linux-pm/drivers/thermal/gov_fair_share.c @@ -53,6 +53,7 @@ static long get_target_state(struct ther * fair_share_throttle - throttles devices associated with the given zone * @tz: thermal_zone_device * @trip: trip point + * @trip_level: number of trips crossed by the zone temperature * * Throttling Logic: This uses three parameters to calculate the new * throttle state of the cooling devices associated with the given zone. @@ -61,22 +62,19 @@ static long get_target_state(struct ther * P1. max_state: Maximum throttle state exposed by the cooling device. * P2. percentage[i]/100: * How 'effective' the 'i'th device is, in cooling the given zone. - * P3. cur_trip_level/max_no_of_trips: + * P3. trip_level/max_no_of_trips: * This describes the extent to which the devices should be throttled. * We do not want to throttle too much when we trip a lower temperature, * whereas the throttling is at full swing if we trip critical levels. - * (Heavily assumes the trip points are in ascending order) * new_state of cooling device = P3 * P2 * P1 */ -static int fair_share_throttle(struct thermal_zone_device *tz, - const struct thermal_trip *trip) +static void fair_share_throttle(struct thermal_zone_device *tz, + const struct thermal_trip *trip, + int trip_level) { struct thermal_instance *instance; int total_weight = 0; int total_instance = 0; - int cur_trip_level = get_trip_level(tz); - - lockdep_assert_held(&tz->lock); list_for_each_entry(instance, &tz->thermal_instances, tz_node) { if (instance->trip != trip) @@ -99,18 +97,35 @@ static int fair_share_throttle(struct th percentage = (instance->weight * 100) / total_weight; instance->target = get_target_state(tz, cdev, percentage, - cur_trip_level); + trip_level); mutex_lock(&cdev->lock); __thermal_cdev_update(cdev); mutex_unlock(&cdev->lock); } +} - return 0; +static void fair_share_manage(struct thermal_zone_device *tz) +{ + int trip_level = get_trip_level(tz); + const struct thermal_trip_desc *td; + + lockdep_assert_held(&tz->lock); + + for_each_trip_desc(tz, td) { + const struct thermal_trip *trip = &td->trip; + + if (trip->temperature == THERMAL_TEMP_INVALID || + trip->type == THERMAL_TRIP_CRITICAL || + trip->type == THERMAL_TRIP_HOT) + continue; + + fair_share_throttle(tz, trip, trip_level); + } } static struct thermal_governor thermal_gov_fair_share = { - .name = "fair_share", - .throttle = fair_share_throttle, + .name = "fair_share", + .manage = fair_share_manage, }; THERMAL_GOVERNOR_DECLARE(thermal_gov_fair_share);