[PATCH v2 08/12] thermal: core: Consolidate thermal zone locking in the exit path

Posted by Rafael J. Wysocki 1 month, 3 weeks ago

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

Re: [PATCH v2 08/12] thermal: core: Consolidate thermal zone locking in the exit path

Posted by Lukasz Luba 1 month, 1 week ago


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>