On 10/4/24 20:30, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> In analogy with a previous change in the thermal zone initialization
> path, to avoid acquiring the thermal zone lock and releasing it multiple
> times back and forth unnecessarily, move all of the code running under
> thermal_list_lock in thermal_zone_device_unregister() into a new
> function called thermal_zone_exit() and make the latter acquire the
> thermal zone lock only once and release it along with thermal_list_lock.
>
> For this purpose, provide an "unlocked" variant of
> thermal_zone_cdev_unbind() to be called by thermal_zone_exit() under the
> thermal zone lock.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> This is a new iteration of
>
> https://lore.kernel.org/linux-pm/7729094.EvYhyI6sBW@rjwysocki.net/
>
> v1 -> v2: Rebase, use list_del_init() for removing thermal zones from the
> global list so that the new list_emty() check doesn't blow up.
>
> ---
> drivers/thermal/thermal_core.c | 66 ++++++++++++++++++++++++-----------------
> 1 file changed, 39 insertions(+), 27 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1266,15 +1266,21 @@ unlock_list:
> }
> EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
>
> -static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> - struct thermal_cooling_device *cdev)
> +static void __thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev)
> {
> struct thermal_trip_desc *td;
>
> - mutex_lock(&tz->lock);
> -
> for_each_trip_desc(tz, td)
> thermal_unbind_cdev_from_trip(tz, &td->trip, cdev);
> +}
> +
> +static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev)
> +{
> + mutex_lock(&tz->lock);
> +
> + __thermal_zone_cdev_unbind(tz, cdev);
>
> mutex_unlock(&tz->lock);
> }
> @@ -1588,43 +1594,49 @@ struct device *thermal_zone_device(struc
> }
> EXPORT_SYMBOL_GPL(thermal_zone_device);
>
> -/**
> - * thermal_zone_device_unregister - removes the registered thermal zone device
> - * @tz: the thermal zone device to remove
> - */
> -void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> +static bool thermal_zone_exit(struct thermal_zone_device *tz)
> {
> struct thermal_cooling_device *cdev;
> - struct thermal_zone_device *pos = NULL;
> -
> - if (!tz)
> - return;
> -
> - thermal_debug_tz_remove(tz);
> + bool ret = true;
>
> mutex_lock(&thermal_list_lock);
> - list_for_each_entry(pos, &thermal_tz_list, node)
> - if (pos == tz)
> - break;
> - if (pos != tz) {
> - /* thermal zone device not found */
> - mutex_unlock(&thermal_list_lock);
> - return;
> +
> + if (list_empty(&tz->node)) {
> + ret = false;
> + goto unlock;
> }
>
> mutex_lock(&tz->lock);
>
> tz->state |= TZ_STATE_FLAG_EXIT;
> - list_del(&tz->node);
> -
> - mutex_unlock(&tz->lock);
> + list_del_init(&tz->node);
>
> - /* Unbind all cdevs associated with 'this' thermal zone */
> + /* Unbind all cdevs associated with this thermal zone. */
> list_for_each_entry(cdev, &thermal_cdev_list, node)
> - thermal_zone_cdev_unbind(tz, cdev);
> + __thermal_zone_cdev_unbind(tz, cdev);
> +
> + mutex_unlock(&tz->lock);
>
> +unlock:
> mutex_unlock(&thermal_list_lock);
>
> + return ret;
> +}
> +
> +/**
> + * thermal_zone_device_unregister - removes the registered thermal zone device
> + * @tz: the thermal zone device to remove
> + */
> +void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> +{
> + if (!tz)
> + return;
> +
> + thermal_debug_tz_remove(tz);
> +
> + if (!thermal_zone_exit(tz))
> + return;
> +
> cancel_delayed_work_sync(&tz->poll_queue);
>
> thermal_set_governor(tz, NULL);
>
>
>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>