diff mbox series

[v2,04/12] thermal: core: Mark thermal zones as initializing to start with

Message ID 9360231.CDJkKcVGEf@rjwysocki.net (mailing list archive)
State In Next
Delegated to: Rafael Wysocki
Headers show
Series thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit | expand

Commit Message

Rafael J. Wysocki Oct. 4, 2024, 7:15 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After thermal_zone_device_register_with_trips() has called
device_register() and it has registered the new thermal zone device
with the driver core, user space may access its sysfs attributes and,
among other things, it may enable the thermal zone before it is ready.

To address this, introduce a new thermal zone state flag for
initialization and set it before calling device_register() in
thermal_zone_device_register_with_trips().  This causes
__thermal_zone_device_update() to return early until the new flag
is cleared.

To clear it when the thermal zone is ready, introduce a new
function called thermal_zone_init_complete() that will also invoke
__thermal_zone_device_update() after clearing that flag (both under the
thernal zone lock) and make thermal_zone_device_register_with_trips()
call the new function instead of checking need_update and calling
thermal_zone_device_update() when it is set.

After this change, if user space enables the thermal zone prematurely,
__thermal_zone_device_update() will return early for it until
thermal_zone_init_complete() is called.  In turn, if the thermal zone
is not enabled by user space before thermal_zone_init_complete() is
called, the __thermal_zone_device_update() call in it will return early
because the thermal zone has not been enabled yet, but that function
will be invoked again by thermal_zone_device_set_mode() when the thermal
zone is enabled and it will not return early this time.

The checking of need_update is not necessary any more because the
__thermal_zone_device_update() calls potentially triggered by cooling
device binding take place before calling thermal_zone_init_complete(),
so they all will return early, which means that
thermal_zone_init_complete() must call __thermal_zone_device_update()
in case the thermal zone is enabled prematurely by user space.

Fixes: 203d3d4aa482 ("the generic thermal sysfs driver")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/2973309.e9J7NaK4W3@rjwysocki.net/

v1 -> v2: Rebase and add a Fixes tag.

---
 drivers/thermal/thermal_core.c |   16 +++++++++++++---
 drivers/thermal/thermal_core.h |    1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

Comments

Lukasz Luba Oct. 21, 2024, 10:26 p.m. UTC | #1 
On 10/4/24 20:15, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> After thermal_zone_device_register_with_trips() has called
> device_register() and it has registered the new thermal zone device
> with the driver core, user space may access its sysfs attributes and,
> among other things, it may enable the thermal zone before it is ready.
> 
> To address this, introduce a new thermal zone state flag for
> initialization and set it before calling device_register() in
> thermal_zone_device_register_with_trips().  This causes
> __thermal_zone_device_update() to return early until the new flag
> is cleared.
> 
> To clear it when the thermal zone is ready, introduce a new
> function called thermal_zone_init_complete() that will also invoke
> __thermal_zone_device_update() after clearing that flag (both under the
> thernal zone lock) and make thermal_zone_device_register_with_trips()
> call the new function instead of checking need_update and calling
> thermal_zone_device_update() when it is set.
> 
> After this change, if user space enables the thermal zone prematurely,
> __thermal_zone_device_update() will return early for it until
> thermal_zone_init_complete() is called.  In turn, if the thermal zone
> is not enabled by user space before thermal_zone_init_complete() is
> called, the __thermal_zone_device_update() call in it will return early
> because the thermal zone has not been enabled yet, but that function
> will be invoked again by thermal_zone_device_set_mode() when the thermal
> zone is enabled and it will not return early this time.
> 
> The checking of need_update is not necessary any more because the
> __thermal_zone_device_update() calls potentially triggered by cooling
> device binding take place before calling thermal_zone_init_complete(),
> so they all will return early, which means that
> thermal_zone_init_complete() must call __thermal_zone_device_update()
> in case the thermal zone is enabled prematurely by user space.
> 
> Fixes: 203d3d4aa482 ("the generic thermal sysfs driver")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/2973309.e9J7NaK4W3@rjwysocki.net/
> 
> v1 -> v2: Rebase and add a Fixes tag.
> 
> ---
>   drivers/thermal/thermal_core.c |   16 +++++++++++++---
>   drivers/thermal/thermal_core.h |    1 +
>   2 files changed, 14 insertions(+), 3 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1332,6 +1332,16 @@ int thermal_zone_get_crit_temp(struct th
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>   
> +static void thermal_zone_init_complete(struct thermal_zone_device *tz)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	tz->state &= ~TZ_STATE_FLAG_INIT;
> +	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +
> +	mutex_unlock(&tz->lock);
> +}
> +
>   /**
>    * thermal_zone_device_register_with_trips() - register a new thermal zone device
>    * @type:	the thermal zone device type
> @@ -1451,6 +1461,8 @@ thermal_zone_device_register_with_trips(
>   	tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay);
>   	tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY;
>   
> +	tz->state = TZ_STATE_FLAG_INIT;
> +
>   	/* sys I/F */
>   	/* Add nodes that are always present via .groups */
>   	result = thermal_zone_create_device_groups(tz);
> @@ -1504,9 +1516,7 @@ thermal_zone_device_register_with_trips(
>   
>   	mutex_unlock(&thermal_list_lock);
>   
> -	/* Update the new thermal zone and mark it as already updated. */
> -	if (atomic_cmpxchg(&tz->need_update, 1, 0))
> -		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +	thermal_zone_init_complete(tz);
>   
>   	thermal_notify_tz_create(tz);
>   
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -63,6 +63,7 @@ struct thermal_governor {
>   
>   #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
>   #define	TZ_STATE_FLAG_RESUMING	BIT(1)
> +#define	TZ_STATE_FLAG_INIT	BIT(2)
>   
>   #define TZ_STATE_READY		0
>   
> 
> 
> 

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

Patch

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1332,6 +1332,16 @@  int thermal_zone_get_crit_temp(struct th
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
 
+static void thermal_zone_init_complete(struct thermal_zone_device *tz)
+{
+	mutex_lock(&tz->lock);
+
+	tz->state &= ~TZ_STATE_FLAG_INIT;
+	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+
+	mutex_unlock(&tz->lock);
+}
+
 /**
  * thermal_zone_device_register_with_trips() - register a new thermal zone device
  * @type:	the thermal zone device type
@@ -1451,6 +1461,8 @@  thermal_zone_device_register_with_trips(
 	tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay);
 	tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY;
 
+	tz->state = TZ_STATE_FLAG_INIT;
+
 	/* sys I/F */
 	/* Add nodes that are always present via .groups */
 	result = thermal_zone_create_device_groups(tz);
@@ -1504,9 +1516,7 @@  thermal_zone_device_register_with_trips(
 
 	mutex_unlock(&thermal_list_lock);
 
-	/* Update the new thermal zone and mark it as already updated. */
-	if (atomic_cmpxchg(&tz->need_update, 1, 0))
-		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+	thermal_zone_init_complete(tz);
 
 	thermal_notify_tz_create(tz);
 
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -63,6 +63,7 @@  struct thermal_governor {
 
 #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
 #define	TZ_STATE_FLAG_RESUMING	BIT(1)
+#define	TZ_STATE_FLAG_INIT	BIT(2)
 
 #define TZ_STATE_READY		0