thermal/debugfs: Fix handling of cdev states and mitigation episodes in progress

From: Rafael J. Wysocki <[email protected]>

Because thermal_debug_cdev_state_update() only creates a duration record for the old state of a cooling device, if its new state is used for the first time, there will be no record for it and cdev_dt_seq_show() will not print the duration information for it even though it contains code to compute the duration value in that case.

Address this by making thermal_debug_cdev_state_update() create a duration record for the new state if there is none.

Fixes 755113d76786 ("thermal/debugfs: Add thermal cooling device debugfs information")

Reported-by Lukasz Luba <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  drivers/thermal/thermal_debugfs.c |    8 ++++++++
  1 file changed, 8 insertions(+)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================

drivers/thermal/thermal_debugfs.c +1 -0

@@ -433,6 +433,14 @@ void thermal_debug_cdev_state_update(con
  	}
 
  	cdev_dbg->current_state = new_state;
+

From: Rafael J. Wysocki <[email protected]>

If cdev_dt_seq_show() runs before the first state transition of a cooling device, it will not print any state residency information for it, even though it might be reasonably expected to print residency information for the initial state of the cooling device.

For this reason, rearrange the code to get the initial state of a cooling device at the registration time and pass it to thermal_debug_cdev_add(), so that the latter can create a duration record for that state which will allow cdev_dt_seq_show() to print its residency information.

Fixes 755113d76786 ("thermal/debugfs: Add thermal cooling device debugfs information")

Reported-by Lukasz Luba <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  drivers/thermal/thermal_core.c    |    9 +++++++--
  drivers/thermal/thermal_debugfs.c |   12 ++++++++++--
  drivers/thermal/thermal_debugfs.h |    4 ++--
  3 files changed, 19 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================

drivers/thermal/thermal_core.c +7 -2

@@ -935,6 +935,7 @@ __thermal_cooling_device_register(struct
  {
  	struct thermal_cooling_device *cdev;
  	struct thermal_zone_device *pos = NULL;
+	unsigned long current_state;
  	int id, ret;
 
  	if (!ops || !ops->get_max_state || !ops->get_cur_state ||

@@ -972,6 +973,10 @@ __thermal_cooling_device_register(struct
  	if (ret)
  		goto out_cdev_type;
 
+	ret = cdev->ops->get_cur_state(cdev, &current_state);
+	if (ret)
+		goto out_cdev_type;
+
  	thermal_cooling_device_setup_sysfs(cdev);
 
  	ret = dev_set_name(&cdev->device, "cooling_device%d", cdev->id);

@@ -985,6 +990,8 @@ __thermal_cooling_device_register(struct
  		return ERR_PTR(ret);
  	}
 
+	thermal_debug_cdev_add(cdev, current_state);
+
  	/* Add 'this' new cdev to the global cdev list */
  	mutex_lock(&thermal_list_lock);
 

@@ -1000,8 +1007,6 @@ __thermal_cooling_device_register(struct
 
  	mutex_unlock(&thermal_list_lock);
 
-	thermal_debug_cdev_add(cdev);
-
  	return cdev;
 
  out_cooling_dev:

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================

drivers/thermal/thermal_debugfs.c +10 -2

@@ -466,8 +466,9 @@ void thermal_debug_cdev_state_update(con
   * Allocates a cooling device object for debug, initializes the
   * statistics and create the entries in sysfs.
   * @cdev: a pointer to a cooling device
+ * @state: current state of the cooling device
   */
-void thermal_debug_cdev_add(struct thermal_cooling_device *cdev)
+void thermal_debug_cdev_add(struct thermal_cooling_device *cdev, int state)
  {
  	struct thermal_debugfs *thermal_dbg;
  	struct cdev_debugfs *cdev_dbg;

@@ -484,9 +485,16 @@ void thermal_debug_cdev_add(struct therm
  		INIT_LIST_HEAD(&cdev_dbg->durations[i]);
  	}
 
-	cdev_dbg->current_state = 0;
+	cdev_dbg->current_state = state;
  	cdev_dbg->timestamp = ktime_get();
 
+	/*
+	 * Create a record for the initial cooling device state, so its
+	 * duration will be printed by cdev_dt_seq_show() as expected if it
+	 * runs before the first state transition.
+	 */
+	thermal_debugfs_cdev_record_get(thermal_dbg, cdev_dbg->durations, state);
+
  	debugfs_create_file("trans_table", 0400, thermal_dbg->d_top,
  			    thermal_dbg, &tt_fops);
 

Index: linux-pm/drivers/thermal/thermal_debugfs.h
===================================================================

drivers/thermal/thermal_debugfs.h +1 -1

@@ -2,7 +2,7 @@
 
  #ifdef CONFIG_THERMAL_DEBUGFS
  void thermal_debug_init(void);
-void thermal_debug_cdev_add(struct thermal_cooling_device *cdev);
+void thermal_debug_cdev_add(struct thermal_cooling_device *cdev, int state);
  void thermal_debug_cdev_remove(struct thermal_cooling_device *cdev);
  void thermal_debug_cdev_state_update(const struct thermal_cooling_device *cdev, int state);
  void thermal_debug_tz_add(struct thermal_zone_device *tz);

From: Rafael J. Wysocki <[email protected]>

If a thermal mitigation event is in progress, its duration value has not been updated yet, so 0 will be printed as the event duration by tze_seq_show() which is confusing.

Avoid doing that by marking the beginning of the event with the KTIME_MIN duration value and making tze_seq_show() compute the current event duration on the fly, in which case '>' will be printed instead of '=' in the event duration value field.

Similarly, for trip points that have been crossed on the down, mark the end of mitigation with the KTIME_MAX timestamp value and make tze_seq_show() compute the current duration on the fly for the trip points still involved in the mitigation, in which cases the duration value printed by it will be prepended with a '>' character.

Fixes 7ef01f228c9f ("thermal/debugfs: Add thermal debugfs information for mitigation episodes")

Signed-off-by Rafael J. Wysocki <[email protected]>

---
  drivers/thermal/thermal_debugfs.c |   39 ++++++++++++++++++++++++++++++++------
  1 file changed, 33 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================

drivers/thermal/thermal_debugfs.c +10 -3

@@ -552,6 +552,7 @@ static struct tz_episode *thermal_debugf
 
  	INIT_LIST_HEAD(&tze->node);
  	tze->timestamp = now;
+	tze->duration = KTIME_MIN;
 
  	for (i = 0; i < tz->num_trips; i++) {
  		tze->trip_stats[i].min = INT_MAX;

@@ -680,6 +681,9 @@ void thermal_debug_tz_trip_down(struct t
  	tze->trip_stats[trip_id].duration =
  		ktime_add(delta, tze->trip_stats[trip_id].duration);
 
+	/* Mark the end of mitigation for this trip point. */
+	tze->trip_stats[trip_id].timestamp = KTIME_MAX;
+
  	/*
  	 * This event closes the mitigation as we are crossing the
  	 * last trip point the way down.

@@ -754,15 +758,25 @@ static int tze_seq_show(struct seq_file
  	struct thermal_trip_desc *td;
  	struct tz_episode *tze;
  	const char *type;
+	u64 duration_ms;
  	int trip_id;
+	char c;
 
  	tze = list_entry((struct list_head *)v, struct tz_episode, node);
 
-	seq_printf(s, ",-Mitigation at %lluus, duration=%llums\n",
-		   ktime_to_us(tze->timestamp),
-		   ktime_to_ms(tze->duration));
+	if (tze->duration == KTIME_MIN) {
+		/* Mitigation in progress. */
+		duration_ms = ktime_to_ms(ktime_sub(ktime_get(), tze->timestamp));
+		c = '>';

drivers/thermal/thermal_debugfs.c +13 -1

  	for_each_trip_desc(tz, td) {
  		const struct thermal_trip *trip = &td->trip;

@@ -794,12 +808,25 @@ static int tze_seq_show(struct seq_file
  		else
  			type = "hot";
 
-		seq_printf(s, "| %*d | %*s | %*d | %*d | %*lld | %*d | %*d | %*d |\n",
+		if (trip_stats->timestamp != KTIME_MAX) {
+			/* Mitigation in progress. */
+			ktime_t delta = ktime_sub(ktime_get(),
+						  trip_stats->timestamp);
+
+			delta = ktime_add(delta, trip_stats->duration);
+			duration_ms = ktime_to_ms(delta);
+			c = '>';
+		} else {
+			duration_ms = ktime_to_ms(trip_stats->duration);
+			c = ' ';
+		}
+

Hi Everyone,

This is an update of

https://lore.kernel.org/linux-pm/5774279.DvuYhMxLoT@kreacher/

and the only non-trivial difference between it and the v1 is a small rebase of the second patch (the v1 of which didn't apply).

It generally has been based on top of

https://lore.kernel.org/linux-pm/12427744.O9o76ZdvQC@kreacher/

but it should apply on top of the linux-next branch in linux-pm.git as well.

It is present in the thermal-core-next branch in that tree, along with the above series.

Thanks!

Hi Rafael,

Quoted 18 lines · Rafael J. Wysocki

Hi Everyone,

This is an update of

https://lore.kernel.org/linux-pm/5774279.DvuYhMxLoT@kreacher/

and the only non-trivial difference between it and the v1 is a small rebase of the second patch (the v1 of which didn't apply).

It generally has been based on top of

https://lore.kernel.org/linux-pm/12427744.O9o76ZdvQC@kreacher/

but it should apply on top of the linux-next branch in linux-pm.git as well.

It is present in the thermal-core-next branch in that tree, along with the above series.

Quoted 4 lines

Thanks!

I have also tested the patches, so feel free to add the tag as well:

Tested-by Lukasz Luba <[email protected]>