public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Lukasz Luba <lukasz.luba@arm.com>
Cc: Linux PM <linux-pm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Zhang Rui <rui.zhang@intel.com>
Subject: [PATCH v1 2/2] thermal: sysfs: Eliminate unnecessary zone locking
Date: Thu, 30 Nov 2023 19:37:45 +0100	[thread overview]
Message-ID: <2933888.e9J7NaK4W3@kreacher> (raw)
In-Reply-To: <5754079.DvuYhMxLoT@kreacher>

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

The _show() callback functions of the trip point sysfs attributes,
temperature, hysteresis and type, need not use thermal zone locking,
because the layout of the data structures they access does not change
after the thermal zone registration.

Namely, they all need to access a specific entry in the thermal
zone's trips[] table that is always present for non-tripless thermal
zones and its size cannot change after the thermal zone has been
registered.  Thus it is always safe to access the trips[] table of a
registered thermal zone from each of the sysfs attributes in question.

Moreover, the type of a trip point does not change after registering its
thermal zone, and while its temperature and hysteresis can change, for
example due to a firmware-induced thermal zone update, holding the zone
lock around reading them is pointless, because it does not prevent stale
values from being returned to user space.  For example, a trip point
temperature can always change ater trip_point_temp_show() has read it
and before the function's return statement is executed, regardless of
whether or not zone locking is used.

For this reason, drop the zone locking from trip_point_type_show(),
trip_point_temp_show(), and trip_point_hyst_show().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_sysfs.c |   60 ++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 39 deletions(-)

Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -83,25 +83,18 @@ trip_point_type_show(struct device *dev,
 		     char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	struct thermal_trip trip;
-	int trip_id, result;
+	int trip_id;
+
+	if (!device_is_registered(dev))
+		return -ENODEV;
 
 	if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1)
 		return -EINVAL;
 
-	mutex_lock(&tz->lock);
-
-	if (device_is_registered(dev))
-		result = __thermal_zone_get_trip(tz, trip_id, &trip);
-	else
-		result = -ENODEV;
-
-	mutex_unlock(&tz->lock);
-
-	if (result)
-		return result;
+	if (trip_id < 0 || trip_id > tz->num_trips)
+		return -EINVAL;
 
-	switch (trip.type) {
+	switch (tz->trips[trip_id].type) {
 	case THERMAL_TRIP_CRITICAL:
 		return sprintf(buf, "critical\n");
 	case THERMAL_TRIP_HOT:
@@ -164,25 +157,18 @@ trip_point_temp_show(struct device *dev,
 		     char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	struct thermal_trip trip;
-	int trip_id, ret;
+	int trip_id;
+
+	if (!device_is_registered(dev))
+		return -ENODEV;
 
 	if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)
 		return -EINVAL;
 
-	mutex_lock(&tz->lock);
-
-	if (device_is_registered(dev))
-		ret = __thermal_zone_get_trip(tz, trip_id, &trip);
-	else
-		ret = -ENODEV;
-
-	mutex_unlock(&tz->lock);
-
-	if (ret)
-		return ret;
+	if (trip_id < 0 || trip_id > tz->num_trips)
+		return -EINVAL;
 
-	return sprintf(buf, "%d\n", trip.temperature);
+	return sprintf(buf, "%d\n", tz->trips[trip_id].temperature);
 }
 
 static ssize_t
@@ -234,22 +220,18 @@ trip_point_hyst_show(struct device *dev,
 		     char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	struct thermal_trip trip;
-	int trip_id, ret;
+	int trip_id;
+
+	if (!device_is_registered(dev))
+		return -ENODEV;
 
 	if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)
 		return -EINVAL;
 
-	mutex_lock(&tz->lock);
-
-	if (device_is_registered(dev))
-		ret = __thermal_zone_get_trip(tz, trip_id, &trip);
-	else
-		ret = -ENODEV;
-
-	mutex_unlock(&tz->lock);
+	if (trip_id < 0 || trip_id > tz->num_trips)
+		return -EINVAL;
 
-	return ret ? ret : sprintf(buf, "%d\n", trip.hysteresis);
+	return sprintf(buf, "%d\n", tz->trips[trip_id].hysteresis);
 }
 
 static ssize_t




  parent reply	other threads:[~2023-11-30 18:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30 18:27 [PATCH v1 0/2] thermal: sysfs: Simplifications of trip point attribute callbacks Rafael J. Wysocki
2023-11-30 18:35 ` [PATCH v1 1/2] thermal: sysfs: Rework the handling of trip point updates Rafael J. Wysocki
2023-11-30 18:37 ` Rafael J. Wysocki [this message]
2023-11-30 19:33   ` [PATCH v1 2/2] thermal: sysfs: Eliminate unnecessary zone locking Rafael J. Wysocki
2023-12-01  9:16   ` Daniel Lezcano
2023-12-01 11:05     ` Rafael J. Wysocki
2023-11-30 19:42 ` [PATCH v1.1 2/2] thermal: sysfs: Simplifications of trip point attribute callbacks Rafael J. Wysocki
2023-11-30 19:51   ` Rafael J. Wysocki
2023-11-30 19:53 ` [PATCH v1.1 2/2] thermal: sysfs: Eliminate unnecessary zone locking 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=2933888.e9J7NaK4W3@kreacher \
    --to=rjw@rjwysocki.net \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukasz.luba@arm.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