On 10/4/24 20:23, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The part of thermal zone initialization carried out under
> thermal_list_lock acquires the thermal zone lock and releases it
> multiple times back and forth which is not really necessary.
>
> Instead of doing this, make it acquire the thermal zone lock once after
> acquiring thermal_list_lock and release it along with that lock.
>
> For this purpose, move all of the code in question to
> thermal_zone_init_complete() introduced previously and provide an
> "unlocked" variant of thermal_zone_cdev_bind() to be invoked from
> there.
>
> Also notice that a thermal zone does not need to be added to
> thermal_tz_list under its own lock, so make the new code acquire
> the thermal zone lock after adding it to the list.
>
> 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/10548633.nUPlyArG6x@rjwysocki.net/
>
> v1 -> v2: Rebase, update the changelog.
>
> ---
> drivers/thermal/thermal_core.c | 39 ++++++++++++++++++++++-----------------
> 1 file changed, 22 insertions(+), 17 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -933,16 +933,14 @@ void print_bind_err_msg(struct thermal_z
> cdev->type, thermal_zone_trip_id(tz, trip), ret);
> }
>
> -static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> - struct thermal_cooling_device *cdev)
> +static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev)
> {
> struct thermal_trip_desc *td;
>
> if (!tz->ops.should_bind)
> return;
>
> - mutex_lock(&tz->lock);
> -
> for_each_trip_desc(tz, td) {
> struct thermal_trip *trip = &td->trip;
> struct cooling_spec c = {
> @@ -959,6 +957,14 @@ static void thermal_zone_cdev_bind(struc
> if (ret)
> print_bind_err_msg(tz, trip, cdev, ret);
> }
> +}
> +
> +static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> + struct thermal_cooling_device *cdev)
> +{
> + mutex_lock(&tz->lock);
> +
> + __thermal_zone_cdev_bind(tz, cdev);
>
> mutex_unlock(&tz->lock);
> }
> @@ -1336,8 +1342,18 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
>
> static void thermal_zone_init_complete(struct thermal_zone_device *tz)
> {
> + struct thermal_cooling_device *cdev;
> +
> + mutex_lock(&thermal_list_lock);
> +
> + list_add_tail(&tz->node, &thermal_tz_list);
> +
> mutex_lock(&tz->lock);
>
> + /* Bind cooling devices for this zone. */
> + list_for_each_entry(cdev, &thermal_cdev_list, node)
> + __thermal_zone_cdev_bind(tz, cdev);
> +
> tz->state &= ~TZ_STATE_FLAG_INIT;
> /*
> * If system suspend or resume is in progress at this point, the
> @@ -1350,6 +1366,8 @@ static void thermal_zone_init_complete(s
> __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> mutex_unlock(&tz->lock);
> +
> + mutex_unlock(&thermal_list_lock);
> }
>
> /**
> @@ -1386,7 +1404,6 @@ thermal_zone_device_register_with_trips(
> unsigned int polling_delay)
> {
> const struct thermal_trip *trip = trips;
> - struct thermal_cooling_device *cdev;
> struct thermal_zone_device *tz;
> struct thermal_trip_desc *td;
> int id;
> @@ -1514,20 +1531,8 @@ thermal_zone_device_register_with_trips(
> goto unregister;
> }
>
> - mutex_lock(&thermal_list_lock);
> -
> - mutex_lock(&tz->lock);
> - list_add_tail(&tz->node, &thermal_tz_list);
> - mutex_unlock(&tz->lock);
> -
> - /* Bind cooling devices for this zone */
> - list_for_each_entry(cdev, &thermal_cdev_list, node)
> - thermal_zone_cdev_bind(tz, cdev);
> -
> thermal_zone_init_complete(tz);
>
> - mutex_unlock(&thermal_list_lock);
> -
> thermal_notify_tz_create(tz);
>
> thermal_debug_tz_add(tz);
>
>
>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>