Re: [PATCH v2 03/11] thermal: core: Separate code running under thermal_list_lock

Lukasz Luba
2024-10-22T21:08:04Z

On 10/10/24 23:09, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> To prepare for a subsequent change that will switch over the thermal
> core to using a mutex guard for thermal_list_lock management, move the
> code running under thermal_list_lock during the initialization and
> unregistration of cooling devices into separate functions.
>
> While at it, drop some comments that do not add value.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> This is a resend of
>
> https://lore.kernel.org/linux-pm/1822468.VLH7GnMWUR@rjwysocki.net/
>
> ---
> drivers/thermal/thermal_core.c | 64 +++++++++++++++++++++++------------------
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -965,6 +965,20 @@ static void thermal_zone_cdev_bind(struc
> __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> }
>
> +static void thermal_cooling_device_init_complete(struct thermal_cooling_device *cdev)
> +{
> + struct thermal_zone_device *tz;
> +
> + mutex_lock(&thermal_list_lock);
> +
> + list_add(&cdev->node, &thermal_cdev_list);
> +
> + list_for_each_entry(tz, &thermal_tz_list, node)
> + thermal_zone_cdev_bind(tz, cdev);
> +
> + mutex_unlock(&thermal_list_lock);
> +}
> +
> /**
> * __thermal_cooling_device_register() - register a new thermal cooling device
> * @np: a pointer to a device tree node.
> @@ -987,7 +1001,6 @@ __thermal_cooling_device_register(struct
> const struct thermal_cooling_device_ops *ops)
> {
> struct thermal_cooling_device *cdev;
> - struct thermal_zone_device *pos;
> unsigned long current_state;
> int id, ret;
>
> @@ -1054,16 +1067,7 @@ __thermal_cooling_device_register(struct
> if (current_state <= cdev->max_state)
> thermal_debug_cdev_add(cdev, current_state);
>
> - /* Add 'this' new cdev to the global cdev list */
> - mutex_lock(&thermal_list_lock);
> -
> - list_add(&cdev->node, &thermal_cdev_list);
> -
> - /* Update binding information for 'this' new cdev */
> - list_for_each_entry(pos, &thermal_tz_list, node)
> - thermal_zone_cdev_bind(pos, cdev);
> -
> - mutex_unlock(&thermal_list_lock);
> + thermal_cooling_device_init_complete(cdev);
>
> return cdev;
>
> @@ -1274,38 +1278,42 @@ static void thermal_zone_cdev_unbind(str
> __thermal_zone_cdev_unbind(tz, cdev);
> }
>
> -/**
> - * thermal_cooling_device_unregister - removes a thermal cooling device
> - * @cdev: the thermal cooling device to remove.
> - *
> - * thermal_cooling_device_unregister() must be called when a registered
> - * thermal cooling device is no longer needed.
> - */
> -void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
> +static bool thermal_cooling_device_exit(struct thermal_cooling_device *cdev)
> {
> struct thermal_zone_device *tz;
> -
> - if (!cdev)
> - return;
> -
> - thermal_debug_cdev_remove(cdev);
> + bool ret = true;
>
> mutex_lock(&thermal_list_lock);
>
> if (!thermal_cooling_device_present(cdev)) {
> - mutex_unlock(&thermal_list_lock);
> - return;
> + ret = false;
> + goto unlock;
> }
>
> list_del(&cdev->node);
>
> - /* Unbind all thermal zones associated with 'this' cdev */
> list_for_each_entry(tz, &thermal_tz_list, node)
> thermal_zone_cdev_unbind(tz, cdev);
>
> +unlock:
> mutex_unlock(&thermal_list_lock);
>
> - device_unregister(&cdev->device);
> + return ret;
> +}
> +
> +/**
> + * thermal_cooling_device_unregister() - removes a thermal cooling device
> + * @cdev: Thermal cooling device to remove.
> + */
> +void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
> +{
> + if (!cdev)
> + return;
> +
> + thermal_debug_cdev_remove(cdev);
> +
> + if (thermal_cooling_device_exit(cdev))
> + device_unregister(&cdev->device);
> }
> EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);
>
>
>
>

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>