[PATCH v1 0/2] thermal: core: Sanitize polling delay values

[PATCH v1 0/2] thermal: core: Sanitize polling delay values

[Rafael J. Wysocki]

Hi Everyone,

These 2 patches deal with issues that may arise if the polling delay value used
for passive polling is at odds with the polling delay values used otherwise.

The first patch simply eliminates the need to worry about possible negative
polling delay values by changing the data type used for representing them
to unsigend int.

The other patch adds a sanity check to ensure that "passive" polling will not
be less frequent than "regular" polling.

Thanks!

[PATCH v1 1/2] thermal: core: Change passive_delay and polling_delay data type

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It is better to use unsigned int as the data type for the passive_delay
and polling_delay arguments of thermal_zone_device_register_with_trips()
because they are implicitly cast to unsigned int anyway in
thermal_set_delay_jiffies() and if they happen to be negative at that
point, the resulting behavior may not be as desired.

Update the thermal_zone_device_register_with_trips() definition
accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This also allows the second patch to be simpler because it need not
worry about possible negative polling delay values.

---
 drivers/thermal/thermal_core.c |    3 ++-
 include/linux/thermal.h        |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1359,7 +1359,8 @@ thermal_zone_device_register_with_trips(
 					int num_trips, void *devdata,
 					const struct thermal_zone_device_ops *ops,
 					const struct thermal_zone_params *tzp,
-					int passive_delay, int polling_delay)
+					unsigned int passive_delay,
+					unsigned int polling_delay)
 {
 	const struct thermal_trip *trip = trips;
 	struct thermal_zone_device *tz;
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -221,7 +221,8 @@ struct thermal_zone_device *thermal_zone
 					int num_trips, void *devdata,
 					const struct thermal_zone_device_ops *ops,
 					const struct thermal_zone_params *tzp,
-					int passive_delay, int polling_delay);
+					unsigned int passive_delay,
+					unsigned int polling_delay);
 
 struct thermal_zone_device *thermal_tripless_zone_device_register(
 					const char *type,

[PATCH v1 2/2] thermal: core: Add sanity check for polling_delay and passive_delay

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If polling_delay is nonzero and passive_delay is 0, the thermal zone
will use polling except when tz->passive is nonzero, which does not make
sense.

Also if polling_delay is nonzero and passive_delay is greater than
polling_delay, the thermal zone temperature will be updated less often
when tz->passive is nonzero.  This does not make sense either.

Ensure that none of the above will happen.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1440,6 +1440,9 @@ thermal_zone_device_register_with_trips(
 		td->threshold = INT_MAX;
 	}
 
+	if (polling_delay > 0 && passive_delay > polling_delay)
+		passive_delay = polling_delay;
+
 	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
 	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
 

Re: [PATCH v1 2/2] thermal: core: Add sanity check for polling_delay and passive_delay

[Rafael J. Wysocki]

On Fri, Jul 5, 2024 at 9:34 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If polling_delay is nonzero and passive_delay is 0, the thermal zone
> will use polling except when tz->passive is nonzero, which does not make
> sense.
>
> Also if polling_delay is nonzero and passive_delay is greater than
> polling_delay, the thermal zone temperature will be updated less often
> when tz->passive is nonzero.  This does not make sense either.
>
> Ensure that none of the above will happen.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Well, the changelog is OK, but the patch is not.  I'll send a v2 shortly.