public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux ACPI <linux-acpi@vger.kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Michal Wilczynski <michal.wilczynski@intel.com>,
	Zhang Rui <rui.zhang@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Subject: [PATCH v4 06/10] ACPI: thermal: Carry out trip point updates under zone lock
Date: Fri, 04 Aug 2023 23:13:08 +0200	[thread overview]
Message-ID: <3205670.5fSG56mABF@kreacher> (raw)
In-Reply-To: <4878513.31r3eYUQgx@kreacher>

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

There is a race condition between acpi_thermal_trips_update() and
acpi_thermal_check_fn(), because the trip points may get updated while
the latter is running which in theory may lead to inconsistent results.
For example, if two trips are updated together, using the temperature
value of one of them from before the update and the temperature value
of the other one from after the update may not lead to the expected
outcome.

Moreover, if thermal_get_trend() runs when a trip points update is in
progress, it may end up using stale trip point temperatures.

To address this, make acpi_thermal_trips_update() call
thermal_zone_device_adjust() to carry out the trip points update and
provide a new  acpi_thermal_adjust_thermal_zone() wrapper around
__acpi_thermal_trips_update() as the callback function for the latter.

While at it, change the acpi_thermal_trips_update() return data type
to void as that function always returns 0 anyway.

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

v3 -> v4:
   * Rework to use thermal_zone_device_adjust() and the .update() callback
     instead of using the (exported) zone lock directly.
   * Call acpi_queue_thermal_check() from acpi_thermal_trips_update() which
     allows code duplication in acpi_thermal_notify() to be reduced.

v2 -> v3: No changes.

v1 -> v2:
   * Hold the thermal zone lock instead of thermal_check_lock around trip
     point updates (this also helps to protect thermal_get_trend() from using
     stale trip temperatures).
   * Add a comment documenting the purpose of the locking.
   * Make acpi_thermal_trips_update() void.

---
 drivers/acpi/thermal.c |   41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -190,7 +190,7 @@ static int acpi_thermal_get_polling_freq
 	return 0;
 }
 
-static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
+static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
 {
 	acpi_status status;
 	unsigned long long tmp;
@@ -398,17 +398,39 @@ static int acpi_thermal_trips_update(str
 			ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device");
 		}
 	}
+}
 
-	return 0;
+static void acpi_thermal_adjust_thermal_zone(struct thermal_zone_device *thermal,
+					     unsigned long data)
+{
+	__acpi_thermal_trips_update(thermal_zone_device_priv(thermal), data);
+}
+
+static void acpi_queue_thermal_check(struct acpi_thermal *tz)
+{
+	if (!work_pending(&tz->thermal_check_work))
+		queue_work(acpi_thermal_pm_queue, &tz->thermal_check_work);
+}
+
+static void acpi_thermal_trips_update(struct acpi_thermal *tz, int flag)
+{
+	/*
+	 * Use thermal_zone_device_adjust() to carry out the trip points
+	 * update, so as to protect thermal_get_trend() from getting stale
+	 * trip point temperatures and to prevent thermal_zone_device_update()
+	 * invoked from acpi_thermal_check_fn() from producing inconsistent
+	 * results.
+	 */
+	thermal_zone_device_adjust(tz->thermal_zone, flag);
+	acpi_queue_thermal_check(tz);
 }
 
 static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
-	int i, ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 	bool valid;
+	int i;
 
-	if (ret)
-		return ret;
+	__acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
 
 	valid = tz->trips.critical.valid |
 		tz->trips.hot.valid |
@@ -715,6 +737,7 @@ static struct thermal_zone_device_ops ac
 	.get_trend = thermal_get_trend,
 	.hot = acpi_thermal_zone_device_hot,
 	.critical = acpi_thermal_zone_device_critical,
+	.update = acpi_thermal_adjust_thermal_zone,
 };
 
 static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz)
@@ -815,12 +838,6 @@ static void acpi_thermal_unregister_ther
                                  Driver Interface
    -------------------------------------------------------------------------- */
 
-static void acpi_queue_thermal_check(struct acpi_thermal *tz)
-{
-	if (!work_pending(&tz->thermal_check_work))
-		queue_work(acpi_thermal_pm_queue, &tz->thermal_check_work);
-}
-
 static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_device *device = data;
@@ -835,13 +852,11 @@ static void acpi_thermal_notify(acpi_han
 		break;
 	case ACPI_THERMAL_NOTIFY_THRESHOLDS:
 		acpi_thermal_trips_update(tz, ACPI_TRIPS_THRESHOLDS);
-		acpi_queue_thermal_check(tz);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 						dev_name(&device->dev), event, 0);
 		break;
 	case ACPI_THERMAL_NOTIFY_DEVICES:
 		acpi_thermal_trips_update(tz, ACPI_TRIPS_DEVICES);
-		acpi_queue_thermal_check(tz);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 						dev_name(&device->dev), event, 0);
 		break;




  parent reply	other threads:[~2023-08-04 21:26 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-18 18:01 [PATCH v1 0/7] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-07-18 18:02 ` [PATCH v1 1/7] thermal: core: Add mechanism for connecting trips with driver data Rafael J. Wysocki
2023-07-18 18:04 ` [PATCH v1 2/7] thermal: core: Do not handle trip points with invalid temperature Rafael J. Wysocki
2023-07-18 18:05 ` [PATCH v1 3/7] ACPI: thermal: Clean up acpi_thermal_register_thermal_zone() Rafael J. Wysocki
2023-07-18 18:06 ` [PATCH v1 4/7] ACPI: thermal: Hold thermal_check_lock around trip updates Rafael J. Wysocki
2023-07-18 18:06 ` [PATCH v1 5/7] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-07-19 18:46   ` Rafael J. Wysocki
2023-07-18 18:07 ` [PATCH v1 6/7] ACPI: thermal: Rework thermal_get_trend() Rafael J. Wysocki
2023-07-19 18:50   ` Rafael J. Wysocki
2023-07-18 18:19 ` [PATCH v1 7/7] ACPI: thermal: Drop unnecessary thermal zone callbacks Rafael J. Wysocki
2023-07-18 22:45 ` [PATCH v1 0/7] ACPI: thermal: Use trip point table to register thermal zones Daniel Lezcano
2023-07-19 13:33   ` Rafael J. Wysocki
2023-07-21 12:44 ` [PATCH v2 0/8] " Rafael J. Wysocki
2023-07-21 12:46   ` [PATCH v2 1/8] thermal: core: Add mechanism for connecting trips with driver data Rafael J. Wysocki
2023-07-21 12:47   ` [PATCH v2 2/8] thermal: core: Do not handle trip points with invalid temperature Rafael J. Wysocki
2023-07-21 12:50   ` [PATCH v2 3/8] thermal: core: Add routines for locking and unlocking thermal zones Rafael J. Wysocki
2023-07-21 14:12   ` [PATCH v2 4/8] ACPI: thermal: Clean up acpi_thermal_register_thermal_zone() Rafael J. Wysocki
2023-07-21 14:51   ` [PATCH v2 5/8] ACPI: thermal: Hold thermal zone lock around trip updates Rafael J. Wysocki
2023-07-21 15:00   ` [PATCH v2 6/8] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-07-21 15:08   ` [PATCH v2 7/8] ACPI: thermal: Rework thermal_get_trend() Rafael J. Wysocki
2023-07-21 15:10   ` [PATCH v2 8/8] ACPI: thermal: Drop unnecessary thermal zone callbacks Rafael J. Wysocki
2023-07-23 10:19   ` [PATCH v2 0/8] ACPI: thermal: Use trip point table to register thermal zones Daniel Lezcano
2023-07-24  7:59     ` Rafael J. Wysocki
2023-07-25 12:02 ` [PATCH v3 " Rafael J. Wysocki
2023-07-25 12:04   ` [PATCH v3 1/8] thermal: core: Add mechanism for connecting trips with driver data Rafael J. Wysocki
2023-08-01 18:29     ` Daniel Lezcano
2023-08-01 19:02       ` Rafael J. Wysocki
2023-08-02 12:34         ` Daniel Lezcano
2023-08-02 13:03           ` Rafael J. Wysocki
2023-08-02 15:50             ` Daniel Lezcano
2023-08-02 16:48               ` Rafael J. Wysocki
2023-08-03 13:06                 ` Daniel Lezcano
2023-08-03 14:15                   ` Rafael J. Wysocki
2023-08-03 16:20                     ` Daniel Lezcano
2023-08-03 19:58                       ` Rafael J. Wysocki
2023-08-04  8:17                         ` Daniel Lezcano
2023-08-04 11:14                           ` Rafael J. Wysocki
2023-07-25 12:06   ` [PATCH v3 2/8] thermal: core: Do not handle trip points with invalid temperature Rafael J. Wysocki
2023-08-01 18:29     ` Daniel Lezcano
2023-08-01 19:05       ` Rafael J. Wysocki
2023-08-01 19:31         ` Daniel Lezcano
2023-07-25 12:08   ` [PATCH v3 3/8] thermal: core: Add routines for locking and unlocking thermal zones Rafael J. Wysocki
2023-08-01 18:30     ` Daniel Lezcano
2023-08-01 19:11       ` Rafael J. Wysocki
2023-07-25 12:09   ` [PATCH v3 4/8] ACPI: thermal: Clean up acpi_thermal_register_thermal_zone() Rafael J. Wysocki
2023-07-25 12:16   ` [PATCH v3 5/8] ACPI: thermal: Hold thermal zone lock around trip updates Rafael J. Wysocki
2023-08-01 18:39     ` Daniel Lezcano
2023-08-01 18:51       ` Rafael J. Wysocki
2023-07-25 12:20   ` [PATCH v3 6/8] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-07-25 18:08     ` Rafael J. Wysocki
2023-07-25 18:12     ` [PATCH v3.1 " Rafael J. Wysocki
2023-07-25 12:22   ` [PATCH v3 7/8] ACPI: thermal: Rework thermal_get_trend() Rafael J. Wysocki
2023-07-25 12:24   ` [PATCH v3 8/8] ACPI: thermal: Drop unnecessary thermal zone callbacks Rafael J. Wysocki
2023-08-01 18:26   ` [PATCH v3 0/8] ACPI: thermal: Use trip point table to register thermal zones Daniel Lezcano
2023-08-01 18:39     ` Rafael J. Wysocki
2023-08-04 20:58 ` [PATCH v4 00/10] " Rafael J. Wysocki
2023-08-04 21:00   ` [PATCH v4 01/10] thermal: core: Do not handle trip points with invalid temperature Rafael J. Wysocki
2023-08-07 11:25     ` Daniel Lezcano
2023-08-04 21:03   ` [PATCH v4 02/10] thermal: core: Introduce thermal_zone_device_adjust() Rafael J. Wysocki
2023-08-04 21:04   ` [PATCH v4 03/10] thermal: core: Add priv pointer to struct thermal_trip Rafael J. Wysocki
2023-08-07 11:25     ` Daniel Lezcano
2023-08-04 21:05   ` [PATCH v4 04/10] thermal: core: Add thermal_zone_update_trip_temp() helper routine Rafael J. Wysocki
2023-08-07 11:29     ` Daniel Lezcano
2023-08-07 15:40       ` Rafael J. Wysocki
2023-08-07 16:17         ` Daniel Lezcano
2023-08-07 16:23           ` Rafael J. Wysocki
2023-08-04 21:07   ` [PATCH v4 05/10] ACPI: thermal: Clean up acpi_thermal_register_thermal_zone() Rafael J. Wysocki
2023-08-04 21:13   ` Rafael J. Wysocki [this message]
2023-08-04 21:18   ` [PATCH v4 07/10] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-08-04 21:22   ` [PATCH v4 08/10] ACPI: thermal: Rework thermal_get_trend() Rafael J. Wysocki
2023-08-04 21:24   ` [PATCH v4 09/10] ACPI: thermal: Drop unnecessary thermal zone callbacks Rafael J. Wysocki
2023-08-04 21:25   ` [PATCH v4 10/10] thermal: core: Eliminate code duplication from acpi_thermal_notify() Rafael J. Wysocki
2023-08-07 17:59 ` [PATCH v5 00/11] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-08-07 18:01   ` [PATCH v5 01/11] thermal: core: Do not handle trip points with invalid temperature Rafael J. Wysocki
2023-08-07 18:02   ` [PATCH v5 02/11] thermal: core: Introduce thermal_zone_device_adjust() Rafael J. Wysocki
2023-08-07 18:04   ` [PATCH v5 03/11] thermal: core: Add priv pointer to struct thermal_trip Rafael J. Wysocki
2023-08-07 18:06   ` [PATCH v5 04/11] ACPI: thermal: Clean up acpi_thermal_register_thermal_zone() Rafael J. Wysocki
2023-08-07 18:08   ` [PATCH v5 05/11] ACPI: thermal: Carry out trip point updates under zone lock Rafael J. Wysocki
2023-08-16 16:25     ` Daniel Lezcano
2023-08-17 13:09       ` Rafael J. Wysocki
2023-08-07 18:10   ` [PATCH v5 06/11] ACPI: thermal: Introduce struct acpi_thermal_trip Rafael J. Wysocki
2023-08-07 18:11   ` [PATCH v5 07/11] thermal: core: Rework and rename __for_each_thermal_trip() Rafael J. Wysocki
2023-08-07 18:14   ` [PATCH v5 08/11] ACPI: thermal: Use trip point table to register thermal zones Rafael J. Wysocki
2023-08-09 11:07     ` [PATCH v5.1 " Rafael J. Wysocki
2023-08-07 18:17   ` [PATCH v5 09/11] ACPI: thermal: Rework thermal_get_trend() Rafael J. Wysocki
2023-08-07 18:18   ` [PATCH v5 10/11] ACPI: thermal: Drop unnecessary thermal zone callbacks Rafael J. Wysocki
2023-08-07 18:20   ` [PATCH v5 11/11] thermal: core: Eliminate code duplication from acpi_thermal_notify() Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3205670.5fSG56mABF@kreacher \
    --to=rjw@rjwysocki.net \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=michal.wilczynski@intel.com \
    --cc=rui.zhang@intel.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox