| Message ID | 1994438.PYKUYFuaPT@rjwysocki.net (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | thermal: core: Two fixes for 6.12 | expand |
On 22/08/2024 21:47, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Using round_jiffies() in thermal_set_delay_jiffies() is invalid because > its argument should be time in the future in absolute jiffies and it > computes the result with respect to the current jiffies value at the > invocation time. Fortunately, in the majority of cases it does not > make any difference due to the time_is_after_jiffies() check in > round_jiffies_common(). > > While using round_jiffies_relative() instead of round_jiffies() might > reflect the intent a bit better, it still would not be defensible > because that function should be called when the timer is about to be > set and it is not suitable for pre-computation of delay values. > > Accordingly, drop thermal_set_delay_jiffies() altogether, simply > convert polling_delay and passive_delay to jiffies during thermal > zone initialization and make thermal_zone_device_set_polling() call > round_jiffies_relative() on the delay if it is greather than 1 second. For the record: In the history, the code was: + if (delay > 1000) + schedule_delayed_work(&(tz->poll_queue), + round_jiffies(msecs_to_jiffies(delay))); + else + schedule_delayed_work(&(tz->poll_queue), + msecs_to_jiffies(delay)); And the initial commit 21bc42ab852549f4a547d18d77e0e4d1b24ffd96: "ACPI: thermal: use round_jiffies when thermal zone polling is enabled" Good catch ! Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Fixes: 17d399cd9c89 ("thermal/core: Precompute the delays from msecs to jiffies") > Fixes: e5f2cda61d06 ("thermal/core: Move thermal_set_delay_jiffies to static") > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/thermal/thermal_core.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -323,11 +323,15 @@ static void thermal_zone_broken_disable( > static void thermal_zone_device_set_polling(struct thermal_zone_device *tz, > unsigned long delay) > { > - if (delay) > - mod_delayed_work(system_freezable_power_efficient_wq, > - &tz->poll_queue, delay); > - else > + if (!delay) { > cancel_delayed_work(&tz->poll_queue); > + return; > + } > + > + if (delay > HZ) > + delay = round_jiffies_relative(delay); > + > + mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, delay); > } > > static void thermal_zone_recheck(struct thermal_zone_device *tz, int error) > @@ -1312,13 +1316,6 @@ void thermal_cooling_device_unregister(s > } > EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister); > > -static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms) > -{ > - *delay_jiffies = msecs_to_jiffies(delay_ms); > - if (delay_ms > 1000) > - *delay_jiffies = round_jiffies(*delay_jiffies); > -} > - > int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp) > { > const struct thermal_trip_desc *td; > @@ -1465,8 +1462,8 @@ thermal_zone_device_register_with_trips( > td->threshold = INT_MAX; > } > > - thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay); > - thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay); > + tz->polling_delay_jiffies = msecs_to_jiffies(polling_delay); > + tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay); > tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY; > > /* sys I/F */ > > >
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -323,11 +323,15 @@ static void thermal_zone_broken_disable( static void thermal_zone_device_set_polling(struct thermal_zone_device *tz, unsigned long delay) { - if (delay) - mod_delayed_work(system_freezable_power_efficient_wq, - &tz->poll_queue, delay); - else + if (!delay) { cancel_delayed_work(&tz->poll_queue); + return; + } + + if (delay > HZ) + delay = round_jiffies_relative(delay); + + mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, delay); } static void thermal_zone_recheck(struct thermal_zone_device *tz, int error) @@ -1312,13 +1316,6 @@ void thermal_cooling_device_unregister(s } EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister); -static void thermal_set_delay_jiffies(unsigned long *delay_jiffies, int delay_ms) -{ - *delay_jiffies = msecs_to_jiffies(delay_ms); - if (delay_ms > 1000) - *delay_jiffies = round_jiffies(*delay_jiffies); -} - int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp) { const struct thermal_trip_desc *td; @@ -1465,8 +1462,8 @@ thermal_zone_device_register_with_trips( td->threshold = INT_MAX; } - thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay); - thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay); + tz->polling_delay_jiffies = msecs_to_jiffies(polling_delay); + tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay); tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY; /* sys I/F */