Messages in this thread | | | Date | Wed, 16 Aug 2023 18:25:30 +0200 | Subject | Re: [PATCH v5 05/11] ACPI: thermal: Carry out trip point updates under zone lock | From | Daniel Lezcano <> |
| |
On 07/08/2023 20:08, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > There is a race condition between acpi_thermal_trips_update() and > acpi_thermal_check_fn(), because the trip points may get updated while > the latter is running which in theory may lead to inconsistent results. > For example, if two trips are updated together, using the temperature > value of one of them from before the update and the temperature value > of the other one from after the update may not lead to the expected > outcome. > > Moreover, if thermal_get_trend() runs when a trip points update is in > progress, it may end up using stale trip point temperatures. > > To address this, make acpi_thermal_trips_update() call > thermal_zone_device_adjust() to carry out the trip points update and > provide a new acpi_thermal_adjust_thermal_zone() wrapper around > __acpi_thermal_trips_update() as the callback function for the latter. > > While at it, change the acpi_thermal_trips_update() return data type > to void as that function always returns 0 anyway. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > ---
[ ... ]
> { > - int i, ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT); > bool valid; > + int i; > > - if (ret) > - return ret; > + __acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT); > > valid = tz->trips.critical.valid | > tz->trips.hot.valid | > @@ -710,6 +732,7 @@ static struct thermal_zone_device_ops ac > .get_trend = thermal_get_trend, > .hot = acpi_thermal_zone_device_hot, > .critical = acpi_thermal_zone_device_critical, > + .update = acpi_thermal_adjust_thermal_zone,
It is too bad we have to add a callback in the core code just for this driver.
I'm wondering if it is not possible to get rid of it ?
Is it possible to use an internal lock for the ACPI driver to solve the race issue above ?
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |