Re: [PATCH v1 1/3] thermal: core: Make thermal_zone_device_unregister() return after freeing the zone

Daniel Lezcano
2023-12-11T16:28:43Z

On 08/12/2023 20:13, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Make thermal_zone_device_unregister() wait until all of the references
> to the given thermal zone object have been dropped and free it before
> returning.
>
> This guarantees that when thermal_zone_device_unregister() returns,
> there is no leftover activity regarding the thermal zone in question
> which is required by some of its callers (for instance, modular driver
> code that wants to know when it is safe to let the module go away).
>
> Subsequently, this will allow some confusing device_is_registered()
> checks to be dropped from the thermal sysfs and core code.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Definitively agree on the change

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Would it make sense to use kref_get/put ?

> drivers/thermal/thermal_core.c | 6 +++++-
> include/linux/thermal.h | 2 ++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -822,7 +822,7 @@ static void thermal_release(struct devic
> tz = to_thermal_zone(dev);
> thermal_zone_destroy_device_groups(tz);
> mutex_destroy(&tz->lock);
> - kfree(tz);
> + complete(&tz->removal);
> } else if (!strncmp(dev_name(dev), "cooling_device",
> sizeof("cooling_device") - 1)) {
> cdev = to_cooling_device(dev);
> @@ -1315,6 +1315,7 @@ thermal_zone_device_register_with_trips(
> INIT_LIST_HEAD(&tz->thermal_instances);
> ida_init(&tz->ida);
> mutex_init(&tz->lock);
> + init_completion(&tz->removal);
> id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
> if (id < 0) {
> result = id;
> @@ -1494,6 +1495,9 @@ void thermal_zone_device_unregister(stru
> put_device(&tz->device);
>
> thermal_notify_tz_delete(tz_id);
> +
> + wait_for_completion(&tz->removal);
> + kfree(tz);
> }
> EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -117,6 +117,7 @@ struct thermal_cooling_device {
> * @id: unique id number for each thermal zone
> * @type: the thermal zone device type
> * @device: &struct device for this thermal zone
> + * @removal: removal completion
> * @trip_temp_attrs: attributes for trip points for sysfs: trip temperature
> * @trip_type_attrs: attributes for trip points for sysfs: trip type
> * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
> @@ -156,6 +157,7 @@ struct thermal_zone_device {
> int id;
> char type[THERMAL_NAME_LENGTH];
> struct device device;
> + struct completion removal;
> struct attribute_group trips_attribute_group;
> struct thermal_attr *trip_temp_attrs;
> struct thermal_attr *trip_type_attrs;
>
>
>

--
<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 |