linux-kernel - Re: [PATCH v1 0/3] thermal/debugfs: Fix and clean up trip point statistics updates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0j5Ja9-vuC9ve9Li=UxATcBtTmdMdhMS1g993XBe1DVqw@mail.gmail.com>
Date: Tue, 23 Apr 2024 14:26:23 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Lukasz Luba <lukasz.luba@....com>, 
	"Rafael J. Wysocki" <rjw@...ysocki.net>, LKML <linux-kernel@...r.kernel.org>, 
	Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v1 0/3] thermal/debugfs: Fix and clean up trip point
 statistics updates

On Mon, Apr 22, 2024 at 6:12 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> On 22/04/2024 17:48, Rafael J. Wysocki wrote:
> > On Mon, Apr 22, 2024 at 5:34 PM Daniel Lezcano
>
> [ ... ]
>
> >> or we should expect at least the residency to be showed even if the
> >> mitigation state is not closed ?
> >
> > Well, in fact the device has already been in that state for some time
> > and the mitigation can continue for a while.
>
> Yes, but when to update the residency time ?
>
> When we cross a trip point point ?
>
> or
>
> When we read the information ?
>
> The former is what we are currently doing AFAIR

Not really.

Records are added by thermal_debug_cdev_state_update() which only runs
when __thermal_cdev_update() is called, ie. from governors.

Moreover, it assumes the initial state to be 0 and checks if the new
state is equal to the current one before doing anything else, so it
will not make a record for the 0 state until the first transition.

> and the latter must add the delta between the last update and the current time for the current
> state, right ?

Yes, and it is doing this already AFAICS.

The problem is that it only creates a record for the old state, so if
the new one is seen for the first time, there will be no record for it
until it changes to some other state.

The appended patch (modulo GMail-induced white space breakage) should
help with this, but the initial state handling needs to be addressed
separately.

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

Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -433,6 +433,14 @@ void thermal_debug_cdev_state_update(con
     }

     cdev_dbg->current_state = new_state;
+
+    /*
+     * Create a record for the new state if it is not there, so its
+     * duration will be printed by cdev_dt_seq_show() as expected if it
+     * runs before the next state transition.
+     */
+    thermal_debugfs_cdev_record_get(thermal_dbg, cdev_dbg->durations,
new_state);
+
     transition = (old_state << 16) | new_state;

     /*