[GIT PULL] Thermal control updates for v6.13-rc1

[GIT PULL] Thermal control updates for v6.13-rc1

[Rafael J. Wysocki]

Hi Linus,

Please pull from the tag

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
 thermal-6.13-rc1

with top-most commit 0104dcdaad3a7afd141e79a5fb817a92ada910ac

 thermal: testing: Initialize some variables annoteded with _free()

on top of commit 5469a8deac05391781bcd27e7c40f2c35121ca09

 Merge tag 'thermal-v6.12-rc7' of
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/thermal/linux

to receive thermal control updates for 6.13-rc1.

These are thermal core changes, including the addition of support for
temperature thresholds that can be set from user space, fixes related
to thermal zone initialization, suspend/resume and exit, locking rework
and rearrangement of the code handling thermal zone temperature updates.

Specifics:

 - Add support for thermal thresholds that can be added and removed from
   user space via netlink along with a related library update (Daniel
   Lezcano).

 - Fix thermal zone initialization, suspend/resume and exit
   synchronization issues (Rafael Wysocki).

 - Rearrange locking in the thermal core to use guards (Rafael Wysocki).

 - Make the code handling thermal zone temperature updates use sorted
   lists of trip points to reduce the number of trip points table walks
   in the thermal core (Rafael Wysocki).

 - Fix and clean up the thermal testing facility code (Rafael Wysocki).

 - Fix a Power Allocator thermal governor issue (ZhengShaobo).

Thanks!


---------------

Daniel Lezcano (8):
      thermal: core: Add user thresholds support
      thermal: core: Connect the threshold with the core
      thermal: netlink: Add the commands and the events for the thresholds
      tools/lib/thermal: Make more generic the command encoding function
      tools/lib/thermal: Add the threshold netlink ABI
      tools/thermal/thermal-engine: Take into account the thresholds API
      thermal: thresholds: Fix thermal lock annotation issue
      thermal/lib: Fix memory leak on error in thermal_genl_auto()

Rafael J. Wysocki (36):
      thermal: core: Initialize thermal zones before registering them
      thermal: core: Rearrange PM notification code
      thermal: core: Represent suspend-related thermal zone flags as bits
      thermal: core: Mark thermal zones as initializing to start with
      thermal: core: Fix race between zone registration and system suspend
      thermal: core: Consolidate thermal zone locking during initialization
      thermal: core: Mark thermal zones as exiting before unregistration
      thermal: core: Consolidate thermal zone locking in the exit path
      thermal: core: Update thermal zones after cooling device binding
      thermal: core: Drop need_update field from struct thermal_zone_device
      thermal: core: Move lists of thermal instances to trip descriptors
      thermal: core: Pass trip descriptors to trip bind/unbind functions
      thermal: core: Add and use thermal zone guard
      thermal: core: Add and use a reverse thermal zone guard
      thermal: core: Separate code running under thermal_list_lock
      thermal: core: Manage thermal_list_lock using a mutex guard
      thermal: core: Call thermal_governor_update_tz() outside of cdev lock
      thermal: core: Introduce thermal_instance_add()
      thermal: core: Introduce thermal_instance_delete()
      thermal: core: Introduce thermal_cdev_update_nocheck()
      thermal: core: Add and use cooling device guard
      thermal: core: Separate thermal zone governor initialization
      thermal: core: Manage thermal_governor_lock using a mutex guard
      thermal: core: Build sorted lists instead of sorting them later
      thermal: core: Rename trip list node in struct thermal_trip_desc
      thermal: core: Prepare for moving trips between sorted lists
      thermal: core: Rearrange __thermal_zone_device_update()
      thermal: core: Pass trip descriptor to thermal_trip_crossed()
      thermal: core: Move some trip processing to thermal_trip_crossed()
      thermal: core: Relocate functions that update trip points
      thermal: core: Eliminate thermal_zone_trip_down()
      thermal: core: Use trip lists for trip crossing detection
      thermal: core: Relocate thermal zone initialization routine
      thermal: testing: Simplify tt_get_tt_zone()
      thermal: testing: Use DEFINE_FREE() and __free() to simplify code
      thermal: testing: Initialize some variables annoteded with _free()

ZhengShaobo (1):
      thermal: gov_power_allocator: Granted power set to max when
nobody request power

---------------

 drivers/thermal/Makefile                      |   1 +
 drivers/thermal/gov_bang_bang.c               |  15 +-
 drivers/thermal/gov_fair_share.c              |  20 +-
 drivers/thermal/gov_power_allocator.c         |  86 +--
 drivers/thermal/gov_step_wise.c               |  22 +-
 drivers/thermal/testing/zone.c                |  41 +-
 drivers/thermal/thermal_core.c                | 883 +++++++++++++++-----------
 drivers/thermal/thermal_core.h                |  41 +-
 drivers/thermal/thermal_debugfs.c             |  50 +-
 drivers/thermal/thermal_helpers.c             |  46 +-
 drivers/thermal/thermal_hwmon.c               |   5 +-
 drivers/thermal/thermal_netlink.c             | 253 +++++++-
 drivers/thermal/thermal_netlink.h             |  34 +
 drivers/thermal/thermal_sysfs.c               | 132 ++--
 drivers/thermal/thermal_thresholds.c          | 240 +++++++
 drivers/thermal/thermal_thresholds.h          |  19 +
 drivers/thermal/thermal_trip.c                |  48 +-
 include/linux/thermal.h                       |   6 +
 include/uapi/linux/thermal.h                  |  29 +-
 tools/lib/thermal/commands.c                  | 188 +++++-
 tools/lib/thermal/events.c                    |  55 +-
 tools/lib/thermal/include/thermal.h           |  40 ++
 tools/lib/thermal/libthermal.map              |   5 +
 tools/lib/thermal/thermal.c                   |  17 +
 tools/thermal/lib/Makefile                    |   2 +-
 tools/thermal/thermal-engine/thermal-engine.c | 105 ++-
 26 files changed, 1650 insertions(+), 733 deletions(-)

Re: [GIT PULL] Thermal control updates for v6.13-rc1

[pr-tracker-bot]

The pull request you sent on Mon, 18 Nov 2024 11:23:46 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal-6.13-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/cd7fa3e1b0bc9c210eba23edbe8d6884f0368281

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] Thermal control updates for v6.13-rc1

[Sasha Levin]

On Mon, Nov 18, 2024 at 11:23:46AM +0100, Rafael J. Wysocki wrote:
>Hi Linus,
>
>Please pull from the tag
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> thermal-6.13-rc1

Hi Rafael,

After merging this PR into linus-next, KernelCI started reporting boot
failures on a few platforms:

[    6.921489] [00000000000000d0] user address but active_mm is swapper
[    6.927831] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[    6.934086] Modules linked in:
[    6.937131] CPU: 7 UID: 0 PID: 12 Comm: kworker/u32:1 Tainted: G        W          6.12.0 #1 cab58e2e59020ebd4be8ada89a65f465a316c742
[    6.949114] Tainted: [W]=WARN
[    6.952070] Hardware name: Google Spherion (rev0 - 3) (DT)
[    6.957543] Workqueue: async async_run_entry_fn
[    6.962066] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    6.969013] pc : power_allocator_bind+0xd4/0x2e8
[    6.973622] lr : power_allocator_bind+0x38/0x2e8
[    6.978226] sp : ffff8000800e3010 
[    6.981528] x29: ffff8000800e3010 x28: ffff71f205b31800 x27: ffff71f205b31c08
[    6.988650] x26: ffffd833bdc11a78 x25: 0000000000000000 x24: ffff71f205b31da8
[    6.995773] x23: 0000000000000000 x22: ffff71f205b31800 x21: ffffd833bcd5f438
[    7.002896] x20: ffff71f200af61c0 x19: 0000000000000000 x18: ffffffffffffffff
[    7.010018] x17: 000000040044ffff x16: 0000000000000100 x15: ffff8000800e2e30
[    7.017141] x14: ffff71f205b40a1c x13: ffff71f205b4026f x12: 0101010101010101
[    7.024264] x11: 7f7f7f7f7f7f7f7f x10: 0000000000000000 x9 : ffffd833baefbd2c
[    7.031387] x8 : ffff71f200af6200 x7 : 0000000000000000 x6 : 0000000000000000
[    7.038509] x5 : 0000000000000000 x4 : 0000000000000040 x3 : 0000000000000000
[    7.045632] x2 : ffffffffffffffc0 x1 : 0000000000000000 x0 : ffff71f205b31da8
[    7.052755] Call trace:
[    7.055190]  power_allocator_bind+0xd4/0x2e8 (P)
[    7.059795]  power_allocator_bind+0x38/0x2e8 (L)
[    7.064399]  thermal_set_governor+0x4c/0xc8
[    7.068571]  thermal_zone_device_register_with_trips+0x480/0x630
[    7.074566]  thermal_tripless_zone_device_register+0x34/0x48
[    7.080214]  __power_supply_register.part.0+0x364/0x4b8
[    7.085428]  __power_supply_register+0x64/0xc0
[    7.089860]  devm_power_supply_register+0x64/0xb8
[    7.094552]  sbs_probe+0x290/0x3a0 
[    7.097941]  i2c_device_probe+0x150/0x2a0
[    7.101941]  really_probe+0xc4/0x2a8
[    7.105506]  __driver_probe_device+0x80/0x140
[    7.109850]  driver_probe_device+0x44/0x120
[    7.114021]  __device_attach_driver+0xc0/0x108
[    7.118451]  bus_for_each_drv+0x8c/0xf0
[    7.122276]  __device_attach+0xa4/0x198 
[    7.126099]  device_initial_probe+0x1c/0x30
[    7.130269]  bus_probe_device+0xb4/0xc0
[    7.134093]  device_add+0x570/0x740
[    7.137570]  device_register+0x28/0x40 
[    7.141307]  i2c_new_client_device+0x19c/0x380
[    7.145740]  of_i2c_register_devices+0x120/0x1a0
[    7.150345]  i2c_register_adapter+0x204/0x670
[    7.154690]  i2c_add_adapter+0x80/0xe0 
[    7.158428]  ec_i2c_probe+0xe0/0x160
[    7.161991]  platform_probe+0x70/0xd0
[    7.165642]  really_probe+0xc4/0x2a8 
[    7.169207]  __driver_probe_device+0x80/0x140
[    7.173550]  driver_probe_device+0x44/0x120
[    7.177720]  __device_attach_driver+0xc0/0x108
[    7.182151]  bus_for_each_drv+0x8c/0xf0
[    7.185976]  __device_attach+0xa4/0x198
[    7.189799]  device_initial_probe+0x1c/0x30
[    7.193970]  bus_probe_device+0xb4/0xc0
[    7.197794]  device_add+0x570/0x740
[    7.201271]  of_device_add+0x5c/0x78
[    7.204836]  of_platform_device_create_pdata+0x98/0x138
[    7.210050]  of_platform_bus_create+0x15c/0x398
[    7.214569]  of_platform_populate+0x58/0x110
[    7.218827]  devm_of_platform_populate+0x60/0xd0
[    7.223432]  cros_ec_register+0x18c/0x370
[    7.227430]  cros_ec_spi_probe+0x17c/0x250
[    7.231514]  spi_probe+0x88/0xf8
[    7.234732]  really_probe+0xc4/0x2a8
[    7.238297]  __driver_probe_device+0x80/0x140
[    7.242641]  driver_probe_device+0x44/0x120
[    7.246812]  __driver_attach_async_helper+0x54/0xc8
[    7.251678]  async_run_entry_fn+0x40/0x160
[    7.255762]  process_one_work+0x18c/0x420
[    7.259762]  worker_thread+0x30c/0x418
[    7.263500]  kthread+0x128/0x138
[    7.266716]  ret_from_fork+0x10/0x20
[    7.270282] Code: b4000b47 aa0703e5 a9019e86 52800013 (f84d0ca0)
[    7.276362] ---[ end trace 0000000000000000 ]---
[    7.282971] Kernel panic - not syncing: Oops: Fatal exception

-- 
Thanks,
Sasha

Re: [GIT PULL] Thermal control updates for v6.13-rc1

[Linus Torvalds]

On Sun, 24 Nov 2024 at 09:10, Sasha Levin <sashal@kernel.org> wrote:
>
> On Mon, Nov 18, 2024 at 11:23:46AM +0100, Rafael J. Wysocki wrote:
> >Hi Linus,
> >
> >Please pull from the tag
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> > thermal-6.13-rc1
>
> Hi Rafael,
>
> After merging this PR into linus-next, KernelCI started reporting boot
> failures on a few platforms:
>
> [    6.921489] [00000000000000d0] user address but active_mm is swapper

This is an odd arm64 way of saying "NULL pointer dereference in kernel".

The NULL pointer is in the user space address range, so it
superficially looks like it's trying to do a user access.

People are more used to the x86 page fault error reporting (and
honestly - they are a bit better worded in this case too).

If I did the disassembly correctly, the code disassembles to

        cbz     x7, 0x168
        mov     x5, x7
        stp     x6, x7, [x20, #24]
        mov     w19, #0x0
        ldr     x0, [x5, #208]!      <--- faulting instruction

which does match that address 00000000000000d0 does match "x5+208",
since x5 is NULL.

Matching it up manually with my build (different config, different
compiler, so different register allocation), it's this:

// drivers/thermal/gov_power_allocator.c:527:   if (last_passive) {
        cbz     x6, .L177       // last_passive,
.L134:
// drivers/thermal/gov_power_allocator.c:595:
list_for_each_entry(instance, &td->thermal_instances, trip_node) {
        mov     x5, x6  // _21, last_passive
// drivers/thermal/gov_power_allocator.c:529:
params->trip_max = last_passive;
        stp     x0, x6, [x21, 24]       // first_passive, last_passive,
// drivers/thermal/gov_power_allocator.c:593:   int ret = 0;
        mov     w19, 0  // <retval>,
// drivers/thermal/gov_power_allocator.c:595:
list_for_each_entry(instance, &td->thermal_instances, trip_node) {
        ldr     x0, [x5, 208]!  // __mptr, MEM[(const struct
thermal_trip_desc *)prephitmp_29].thermal_instances.next

so it looks like it is that

        list_for_each_entry(instance, &td->thermal_instances, trip_node) {

with the 'td' being NULL.

The code seems to do that

        const struct thermal_trip_desc *td =
trip_to_trip_desc(params->trip_max);

So apparently params->trip_max is NULL.

That's where I stopped looking. It's almost certainly due to  commit
0dc23567c206 ("thermal: core: Move lists of thermal instances to trip
descriptors") but I don't know the code.

Over to Rafael and Lukasz,

                Linus

Re: [GIT PULL] Thermal control updates for v6.13-rc1

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]

Sasha, thanks for the report!

Linus, thanks for the analysis!

On Sun, Nov 24, 2024 at 7:40 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, 24 Nov 2024 at 09:10, Sasha Levin <sashal@kernel.org> wrote:
> >
> > On Mon, Nov 18, 2024 at 11:23:46AM +0100, Rafael J. Wysocki wrote:
> > >Hi Linus,
> > >
> > >Please pull from the tag
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
> > > thermal-6.13-rc1
> >
> > Hi Rafael,
> >
> > After merging this PR into linus-next, KernelCI started reporting boot
> > failures on a few platforms:
> >
> > [    6.921489] [00000000000000d0] user address but active_mm is swapper
>
> This is an odd arm64 way of saying "NULL pointer dereference in kernel".
>
> The NULL pointer is in the user space address range, so it
> superficially looks like it's trying to do a user access.
>
> People are more used to the x86 page fault error reporting (and
> honestly - they are a bit better worded in this case too).
>
> If I did the disassembly correctly, the code disassembles to
>
>         cbz     x7, 0x168
>         mov     x5, x7
>         stp     x6, x7, [x20, #24]
>         mov     w19, #0x0
>         ldr     x0, [x5, #208]!      <--- faulting instruction
>
> which does match that address 00000000000000d0 does match "x5+208",
> since x5 is NULL.
>
> Matching it up manually with my build (different config, different
> compiler, so different register allocation), it's this:
>
> // drivers/thermal/gov_power_allocator.c:527:   if (last_passive) {
>         cbz     x6, .L177       // last_passive,
> .L134:
> // drivers/thermal/gov_power_allocator.c:595:
> list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>         mov     x5, x6  // _21, last_passive
> // drivers/thermal/gov_power_allocator.c:529:
> params->trip_max = last_passive;
>         stp     x0, x6, [x21, 24]       // first_passive, last_passive,
> // drivers/thermal/gov_power_allocator.c:593:   int ret = 0;
>         mov     w19, 0  // <retval>,
> // drivers/thermal/gov_power_allocator.c:595:
> list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>         ldr     x0, [x5, 208]!  // __mptr, MEM[(const struct
> thermal_trip_desc *)prephitmp_29].thermal_instances.next
>
> so it looks like it is that
>
>         list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>
> with the 'td' being NULL.
>
> The code seems to do that
>
>         const struct thermal_trip_desc *td =
> trip_to_trip_desc(params->trip_max);
>
> So apparently params->trip_max is NULL.
>
> That's where I stopped looking. It's almost certainly due to  commit
> 0dc23567c206 ("thermal: core: Move lists of thermal instances to trip
> descriptors") but I don't know the code.
>
> Over to Rafael and Lukasz,

You are right, the above commit introduced this issue and there is one
more hint on it in Sasha's call trace, which is
thermal_tripless_zone_device_register() used for registering the
thermal zone.  Of course, this means that there are no trip points in
that zone, so params->trip_max is NULL in check_power_actors() and it
needs to be explicitly checked against NULL.

Previously, the check was not needed because the list of thermal
instances was located in the thermal zone object, so walking it would
just produce no results.

IOW, something like the attached patch is needed.

Thanks, Rafael

[-- Attachment #2: thermal-gov_power_allocator-fix-null-deref.patch --]
[-- Type: text/x-patch, Size: 1738 bytes --]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH v1] thermal: gov_power_allocator: Add missing NULL pointer check

Commit 0dc23567c206 ("thermal: core: Move lists of thermal instances
to trip descriptors") overlooked the case in which the Power Allocator
governor attempts to bind to a tripless thermal zone and params->trip_max
is NULL in check_power_actors().

No power actors can be found in that case, so check_power_actors() needs
to be made return 0 then to restore its previous behavior.

Fixes: 0dc23567c206 ("thermal: core: Move lists of thermal instances to trip descriptors")
Closes: https://lore.kernel.org/linux-pm/Z0NeGF4ryCe_b5rr@sashalap/
Reported-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/gov_power_allocator.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/thermal/gov_power_allocator.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_power_allocator.c
+++ linux-pm/drivers/thermal/gov_power_allocator.c
@@ -588,10 +588,15 @@ static void allow_maximum_power(struct t
 static int check_power_actors(struct thermal_zone_device *tz,
 			      struct power_allocator_params *params)
 {
-	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
+	const struct thermal_trip_desc *td;
 	struct thermal_instance *instance;
 	int ret = 0;
 
+	if (!params->trip_max)
+		return 0;
+
+	td = trip_to_trip_desc(params->trip_max);
+
 	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		if (!cdev_is_power_actor(instance->cdev)) {
 			dev_warn(&tz->device, "power_allocator: %s is not a power actor\n",