Message ID | 9360231.CDJkKcVGEf@rjwysocki.net |
---|---|
State | New |
Headers | show |
Series | thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit | expand |
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>
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