linux-kernel - Re: [PATCH v2] thermal: core: Allow thermal zones to tell the core to ignore them

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: <20240717212421.GA1191@sol.localdomain>
Date: Wed, 17 Jul 2024 14:24:21 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Linux PM <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Lukasz Luba <lukasz.luba@....com>,
	Miri Korenblit <miriam.rachel.korenblit@...el.com>,
	Kalle Valo <kvalo@...nel.org>, linux-wireless@...r.kernel.org,
	Stefan Lippers-Hollmann <s.l-h@....de>,
	Oleksandr Natalenko <oleksandr@...alenko.name>,
	Ben Greear <greearb@...delatech.com>
Subject: Re: [PATCH v2] thermal: core: Allow thermal zones to tell the core
 to ignore them

On Wed, Jul 17, 2024 at 09:45:02PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> The iwlwifi wireless driver registers a thermal zone that is only needed
> when the network interface handled by it is up and it wants that thermal
> zone to be effectively ignored by the core otherwise.
> 
> Before commit a8a261774466 ("thermal: core: Call monitor_thermal_zone()
> if zone temperature is invalid") that could be achieved by returning
> an error code from the thermal zone's .get_temp() callback because the
> core did not really handle errors returned by it almost at all.
> However, commit a8a261774466 made the core attempt to recover from the
> situation in which the temperature of a thermal zone cannot be
> determined due to errors returned by its .get_temp() and is always
> invalid from the core's perspective.
> 
> That was done because there are thermal zones in which .get_temp()
> returns errors to start with due to some difficulties related to the
> initialization ordering, but then it will start to produce valid
> temperature values at one point.
> 
> Unfortunately, the simple approach taken by commit a8a261774466,
> which is to poll the thermal zone periodically until its .get_temp()
> callback starts to return valid temperature values, is at odds with
> the special thermal zone in iwlwifi in which .get_temp() may always
> return an error because its network interface may always be down.  If
> that happens, every attempt to invoke the thermal zone's .get_temp()
> callback resulting in an error causes the thermal core to print a
> dev_warn() message to the kernel log which is super-noisy.
> 
> To address this problem, make the core handle the case in which
> .get_temp() returns 0, but the temperature value returned by it
> is not actually valid, in a special way.  Namely, make the core
> completely ignore the invalid temperature value coming from
> .get_temp() in that case, which requires folding in
> update_temperature() into its caller and a few related changes.
> 
> On the iwlwifi side, modify iwl_mvm_tzone_get_temp() to return 0
> and put THERMAL_TEMP_INVALID into the temperature return memory
> location instead of returning an error when the firmware is not
> running or it is not of the right type.
> 
> Also, to clearly separate the handling of invalid temperature
> values from the thermal zone initialization, introduce a special
> THERMAL_TEMP_INIT value specifically for the latter purpose.
> 
> Fixes: a8a261774466 ("thermal: core: Call monitor_thermal_zone() if zone temperature is invalid")
> Closes: https://lore.kernel.org/linux-pm/20240715044527.GA1544@sol.localdomain/
> Reported-by: Eric Biggers <ebiggers@...nel.org>
> Reported-by: Stefan Lippers-Hollmann <s.l-h@....de>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201761
> Tested-by: Oleksandr Natalenko <oleksandr@...alenko.name>
> Tested-by: Stefan Lippers-Hollmann <s.l-h@....de>
> Cc: 6.10+ <stable@...r.kernel.org> # 6.10+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> 
> v1 -> v2:
>    * It is safer to retain the old behavior in thermal_zone_get_temp(),
>      which is the second place where the .get_temp() zone callback is
>      used, so make it return -ENODATA if the temperature value coming
>      from that callback is invalid.
>    * Add Tested-by: for Stefan.
> 
> I have retained the previous Tested-by because the part of the patch that has
> been tested remains unchanged.
> 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/tt.c |    7 +++
>  drivers/thermal/thermal_core.c              |   51 +++++++++++++---------------
>  drivers/thermal/thermal_core.h              |    3 +
>  drivers/thermal/thermal_helpers.c           |    2 +
>  4 files changed, 35 insertions(+), 28 deletions(-)

This makes the log messages go away for me.  However I had to resolve a conflict
in drivers/net/wireless/intel/iwlwifi/mvm/tt.c to apply this patch to the latest
upstream.

- Eric