2023-01-18 10:05:58

by Viresh Kumar

Subject: [PATCH 2/3] thermal: core: Move cdev cleanup to thermal_release()

thermal_release() already frees cdev, let it do rest of the cleanup as
well in order to simplify the error paths in
__thermal_cooling_device_register().

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/thermal/thermal_core.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 77bd47d976a2..1fb109a97ff6 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -774,6 +774,9 @@ static void thermal_release(struct device *dev)
} else if (!strncmp(dev_name(dev), "cooling_device",
sizeof("cooling_device") - 1)) {
cdev = to_cooling_device(dev);
+ thermal_cooling_device_destroy_sysfs(cdev);
+ kfree(cdev->type);
+ ida_free(&thermal_cdev_ida, cdev->id);
kfree(cdev);
}
}
@@ -909,23 +912,21 @@ __thermal_cooling_device_register(struct device_node *np,
cdev->devdata = devdata;

ret = cdev->ops->get_max_state(cdev, &cdev->max_state);
- if (ret) {
- kfree(cdev->type);
- goto out_ida_remove;
- }
+ if (ret)
+ goto out_cdev_type;

thermal_cooling_device_setup_sysfs(cdev);

ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
- if (ret) {
- kfree(cdev->type);
- thermal_cooling_device_destroy_sysfs(cdev);
- goto out_ida_remove;
- }
+ if (ret)
+ goto out_cooling_dev;

ret = device_register(&cdev->device);
- if (ret)
- goto out_kfree_type;
+ if (ret) {
+ /* thermal_release() handles rest of the cleanup */
+ put_device(&cdev->device);
+ return ERR_PTR(ret);
+ }

/* Add 'this' new cdev to the global cdev list */
mutex_lock(&thermal_list_lock);
@@ -944,13 +945,10 @@ __thermal_cooling_device_register(struct device_node *np,

return cdev;

-out_kfree_type:
+out_cooling_dev:
thermal_cooling_device_destroy_sysfs(cdev);
+out_cdev_type:
kfree(cdev->type);
- put_device(&cdev->device);
-
- /* thermal_release() takes care of the rest */
- cdev = NULL;
out_ida_remove:
ida_free(&thermal_cdev_ida, id);
out_kfree_cdev:
@@ -1111,10 +1109,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)

mutex_unlock(&thermal_list_lock);

- ida_free(&thermal_cdev_ida, cdev->id);
device_del(&cdev->device);
- thermal_cooling_device_destroy_sysfs(cdev);
- kfree(cdev->type);
put_device(&cdev->device);
}
EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);
--
2.31.1.272.g89b43f80a514

2023-01-19 08:54:20

by Yang Yingliang

Subject: Re: [PATCH 2/3] thermal: core: Move cdev cleanup to thermal_release()

On 2023/1/18 16:38, Viresh Kumar wrote:
> thermal_release() already frees cdev, let it do rest of the cleanup as
> well in order to simplify the error paths in
> __thermal_cooling_device_register().
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
Reviewed-by: Yang Yingliang <[email protected]>
> drivers/thermal/thermal_core.c | 33 ++++++++++++++-------------------
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 77bd47d976a2..1fb109a97ff6 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -774,6 +774,9 @@ static void thermal_release(struct device *dev)
> } else if (!strncmp(dev_name(dev), "cooling_device",
> sizeof("cooling_device") - 1)) {
> cdev = to_cooling_device(dev);
> + thermal_cooling_device_destroy_sysfs(cdev);
> + kfree(cdev->type);
> + ida_free(&thermal_cdev_ida, cdev->id);
> kfree(cdev);
> }
> }
> @@ -909,23 +912,21 @@ __thermal_cooling_device_register(struct device_node *np,
> cdev->devdata = devdata;
>
> ret = cdev->ops->get_max_state(cdev, &cdev->max_state);
> - if (ret) {
> - kfree(cdev->type);
> - goto out_ida_remove;
> - }
> + if (ret)
> + goto out_cdev_type;
>
> thermal_cooling_device_setup_sysfs(cdev);
>
> ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
> - if (ret) {
> - kfree(cdev->type);
> - thermal_cooling_device_destroy_sysfs(cdev);
> - goto out_ida_remove;
> - }
> + if (ret)
> + goto out_cooling_dev;
>
> ret = device_register(&cdev->device);
> - if (ret)
> - goto out_kfree_type;
> + if (ret) {
> + /* thermal_release() handles rest of the cleanup */
> + put_device(&cdev->device);
> + return ERR_PTR(ret);
> + }
>
> /* Add 'this' new cdev to the global cdev list */
> mutex_lock(&thermal_list_lock);
> @@ -944,13 +945,10 @@ __thermal_cooling_device_register(struct device_node *np,
>
> return cdev;
>
> -out_kfree_type:
> +out_cooling_dev:
> thermal_cooling_device_destroy_sysfs(cdev);
> +out_cdev_type:
> kfree(cdev->type);
> - put_device(&cdev->device);
> -
> - /* thermal_release() takes care of the rest */
> - cdev = NULL;
> out_ida_remove:
> ida_free(&thermal_cdev_ida, id);
> out_kfree_cdev:
> @@ -1111,10 +1109,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
>
> mutex_unlock(&thermal_list_lock);
>
> - ida_free(&thermal_cdev_ida, cdev->id);
> device_del(&cdev->device);
> - thermal_cooling_device_destroy_sysfs(cdev);
> - kfree(cdev->type);
> put_device(&cdev->device);
> }
> EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);