linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking
@ 2024-09-14 10:24 Rafael J. Wysocki
  2024-09-14 10:25 ` [RFC PATCH for 6.13 v1 01/20] thermal: core: Use the thermal zone guard in more cases Rafael J. Wysocki
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:24 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

Hi Everyone,

This is a continuation of

https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/

that eliminates a couple of races related to thermal zone initialization
and system suspend, cleans up thermal zone updates handling during thermal
zone and cooling device initialization and generally switches over the
thermal core to using guards for locking.

Please see the individual patch changelogs for details.

The patches in this series only received minimum testing (hence RFC), but
all of these changes make sense IMV.

Thanks!




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 01/20] thermal: core: Use the thermal zone guard in more cases
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
@ 2024-09-14 10:25 ` Rafael J. Wysocki
  2024-09-14 10:27 ` [RFC PATCH for 6.13 v1 02/20] thermal: core: Rearrange PM notification code Rafael J. Wysocki
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:25 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

There are a few more cases in which the thermal zone guard introduced
previously can be used to help clarify the code, so do that.

No intentional functional impact.

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

To be merged with https://lore.kernel.org/linux-pm/3241904.5fSG56mABF@rjwysocki.net/

---
 drivers/thermal/thermal_debugfs.c |   25 +++++++++++++++----------
 drivers/thermal/thermal_hwmon.c   |    6 +-----
 drivers/thermal/thermal_netlink.c |   21 ++++++---------------
 3 files changed, 22 insertions(+), 30 deletions(-)

Index: linux-pm/drivers/thermal/thermal_netlink.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_netlink.c
+++ linux-pm/drivers/thermal/thermal_netlink.c
@@ -460,7 +460,7 @@ static int thermal_genl_cmd_tz_get_trip(
 	if (!start_trip)
 		return -EMSGSIZE;
 
-	mutex_lock(&tz->lock);
+	guard(thermal_zone)(tz);
 
 	for_each_trip_desc(tz, td) {
 		const struct thermal_trip *trip = &td->trip;
@@ -470,19 +470,12 @@ static int thermal_genl_cmd_tz_get_trip(
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip->type) ||
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, trip->temperature) ||
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, trip->hysteresis))
-			goto out_cancel_nest;
+			return -EMSGSIZE;
 	}
 
-	mutex_unlock(&tz->lock);
-
 	nla_nest_end(msg, start_trip);
 
 	return 0;
-
-out_cancel_nest:
-	mutex_unlock(&tz->lock);
-
-	return -EMSGSIZE;
 }
 
 static int thermal_genl_cmd_tz_get_temp(struct param *p)
@@ -515,7 +508,7 @@ static int thermal_genl_cmd_tz_get_gov(s
 {
 	struct sk_buff *msg = p->msg;
 	struct thermal_zone_device *tz;
-	int id, ret = 0;
+	int id;
 
 	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
 		return -EINVAL;
@@ -526,16 +519,14 @@ static int thermal_genl_cmd_tz_get_gov(s
 	if (!tz)
 		return -EINVAL;
 
-	mutex_lock(&tz->lock);
+	guard(thermal_zone)(tz);
 
 	if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id) ||
 	    nla_put_string(msg, THERMAL_GENL_ATTR_TZ_GOV_NAME,
 			   tz->governor->name))
-		ret = -EMSGSIZE;
-
-	mutex_unlock(&tz->lock);
+		return -EMSGSIZE;
 
-	return ret;
+	return 0;
 }
 
 static int __thermal_genl_cmd_cdev_get(struct thermal_cooling_device *cdev,
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -885,6 +885,19 @@ void thermal_debug_tz_add(struct thermal
 	tz->debugfs = thermal_dbg;
 }
 
+static struct thermal_debugfs *thermal_debug_tz_clear(struct thermal_zone_device *tz)
+{
+	struct thermal_debugfs *thermal_dbg;
+
+	guard(thermal_zone)(tz);
+
+	thermal_dbg = tz->debugfs;
+	if (thermal_dbg)
+		tz->debugfs = NULL;
+
+	return thermal_dbg;
+}
+
 void thermal_debug_tz_remove(struct thermal_zone_device *tz)
 {
 	struct thermal_debugfs *thermal_dbg;
@@ -892,17 +905,9 @@ void thermal_debug_tz_remove(struct ther
 	struct tz_debugfs *tz_dbg;
 	int *trips_crossed;
 
-	mutex_lock(&tz->lock);
-
-	thermal_dbg = tz->debugfs;
-	if (!thermal_dbg) {
-		mutex_unlock(&tz->lock);
+	thermal_dbg = thermal_debug_tz_clear(tz);
+	if (!thermal_dbg)
 		return;
-	}
-
-	tz->debugfs = NULL;
-
-	mutex_unlock(&tz->lock);
 
 	tz_dbg = &thermal_dbg->tz_dbg;
 
Index: linux-pm/drivers/thermal/thermal_hwmon.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_hwmon.c
+++ linux-pm/drivers/thermal/thermal_hwmon.c
@@ -78,19 +78,15 @@ temp_crit_show(struct device *dev, struc
 	int temperature;
 	int ret;
 
-	mutex_lock(&tz->lock);
+	guard(thermal_zone)(tz);
 
 	ret = tz->ops.get_crit_temp(tz, &temperature);
-
-	mutex_unlock(&tz->lock);
-
 	if (ret)
 		return ret;
 
 	return sprintf(buf, "%d\n", temperature);
 }
 
-
 static struct thermal_hwmon_device *
 thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
 {




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 02/20] thermal: core: Rearrange PM notification code
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
  2024-09-14 10:25 ` [RFC PATCH for 6.13 v1 01/20] thermal: core: Use the thermal zone guard in more cases Rafael J. Wysocki
@ 2024-09-14 10:27 ` Rafael J. Wysocki
  2024-09-14 10:28 ` [RFC PATCH for 6.13 v1 03/20] thermal: core: Represent suspend-related thermal zone flags as bits Rafael J. Wysocki
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:27 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Move the code run for each thermal zone by the thermal PM notify
handler to separate functions and use the thermal zone guard to
implement locking in them.

This will help to make some subsequent changes look somewhat more
straightforward, among other things.

No intentional functional impact.

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

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1662,6 +1662,44 @@ static void thermal_zone_device_resume(s
 	tz->resuming = false;
 }
 
+static void thermal_zone_pm_prepare(struct thermal_zone_device *tz)
+{
+	guard(thermal_zone)(tz);
+
+	if (tz->resuming) {
+		/*
+		 * thermal_zone_device_resume() queued up for this zone has not
+		 * acquired the lock yet, so release it to let the function run
+		 * and wait util it has done the work.
+		 */
+		mutex_unlock(&tz->lock);
+
+		wait_for_completion(&tz->resume);
+
+		mutex_lock(&tz->lock);
+	}
+
+	tz->suspended = true;
+}
+
+static void thermal_zone_pm_complete(struct thermal_zone_device *tz)
+{
+	guard(thermal_zone)(tz);
+
+	cancel_delayed_work(&tz->poll_queue);
+
+	reinit_completion(&tz->resume);
+	tz->resuming = true;
+
+	/*
+	 * Replace the work function with the resume one, which will restore the
+	 * original work function and schedule the polling work if needed.
+	 */
+	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_resume);
+	/* Queue up the work without a delay. */
+	mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, 0);
+}
+
 static int thermal_pm_notify(struct notifier_block *nb,
 			     unsigned long mode, void *_unused)
 {
@@ -1673,27 +1711,8 @@ static int thermal_pm_notify(struct noti
 	case PM_SUSPEND_PREPARE:
 		mutex_lock(&thermal_list_lock);
 
-		list_for_each_entry(tz, &thermal_tz_list, node) {
-			mutex_lock(&tz->lock);
-
-			if (tz->resuming) {
-				/*
-				 * thermal_zone_device_resume() queued up for
-				 * this zone has not acquired the lock yet, so
-				 * release it to let the function run and wait
-				 * util it has done the work.
-				 */
-				mutex_unlock(&tz->lock);
-
-				wait_for_completion(&tz->resume);
-
-				mutex_lock(&tz->lock);
-			}
-
-			tz->suspended = true;
-
-			mutex_unlock(&tz->lock);
-		}
+		list_for_each_entry(tz, &thermal_tz_list, node)
+			thermal_zone_pm_prepare(tz);
 
 		mutex_unlock(&thermal_list_lock);
 		break;
@@ -1702,27 +1721,8 @@ static int thermal_pm_notify(struct noti
 	case PM_POST_SUSPEND:
 		mutex_lock(&thermal_list_lock);
 
-		list_for_each_entry(tz, &thermal_tz_list, node) {
-			mutex_lock(&tz->lock);
-
-			cancel_delayed_work(&tz->poll_queue);
-
-			reinit_completion(&tz->resume);
-			tz->resuming = true;
-
-			/*
-			 * Replace the work function with the resume one, which
-			 * will restore the original work function and schedule
-			 * the polling work if needed.
-			 */
-			INIT_DELAYED_WORK(&tz->poll_queue,
-					  thermal_zone_device_resume);
-			/* Queue up the work without a delay. */
-			mod_delayed_work(system_freezable_power_efficient_wq,
-					 &tz->poll_queue, 0);
-
-			mutex_unlock(&tz->lock);
-		}
+		list_for_each_entry(tz, &thermal_tz_list, node)
+			thermal_zone_pm_complete(tz);
 
 		mutex_unlock(&thermal_list_lock);
 		break;




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 03/20] thermal: core: Represent suspend-related thermal zone flags as bits
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
  2024-09-14 10:25 ` [RFC PATCH for 6.13 v1 01/20] thermal: core: Use the thermal zone guard in more cases Rafael J. Wysocki
  2024-09-14 10:27 ` [RFC PATCH for 6.13 v1 02/20] thermal: core: Rearrange PM notification code Rafael J. Wysocki
@ 2024-09-14 10:28 ` Rafael J. Wysocki
  2024-09-14 10:30 ` [RFC PATCH for 6.13 v1 04/20] thermal: core: Mark thermal zones as initializing to start with Rafael J. Wysocki
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:28 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Instead of using two separate fields in struct thermal_zone_device for
representing flags related to thermal zone suspend, represent them
explicitly as bits in one u8 "state" field.

Subsequently, that field will be used for addressing race conditions
related to thermal zone initialization and exit.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   11 +++++------
 drivers/thermal/thermal_core.h |   11 +++++++----
 2 files changed, 12 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -497,7 +497,7 @@ void __thermal_zone_device_update(struct
 	int low = -INT_MAX, high = INT_MAX;
 	int temp, ret;
 
-	if (tz->suspended || tz->mode != THERMAL_DEVICE_ENABLED)
+	if (tz->state != TZ_STATE_READY || tz->mode != THERMAL_DEVICE_ENABLED)
 		return;
 
 	ret = __thermal_zone_get_temp(tz, &temp);
@@ -1651,7 +1651,7 @@ static void thermal_zone_device_resume(s
 
 	guard(thermal_zone)(tz);
 
-	tz->suspended = false;
+	tz->state &= ~(TZ_STATE_FLAG_SUSPENDED | TZ_STATE_FLAG_RESUMING);
 
 	thermal_debug_tz_resume(tz);
 	thermal_zone_device_init(tz);
@@ -1659,14 +1659,13 @@ static void thermal_zone_device_resume(s
 	__thermal_zone_device_update(tz, THERMAL_TZ_RESUME);
 
 	complete(&tz->resume);
-	tz->resuming = false;
 }
 
 static void thermal_zone_pm_prepare(struct thermal_zone_device *tz)
 {
 	guard(thermal_zone)(tz);
 
-	if (tz->resuming) {
+	if (tz->state & TZ_STATE_FLAG_RESUMING) {
 		/*
 		 * thermal_zone_device_resume() queued up for this zone has not
 		 * acquired the lock yet, so release it to let the function run
@@ -1679,7 +1678,7 @@ static void thermal_zone_pm_prepare(stru
 		mutex_lock(&tz->lock);
 	}
 
-	tz->suspended = true;
+	tz->state |= TZ_STATE_FLAG_SUSPENDED;
 }
 
 static void thermal_zone_pm_complete(struct thermal_zone_device *tz)
@@ -1689,7 +1688,7 @@ static void thermal_zone_pm_complete(str
 	cancel_delayed_work(&tz->poll_queue);
 
 	reinit_completion(&tz->resume);
-	tz->resuming = true;
+	tz->state |= TZ_STATE_FLAG_RESUMING;
 
 	/*
 	 * Replace the work function with the resume one, which will restore the
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -62,6 +62,11 @@ struct thermal_governor {
 	struct list_head	governor_list;
 };
 
+#define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
+#define	TZ_STATE_FLAG_RESUMING	BIT(1)
+
+#define TZ_STATE_READY		0
+
 /**
  * struct thermal_zone_device - structure for a thermal zone
  * @id:		unique id number for each thermal zone
@@ -103,8 +108,7 @@ struct thermal_governor {
  * @node:	node in thermal_tz_list (in thermal_core.c)
  * @poll_queue:	delayed work for polling
  * @notify_event: Last notification event
- * @suspended: thermal zone suspend indicator
- * @resuming:	indicates whether or not thermal zone resume is in progress
+ * @state: 	current state of the thermal zone
  * @trips:	array of struct thermal_trip objects
  */
 struct thermal_zone_device {
@@ -139,8 +143,7 @@ struct thermal_zone_device {
 	struct list_head node;
 	struct delayed_work poll_queue;
 	enum thermal_notify_event notify_event;
-	bool suspended;
-	bool resuming;
+	u8 state;
 #ifdef CONFIG_THERMAL_DEBUGFS
 	struct thermal_debugfs *debugfs;
 #endif




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 04/20] thermal: core: Mark thermal zones as initializing to start with
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2024-09-14 10:28 ` [RFC PATCH for 6.13 v1 03/20] thermal: core: Represent suspend-related thermal zone flags as bits Rafael J. Wysocki
@ 2024-09-14 10:30 ` Rafael J. Wysocki
  2024-09-14 10:31 ` [RFC PATCH for 6.13 v1 05/20] thermal: core: Fix race between zone registration and system suspend Rafael J. Wysocki
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:30 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

After thermal_zone_device_register_with_trips() has called
device_register() and it has registered the new thermal zone device
with the driver core, user space may access its sysfs attributes and,
among other things, it may enable the thermal zone before it is ready.

To address this, introduce a new thermal zone state flag for
initialization and set it before calling device_register() in
thermal_zone_device_register_with_trips().  This causes
__thermal_zone_device_update() to return early until the new flag
is cleared.

To clear it when the thermal zone is ready, introduce a new
function called thermal_zone_init_complete() that will also invoke
__thermal_zone_device_update() after clearing that flag (both under the
thernal zone lock) and make thermal_zone_device_register_with_trips()
call the new function instead of checking need_update and calling
thermal_zone_device_update() when it is set.

After this change, if user space enables the thermal zone prematurely,
__thermal_zone_device_update() will return early for it until
thermal_zone_init_complete() is called.  In turn, if the thermal zone
is not enabled by user space before thermal_zone_init_complete() is
called, the __thermal_zone_device_update() call in it will return early
because the thermal zone has not been enabled yet, but that function
will be invoked again by thermal_zone_device_set_mode() when the thermal
zone is enabled and it will not return early this time.

The checking of need_update is not necessary any more because the
__thermal_zone_device_update() calls potentially triggered by cooling
device binding take place before calling thermal_zone_init_complete(),
so they all will return early, which means that
thermal_zone_init_complete() must call __thermal_zone_device_update()
in case the thermal zone is enabled prematurely by user space.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   14 +++++++++++---
 drivers/thermal/thermal_core.h |    1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1318,6 +1318,14 @@ static void thermal_zone_add_to_list(str
 	list_add_tail(&tz->node, &thermal_tz_list);
 }
 
+static void thermal_zone_init_complete(struct thermal_zone_device *tz)
+{
+	guard(thermal_zone)(tz);
+
+	tz->state &= ~TZ_STATE_FLAG_INIT;
+	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+}
+
 /**
  * thermal_zone_device_register_with_trips() - register a new thermal zone device
  * @type:	the thermal zone device type
@@ -1435,6 +1443,8 @@ thermal_zone_device_register_with_trips(
 	tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay);
 	tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY;
 
+	tz->state = TZ_STATE_FLAG_INIT;
+
 	/* sys I/F */
 	/* Add nodes that are always present via .groups */
 	result = thermal_zone_create_device_groups(tz);
@@ -1486,9 +1496,7 @@ thermal_zone_device_register_with_trips(
 
 	mutex_unlock(&thermal_list_lock);
 
-	/* Update the new thermal zone and mark it as already updated. */
-	if (atomic_cmpxchg(&tz->need_update, 1, 0))
-		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+	thermal_zone_init_complete(tz);
 
 	thermal_notify_tz_create(tz);
 
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -64,6 +64,7 @@ struct thermal_governor {
 
 #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
 #define	TZ_STATE_FLAG_RESUMING	BIT(1)
+#define	TZ_STATE_FLAG_INIT	BIT(2)
 
 #define TZ_STATE_READY		0
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 05/20] thermal: core: Fix race between zone registration and system suspend
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2024-09-14 10:30 ` [RFC PATCH for 6.13 v1 04/20] thermal: core: Mark thermal zones as initializing to start with Rafael J. Wysocki
@ 2024-09-14 10:31 ` Rafael J. Wysocki
  2024-09-14 10:32 ` [RFC PATCH for 6.13 v1 06/20] thermal: core: Consolidate thermal zone locking during initialization Rafael J. Wysocki
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:31 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

If the registration of a thermal zone takes place at the same time when
system suspend is started, thermal_pm_notify() can run before the new
thermal zone is added to thermal_tz_list and its "suspended" flag will
not be set.  Consequently, if __thermal_zone_device_update() is called
for that thermal zone, it will not return early as expected which may
cause some destructive interference with the system suspend or resume
flow to occur.

To avoid that, make thermal_zone_init_complete() introduced previously
set the "suspended" flag for new thermal zones if it runs during system
suspend or resume.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -39,6 +39,8 @@ static DEFINE_MUTEX(thermal_governor_loc
 
 static struct thermal_governor *def_governor;
 
+static bool thermal_pm_suspended;
+
 /*
  * Governor section: set of functions to handle thermal governors
  *
@@ -1323,6 +1325,14 @@ static void thermal_zone_init_complete(s
 	guard(thermal_zone)(tz);
 
 	tz->state &= ~TZ_STATE_FLAG_INIT;
+	/*
+	 * If system suspend or resume is in progress at this point, the
+	 * new thermal zone needs to be marked as suspended because
+	 * thermal_pm_notify() has run already.
+	 */
+	if (thermal_pm_suspended)
+		tz->state |= TZ_STATE_FLAG_SUSPENDED;
+
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 }
 
@@ -1494,10 +1504,10 @@ thermal_zone_device_register_with_trips(
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
 		thermal_zone_cdev_bind(tz, cdev);
 
-	mutex_unlock(&thermal_list_lock);
-
 	thermal_zone_init_complete(tz);
 
+	mutex_unlock(&thermal_list_lock);
+
 	thermal_notify_tz_create(tz);
 
 	thermal_debug_tz_add(tz);
@@ -1718,6 +1728,8 @@ static int thermal_pm_notify(struct noti
 	case PM_SUSPEND_PREPARE:
 		mutex_lock(&thermal_list_lock);
 
+		thermal_pm_suspended = true;
+
 		list_for_each_entry(tz, &thermal_tz_list, node)
 			thermal_zone_pm_prepare(tz);
 
@@ -1728,6 +1740,8 @@ static int thermal_pm_notify(struct noti
 	case PM_POST_SUSPEND:
 		mutex_lock(&thermal_list_lock);
 
+		thermal_pm_suspended = false;
+
 		list_for_each_entry(tz, &thermal_tz_list, node)
 			thermal_zone_pm_complete(tz);
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 06/20] thermal: core: Consolidate thermal zone locking during initialization
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2024-09-14 10:31 ` [RFC PATCH for 6.13 v1 05/20] thermal: core: Fix race between zone registration and system suspend Rafael J. Wysocki
@ 2024-09-14 10:32 ` Rafael J. Wysocki
  2024-09-14 10:34 ` [RFC PATCH for 6.13 v1 07/20] thermal: core: Mark thermal zones as exiting before unregistration Rafael J. Wysocki
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:32 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

The part of thermal zone initialization carried out under
thermal_list_lock acquires the thermal zone lock and releases it
multiple times back-to-back which is not really necessary.

Instead of doing this, acquire the thermal zone lock once after
acquiring thermal_list_lock and release it along with that lock.

For this purpose, move all of the code in question to
thermal_zone_init_complete() introduced previously and provide an
"unloacked" variant of thermal_zone_cdev_bind() to be invoked from
there.

No intentional functional impact.

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

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -919,16 +919,14 @@ void print_bind_err_msg(struct thermal_z
 		cdev->type, thermal_zone_trip_id(tz, &td->trip), ret);
 }
 
-static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
-				   struct thermal_cooling_device *cdev)
+static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+				     struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
 
 	if (!tz->ops.should_bind)
 		return;
 
-	guard(thermal_zone)(tz);
-
 	for_each_trip_desc(tz, td) {
 		struct cooling_spec c = {
 			.upper = THERMAL_NO_LIMIT,
@@ -946,6 +944,14 @@ static void thermal_zone_cdev_bind(struc
 	}
 }
 
+static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+				   struct thermal_cooling_device *cdev)
+{
+	guard(thermal_zone)(tz);
+
+	__thermal_zone_cdev_bind(tz, cdev);
+}
+
 /**
  * __thermal_cooling_device_register() - register a new thermal cooling device
  * @np:		a pointer to a device tree node.
@@ -1313,17 +1319,20 @@ int thermal_zone_get_crit_temp(struct th
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
 
-static void thermal_zone_add_to_list(struct thermal_zone_device *tz)
+static void thermal_zone_init_complete(struct thermal_zone_device *tz)
 {
-	guard(thermal_zone)(tz);
+	struct thermal_cooling_device *cdev;
+
+	mutex_lock(&thermal_list_lock);
 
 	list_add_tail(&tz->node, &thermal_tz_list);
-}
 
-static void thermal_zone_init_complete(struct thermal_zone_device *tz)
-{
 	guard(thermal_zone)(tz);
 
+	/* Bind cooling devices for this zone. */
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		__thermal_zone_cdev_bind(tz, cdev);
+
 	tz->state &= ~TZ_STATE_FLAG_INIT;
 	/*
 	 * If system suspend or resume is in progress at this point, the
@@ -1334,6 +1343,8 @@ static void thermal_zone_init_complete(s
 		tz->state |= TZ_STATE_FLAG_SUSPENDED;
 
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+
+	mutex_unlock(&thermal_list_lock);
 }
 
 /**
@@ -1370,7 +1381,6 @@ thermal_zone_device_register_with_trips(
 					unsigned int polling_delay)
 {
 	const struct thermal_trip *trip = trips;
-	struct thermal_cooling_device *cdev;
 	struct thermal_zone_device *tz;
 	struct thermal_trip_desc *td;
 	int id;
@@ -1496,18 +1506,8 @@ thermal_zone_device_register_with_trips(
 			goto unregister;
 	}
 
-	mutex_lock(&thermal_list_lock);
-
-	thermal_zone_add_to_list(tz);
-
-	/* Bind cooling devices for this zone */
-	list_for_each_entry(cdev, &thermal_cdev_list, node)
-		thermal_zone_cdev_bind(tz, cdev);
-
 	thermal_zone_init_complete(tz);
 
-	mutex_unlock(&thermal_list_lock);
-
 	thermal_notify_tz_create(tz);
 
 	thermal_debug_tz_add(tz);




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 07/20] thermal: core: Mark thermal zones as exiting before unregistration
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2024-09-14 10:32 ` [RFC PATCH for 6.13 v1 06/20] thermal: core: Consolidate thermal zone locking during initialization Rafael J. Wysocki
@ 2024-09-14 10:34 ` Rafael J. Wysocki
  2024-09-14 10:35 ` [RFC PATCH for 6.13 v1 08/20] thermal: core: Consolidate thermal zone locking in the exit path Rafael J. Wysocki
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:34 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

In analogy with a previous change in the thermal zone registration code
path, to ensure that __thermal_zone_device_update() will return early
for thermal zones that are going away, introduce a thermal zone state
flag representing the "exit" state and set it along with deleting the
thermal zone from thermal_tz_list.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |    5 +++--
 drivers/thermal/thermal_core.h |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1563,10 +1563,11 @@ struct device *thermal_zone_device(struc
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device);
 
-static void thermal_zone_del_from_list(struct thermal_zone_device *tz)
+static void thermal_zone_exit(struct thermal_zone_device *tz)
 {
 	guard(thermal_zone)(tz);
 
+	tz->state |= TZ_STATE_FLAG_EXIT;
 	list_del(&tz->node);
 }
 
@@ -1594,7 +1595,7 @@ void thermal_zone_device_unregister(stru
 		return;
 	}
 
-	thermal_zone_del_from_list(tz);
+	thermal_zone_exit(tz);
 
 	/* Unbind all cdevs associated with 'this' thermal zone */
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -65,6 +65,7 @@ struct thermal_governor {
 #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
 #define	TZ_STATE_FLAG_RESUMING	BIT(1)
 #define	TZ_STATE_FLAG_INIT	BIT(2)
+#define	TZ_STATE_FLAG_EXIT	BIT(3)
 
 #define TZ_STATE_READY		0
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 08/20] thermal: core: Consolidate thermal zone locking in the exit path
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2024-09-14 10:34 ` [RFC PATCH for 6.13 v1 07/20] thermal: core: Mark thermal zones as exiting before unregistration Rafael J. Wysocki
@ 2024-09-14 10:35 ` Rafael J. Wysocki
  2024-09-14 10:36 ` [RFC PATCH for 6.13 v1 09/20] thermal: core: Update thermal zones after cooling device binding Rafael J. Wysocki
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:35 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

In analogy with a previous change in the thermal zone initialization
path, to avoid acquiring the thermal zone lock and releasing it multiple
times back-to-back unnecessarily, move all of the code running under
thermal_list_lock in thermal_zone_device_unregister() into
thermal_zone_exit() and make the latter acquire the thermal zone lock
only once and release it along with thermal_list_lock.

For this purpose, provide an "unlocked" variant of
thermal_zone_cdev_unbind() to be called by thermal_zone_exit()
under the thermal zone lock.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   53 +++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1249,17 +1249,23 @@ unlock_list:
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
 
-static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
+static void __thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
 				     struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
 
-	guard(thermal_zone)(tz);
-
 	for_each_trip_desc(tz, td)
 		thermal_unbind_cdev_from_trip(tz, td, cdev);
 }
 
+static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
+				     struct thermal_cooling_device *cdev)
+{
+	guard(thermal_zone)(tz);
+
+	__thermal_zone_cdev_unbind(tz, cdev);
+}
+
 /**
  * thermal_cooling_device_unregister - removes a thermal cooling device
  * @cdev:	the thermal cooling device to remove.
@@ -1563,12 +1569,31 @@ struct device *thermal_zone_device(struc
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device);
 
-static void thermal_zone_exit(struct thermal_zone_device *tz)
+static bool thermal_zone_exit(struct thermal_zone_device *tz)
 {
+	struct thermal_cooling_device *cdev;
+	bool ret = true;
+
+	mutex_lock(&thermal_list_lock);
+
+	if (list_empty(&tz->node)) {
+		ret = false;
+		goto unlock;
+	}
+
 	guard(thermal_zone)(tz);
 
 	tz->state |= TZ_STATE_FLAG_EXIT;
 	list_del(&tz->node);
+
+	/* Unbind all cdevs associated with this thermal zone. */
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		__thermal_zone_cdev_unbind(tz, cdev);
+
+unlock:
+	mutex_unlock(&thermal_list_lock);
+
+	return ret;
 }
 
 /**
@@ -1577,31 +1602,13 @@ static void thermal_zone_exit(struct the
  */
 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 {
-	struct thermal_cooling_device *cdev;
-	struct thermal_zone_device *pos = NULL;
-
 	if (!tz)
 		return;
 
 	thermal_debug_tz_remove(tz);
 
-	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(pos, &thermal_tz_list, node)
-		if (pos == tz)
-			break;
-	if (pos != tz) {
-		/* thermal zone device not found */
-		mutex_unlock(&thermal_list_lock);
+	if (!thermal_zone_exit(tz))
 		return;
-	}
-
-	thermal_zone_exit(tz);
-
-	/* Unbind all cdevs associated with 'this' thermal zone */
-	list_for_each_entry(cdev, &thermal_cdev_list, node)
-		thermal_zone_cdev_unbind(tz, cdev);
-
-	mutex_unlock(&thermal_list_lock);
 
 	cancel_delayed_work_sync(&tz->poll_queue);
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 09/20] thermal: core: Update thermal zones after cooling device binding
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2024-09-14 10:35 ` [RFC PATCH for 6.13 v1 08/20] thermal: core: Consolidate thermal zone locking in the exit path Rafael J. Wysocki
@ 2024-09-14 10:36 ` Rafael J. Wysocki
  2024-09-14 10:37 ` [RFC PATCH for 6.13 v1 10/20] thermal: core: Drop need_update field from struct thermal_zone_device Rafael J. Wysocki
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:36 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

If a new cooling device is registered and it is bound to at least one
trip point in a given thermal zone, that thermal zone needs to be
updated via __thermal_zone_device_update().

Instead of doing this with the help of the need_update atomic field in
struct thermal_zone_device, which is not particularly straightforward,
make __thermal_zone_cdev_bind() return a bool value indicating whether
or not the given thermal zone needs to be updated because a new cooling
device has been bound to it and update thermal_zone_cdev_bind() to
call __thermal_zone_device_update() when this value is "true".

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -919,13 +919,14 @@ void print_bind_err_msg(struct thermal_z
 		cdev->type, thermal_zone_trip_id(tz, &td->trip), ret);
 }
 
-static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
 				     struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
+	bool update_tz = false;
 
 	if (!tz->ops.should_bind)
-		return;
+		return false;
 
 	for_each_trip_desc(tz, td) {
 		struct cooling_spec c = {
@@ -939,9 +940,15 @@ static void __thermal_zone_cdev_bind(str
 			continue;
 
 		ret = thermal_bind_cdev_to_trip(tz, td, cdev, &c);
-		if (ret)
+		if (ret) {
 			print_bind_err_msg(tz, td, cdev, ret);
+			continue;
+		}
+
+		update_tz = true;
 	}
+
+	return update_tz;
 }
 
 static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
@@ -949,7 +956,8 @@ static void thermal_zone_cdev_bind(struc
 {
 	guard(thermal_zone)(tz);
 
-	__thermal_zone_cdev_bind(tz, cdev);
+	if (__thermal_zone_cdev_bind(tz, cdev))
+		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 }
 
 /**
@@ -974,7 +982,7 @@ __thermal_cooling_device_register(struct
 				  const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
-	struct thermal_zone_device *pos = NULL;
+	struct thermal_zone_device *pos;
 	unsigned long current_state;
 	int id, ret;
 
@@ -1050,11 +1058,6 @@ __thermal_cooling_device_register(struct
 	list_for_each_entry(pos, &thermal_tz_list, node)
 		thermal_zone_cdev_bind(pos, cdev);
 
-	list_for_each_entry(pos, &thermal_tz_list, node)
-		if (atomic_cmpxchg(&pos->need_update, 1, 0))
-			thermal_zone_device_update(pos,
-						   THERMAL_EVENT_UNSPECIFIED);
-
 	mutex_unlock(&thermal_list_lock);
 
 	return cdev;




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 10/20] thermal: core: Drop need_update field from struct thermal_zone_device
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (8 preceding siblings ...)
  2024-09-14 10:36 ` [RFC PATCH for 6.13 v1 09/20] thermal: core: Update thermal zones after cooling device binding Rafael J. Wysocki
@ 2024-09-14 10:37 ` Rafael J. Wysocki
  2024-09-14 10:38 ` [RFC PATCH for 6.13 v1 11/20] thermal: core: Separate code running under thermal_list_lock Rafael J. Wysocki
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:37 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

After previous changes, the need_update field in struct thermal_zone_device
is only set and never read, so drop it.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |    4 ----
 drivers/thermal/thermal_core.h |    2 --
 2 files changed, 6 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -826,7 +826,6 @@ static int thermal_bind_cdev_to_trip(str
 	if (!result) {
 		list_add_tail(&dev->trip_node, &td->thermal_instances);
 		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
-		atomic_set(&tz->need_update, 1);
 
 		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
 	}
@@ -1480,9 +1479,6 @@ thermal_zone_device_register_with_trips(
 	if (result)
 		goto remove_id;
 
-	/* A new thermal zone needs to be updated anyway. */
-	atomic_set(&tz->need_update, 1);
-
 	result = dev_set_name(&tz->device, "thermal_zone%d", tz->id);
 	if (result) {
 		thermal_zone_destroy_device_groups(tz);
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -99,7 +99,6 @@ struct thermal_governor {
 			trip point.
  * @prev_high_trip:	the above current temperature if you've crossed a
 			passive trip point.
- * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
  * @ops:	operations this &thermal_zone_device supports
  * @tzp:	thermal zone parameters
  * @governor:	pointer to the governor for this thermal zone
@@ -135,7 +134,6 @@ struct thermal_zone_device {
 	int passive;
 	int prev_low_trip;
 	int prev_high_trip;
-	atomic_t need_update;
 	struct thermal_zone_device_ops ops;
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 11/20] thermal: core: Separate code running under thermal_list_lock
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (9 preceding siblings ...)
  2024-09-14 10:37 ` [RFC PATCH for 6.13 v1 10/20] thermal: core: Drop need_update field from struct thermal_zone_device Rafael J. Wysocki
@ 2024-09-14 10:38 ` Rafael J. Wysocki
  2024-09-14 10:40 ` [RFC PATCH for 6.13 v1 12/20] thermal: core: Manage thermal_list_lock using a mutex guard Rafael J. Wysocki
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:38 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

To prepare for a subsequent change that will switch over the thermal
core to using a mutex guard for thermal_list_lock management, move the
code running under thermal_list_lock during the initialization and
unregistration of cooling devices into separate functions.

While at it, drop some comments that do not add value.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   64 +++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -959,6 +959,20 @@ static void thermal_zone_cdev_bind(struc
 		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 }
 
+static void thermal_cooling_device_init_complete(struct thermal_cooling_device *cdev)
+{
+	struct thermal_zone_device *tz;
+
+	mutex_lock(&thermal_list_lock);
+
+	list_add(&cdev->node, &thermal_cdev_list);
+
+	list_for_each_entry(tz, &thermal_tz_list, node)
+		thermal_zone_cdev_bind(tz, cdev);
+
+	mutex_unlock(&thermal_list_lock);
+}
+
 /**
  * __thermal_cooling_device_register() - register a new thermal cooling device
  * @np:		a pointer to a device tree node.
@@ -981,7 +995,6 @@ __thermal_cooling_device_register(struct
 				  const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
-	struct thermal_zone_device *pos;
 	unsigned long current_state;
 	int id, ret;
 
@@ -1048,16 +1061,7 @@ __thermal_cooling_device_register(struct
 	if (current_state <= cdev->max_state)
 		thermal_debug_cdev_add(cdev, current_state);
 
-	/* Add 'this' new cdev to the global cdev list */
-	mutex_lock(&thermal_list_lock);
-
-	list_add(&cdev->node, &thermal_cdev_list);
-
-	/* Update binding information for 'this' new cdev */
-	list_for_each_entry(pos, &thermal_tz_list, node)
-		thermal_zone_cdev_bind(pos, cdev);
-
-	mutex_unlock(&thermal_list_lock);
+	thermal_cooling_device_init_complete(cdev);
 
 	return cdev;
 
@@ -1268,38 +1272,42 @@ static void thermal_zone_cdev_unbind(str
 	__thermal_zone_cdev_unbind(tz, cdev);
 }
 
-/**
- * thermal_cooling_device_unregister - removes a thermal cooling device
- * @cdev:	the thermal cooling device to remove.
- *
- * thermal_cooling_device_unregister() must be called when a registered
- * thermal cooling device is no longer needed.
- */
-void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
+static bool thermal_cooling_device_exit(struct thermal_cooling_device *cdev)
 {
 	struct thermal_zone_device *tz;
-
-	if (!cdev)
-		return;
-
-	thermal_debug_cdev_remove(cdev);
+	bool ret = true;
 
 	mutex_lock(&thermal_list_lock);
 
 	if (!thermal_cooling_device_present(cdev)) {
-		mutex_unlock(&thermal_list_lock);
-		return;
+		ret = false;
+		goto unlock;
 	}
 
 	list_del(&cdev->node);
 
-	/* Unbind all thermal zones associated with 'this' cdev */
 	list_for_each_entry(tz, &thermal_tz_list, node)
 		thermal_zone_cdev_unbind(tz, cdev);
 
+unlock:
 	mutex_unlock(&thermal_list_lock);
 
-	device_unregister(&cdev->device);
+	return ret;
+}
+
+/**
+ * thermal_cooling_device_unregister() - removes a thermal cooling device
+ * @cdev: Thermal cooling device to remove.
+ */
+void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
+{
+	if (!cdev)
+		return;
+
+	thermal_debug_cdev_remove(cdev);
+
+	if (thermal_cooling_device_exit(cdev))
+		device_unregister(&cdev->device);
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_unregister);
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 12/20] thermal: core: Manage thermal_list_lock using a mutex guard
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (10 preceding siblings ...)
  2024-09-14 10:38 ` [RFC PATCH for 6.13 v1 11/20] thermal: core: Separate code running under thermal_list_lock Rafael J. Wysocki
@ 2024-09-14 10:40 ` Rafael J. Wysocki
  2024-09-14 10:41 ` [RFC PATCH for 6.13 v1 13/20] thermal: core: Call thermal_governor_update_tz() outside of cdev lock Rafael J. Wysocki
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:40 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Switch over the thermal core to using a mutex guard for
thermal_list_lock management.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |  151 ++++++++++++++++++-----------------------
 1 file changed, 69 insertions(+), 82 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -139,7 +139,7 @@ int thermal_register_governor(struct the
 			def_governor = governor;
 	}
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
 	list_for_each_entry(pos, &thermal_tz_list, node) {
 		/*
@@ -162,7 +162,6 @@ int thermal_register_governor(struct the
 		}
 	}
 
-	mutex_unlock(&thermal_list_lock);
 	mutex_unlock(&thermal_governor_lock);
 
 	return err;
@@ -180,7 +179,9 @@ void thermal_unregister_governor(struct
 	if (!__find_governor(governor->name))
 		goto exit;
 
-	mutex_lock(&thermal_list_lock);
+	list_del(&governor->governor_list);
+
+	guard(mutex)(&thermal_list_lock);
 
 	list_for_each_entry(pos, &thermal_tz_list, node) {
 		if (!strncasecmp(pos->governor->name, governor->name,
@@ -188,8 +189,6 @@ void thermal_unregister_governor(struct
 			thermal_set_governor(pos, NULL);
 	}
 
-	mutex_unlock(&thermal_list_lock);
-	list_del(&governor->governor_list);
 exit:
 	mutex_unlock(&thermal_governor_lock);
 }
@@ -681,50 +680,50 @@ int for_each_thermal_cooling_device(int
 					      void *), void *data)
 {
 	struct thermal_cooling_device *cdev;
-	int ret = 0;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
+
 	list_for_each_entry(cdev, &thermal_cdev_list, node) {
+		int ret;
+
 		ret = cb(cdev, data);
 		if (ret)
-			break;
+			return ret;
 	}
-	mutex_unlock(&thermal_list_lock);
 
-	return ret;
+	return 0;
 }
 
 int for_each_thermal_zone(int (*cb)(struct thermal_zone_device *, void *),
 			  void *data)
 {
 	struct thermal_zone_device *tz;
-	int ret = 0;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
+
 	list_for_each_entry(tz, &thermal_tz_list, node) {
+		int ret;
+
 		ret = cb(tz, data);
 		if (ret)
-			break;
+			return ret;
 	}
-	mutex_unlock(&thermal_list_lock);
 
-	return ret;
+	return 0;
 }
 
 struct thermal_zone_device *thermal_zone_get_by_id(int id)
 {
-	struct thermal_zone_device *tz, *match = NULL;
+	struct thermal_zone_device *tz;
+
+	guard(mutex)(&thermal_list_lock);
 
-	mutex_lock(&thermal_list_lock);
 	list_for_each_entry(tz, &thermal_tz_list, node) {
-		if (tz->id == id) {
-			match = tz;
-			break;
-		}
+		if (tz->id == id)
+			return tz;
 	}
-	mutex_unlock(&thermal_list_lock);
 
-	return match;
+	return NULL;
 }
 
 /*
@@ -963,14 +962,12 @@ static void thermal_cooling_device_init_
 {
 	struct thermal_zone_device *tz;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
 	list_add(&cdev->node, &thermal_cdev_list);
 
 	list_for_each_entry(tz, &thermal_tz_list, node)
 		thermal_zone_cdev_bind(tz, cdev);
-
-	mutex_unlock(&thermal_list_lock);
 }
 
 /**
@@ -1204,10 +1201,10 @@ void thermal_cooling_device_update(struc
 	 * Hold thermal_list_lock throughout the update to prevent the device
 	 * from going away while being updated.
 	 */
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
 	if (!thermal_cooling_device_present(cdev))
-		goto unlock_list;
+		return;
 
 	/*
 	 * Update under the cdev lock to prevent the state from being set beyond
@@ -1249,9 +1246,6 @@ void thermal_cooling_device_update(struc
 
 unlock:
 	mutex_unlock(&cdev->lock);
-
-unlock_list:
-	mutex_unlock(&thermal_list_lock);
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
 
@@ -1275,24 +1269,18 @@ static void thermal_zone_cdev_unbind(str
 static bool thermal_cooling_device_exit(struct thermal_cooling_device *cdev)
 {
 	struct thermal_zone_device *tz;
-	bool ret = true;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
-	if (!thermal_cooling_device_present(cdev)) {
-		ret = false;
-		goto unlock;
-	}
+	if (!thermal_cooling_device_present(cdev))
+		return false;
 
 	list_del(&cdev->node);
 
 	list_for_each_entry(tz, &thermal_tz_list, node)
 		thermal_zone_cdev_unbind(tz, cdev);
 
-unlock:
-	mutex_unlock(&thermal_list_lock);
-
-	return ret;
+	return true;
 }
 
 /**
@@ -1339,7 +1327,7 @@ static void thermal_zone_init_complete(s
 {
 	struct thermal_cooling_device *cdev;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
 	list_add_tail(&tz->node, &thermal_tz_list);
 
@@ -1359,8 +1347,6 @@ static void thermal_zone_init_complete(s
 		tz->state |= TZ_STATE_FLAG_SUSPENDED;
 
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
-	mutex_unlock(&thermal_list_lock);
 }
 
 /**
@@ -1579,14 +1565,11 @@ EXPORT_SYMBOL_GPL(thermal_zone_device);
 static bool thermal_zone_exit(struct thermal_zone_device *tz)
 {
 	struct thermal_cooling_device *cdev;
-	bool ret = true;
 
-	mutex_lock(&thermal_list_lock);
+	guard(mutex)(&thermal_list_lock);
 
-	if (list_empty(&tz->node)) {
-		ret = false;
-		goto unlock;
-	}
+	if (list_empty(&tz->node))
+		return false;
 
 	guard(thermal_zone)(tz);
 
@@ -1597,10 +1580,7 @@ static bool thermal_zone_exit(struct the
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
 		__thermal_zone_cdev_unbind(tz, cdev);
 
-unlock:
-	mutex_unlock(&thermal_list_lock);
-
-	return ret;
+	return true;
 }
 
 /**
@@ -1654,24 +1634,23 @@ struct thermal_zone_device *thermal_zone
 	unsigned int found = 0;
 
 	if (!name)
-		goto exit;
+		return ERR_PTR(-EINVAL);
+
+	guard(mutex)(&thermal_list_lock);
 
-	mutex_lock(&thermal_list_lock);
 	list_for_each_entry(pos, &thermal_tz_list, node)
 		if (!strncasecmp(name, pos->type, THERMAL_NAME_LENGTH)) {
 			found++;
 			ref = pos;
 		}
-	mutex_unlock(&thermal_list_lock);
 
-	/* nothing has been found, thus an error code for it */
-	if (found == 0)
-		ref = ERR_PTR(-ENODEV);
-	else if (found > 1)
-	/* Success only when an unique zone is found */
-		ref = ERR_PTR(-EEXIST);
+	if (!found)
+		return ERR_PTR(-ENODEV);
+
+	/* Success only when one zone is found. */
+	if (found > 1)
+		return ERR_PTR(-EEXIST);
 
-exit:
 	return ref;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name);
@@ -1714,6 +1693,18 @@ static void thermal_zone_pm_prepare(stru
 	tz->state |= TZ_STATE_FLAG_SUSPENDED;
 }
 
+static void thermal_pm_notify_prepare(void)
+{
+	struct thermal_zone_device *tz;
+
+	guard(mutex)(&thermal_list_lock);
+
+	thermal_pm_suspended = true;
+
+	list_for_each_entry(tz, &thermal_tz_list, node)
+		thermal_zone_pm_prepare(tz);
+}
+
 static void thermal_zone_pm_complete(struct thermal_zone_device *tz)
 {
 	guard(thermal_zone)(tz);
@@ -1732,35 +1723,31 @@ static void thermal_zone_pm_complete(str
 	mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, 0);
 }
 
-static int thermal_pm_notify(struct notifier_block *nb,
-			     unsigned long mode, void *_unused)
+static void thermal_pm_notify_complete(void)
 {
 	struct thermal_zone_device *tz;
 
+	guard(mutex)(&thermal_list_lock);
+
+	thermal_pm_suspended = false;
+
+	list_for_each_entry(tz, &thermal_tz_list, node)
+		thermal_zone_pm_complete(tz);
+}
+
+static int thermal_pm_notify(struct notifier_block *nb,
+			     unsigned long mode, void *_unused)
+{
 	switch (mode) {
 	case PM_HIBERNATION_PREPARE:
 	case PM_RESTORE_PREPARE:
 	case PM_SUSPEND_PREPARE:
-		mutex_lock(&thermal_list_lock);
-
-		thermal_pm_suspended = true;
-
-		list_for_each_entry(tz, &thermal_tz_list, node)
-			thermal_zone_pm_prepare(tz);
-
-		mutex_unlock(&thermal_list_lock);
+		thermal_pm_notify_prepare();
 		break;
 	case PM_POST_HIBERNATION:
 	case PM_POST_RESTORE:
 	case PM_POST_SUSPEND:
-		mutex_lock(&thermal_list_lock);
-
-		thermal_pm_suspended = false;
-
-		list_for_each_entry(tz, &thermal_tz_list, node)
-			thermal_zone_pm_complete(tz);
-
-		mutex_unlock(&thermal_list_lock);
+		thermal_pm_notify_complete();
 		break;
 	default:
 		break;




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 13/20] thermal: core: Call thermal_governor_update_tz() outside of cdev lock
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (11 preceding siblings ...)
  2024-09-14 10:40 ` [RFC PATCH for 6.13 v1 12/20] thermal: core: Manage thermal_list_lock using a mutex guard Rafael J. Wysocki
@ 2024-09-14 10:41 ` Rafael J. Wysocki
  2024-09-14 10:43 ` [RFC PATCH for 6.13 v1 14/20] thermal: core: Introduce thermal_instance_add() Rafael J. Wysocki
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:41 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Holding a cooling device lock around thermal_governor_update_tz() calls
is not necessary and it may cause lockdep to complain if any governor's
.update_tz() callback attempts to lock a cdev.

For this reason, move the thermal_governor_update_tz() calls in
thermal_bind_cdev_to_trip() and thermal_unbind_cdev_from_trip() from
under the cdev lock.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -825,13 +825,13 @@ static int thermal_bind_cdev_to_trip(str
 	if (!result) {
 		list_add_tail(&dev->trip_node, &td->thermal_instances);
 		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
-
-		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
 	}
 	mutex_unlock(&cdev->lock);
 
-	if (!result)
+	if (!result) {
+		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
 		return 0;
+	}
 
 	device_remove_file(&tz->device, &dev->weight_attr);
 remove_trip_file:
@@ -866,9 +866,6 @@ static void thermal_unbind_cdev_from_tri
 		if (pos->cdev == cdev) {
 			list_del(&pos->trip_node);
 			list_del(&pos->cdev_node);
-
-			thermal_governor_update_tz(tz, THERMAL_TZ_UNBIND_CDEV);
-
 			mutex_unlock(&cdev->lock);
 			goto unbind;
 		}
@@ -878,6 +875,8 @@ static void thermal_unbind_cdev_from_tri
 	return;
 
 unbind:
+	thermal_governor_update_tz(tz, THERMAL_TZ_UNBIND_CDEV);
+
 	device_remove_file(&tz->device, &pos->weight_attr);
 	device_remove_file(&tz->device, &pos->attr);
 	sysfs_remove_link(&tz->device.kobj, pos->name);




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 14/20] thermal: core: Introduce thermal_instance_add()
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (12 preceding siblings ...)
  2024-09-14 10:41 ` [RFC PATCH for 6.13 v1 13/20] thermal: core: Call thermal_governor_update_tz() outside of cdev lock Rafael J. Wysocki
@ 2024-09-14 10:43 ` Rafael J. Wysocki
  2024-09-14 10:44 ` [RFC PATCH for 6.13 v1 15/20] thermal: core: Introduce thermal_instance_delete() Rafael J. Wysocki
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:43 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

To reduce the number of redundant result checks in
thermal_bind_cdev_to_trip() and make the code in it easier to
follow, move some of that code to a new function called
thermal_instance_add() and make thermal_bind_cdev_to_trip()
invoke that function.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   46 ++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -736,6 +736,28 @@ struct thermal_zone_device *thermal_zone
  *				     binding, and unbinding.
  */
 
+static int thermal_instance_add(struct thermal_instance *new_instance,
+				struct thermal_cooling_device *cdev,
+				struct thermal_trip_desc *td)
+{
+	struct thermal_instance *instance;
+
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
+		if (instance->cdev == cdev)
+			return -EEXIST;
+	}
+
+	list_add_tail(&new_instance->trip_node, &td->thermal_instances);
+
+	mutex_lock(&cdev->lock);
+
+	list_add_tail(&new_instance->cdev_node, &cdev->thermal_instances);
+
+	mutex_unlock(&cdev->lock);
+
+	return 0;
+}
+
 /**
  * thermal_bind_cdev_to_trip - bind a cooling device to a thermal zone
  * @tz:		pointer to struct thermal_zone_device
@@ -754,7 +776,7 @@ static int thermal_bind_cdev_to_trip(str
 				     struct thermal_cooling_device *cdev,
 				     struct cooling_spec *cool_spec)
 {
-	struct thermal_instance *dev, *instance;
+	struct thermal_instance *dev;
 	bool upper_no_limit;
 	int result;
 
@@ -816,23 +838,15 @@ static int thermal_bind_cdev_to_trip(str
 	if (result)
 		goto remove_trip_file;
 
-	mutex_lock(&cdev->lock);
-	list_for_each_entry(instance, &td->thermal_instances, trip_node)
-		if (instance->cdev == cdev) {
-			result = -EEXIST;
-			break;
-		}
-	if (!result) {
-		list_add_tail(&dev->trip_node, &td->thermal_instances);
-		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
-	}
-	mutex_unlock(&cdev->lock);
+	result = thermal_instance_add(dev, cdev, td);
+	if (result)
+		goto remove_weight_file;
 
-	if (!result) {
-		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
-		return 0;
-	}
+	thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
+
+	return 0;
 
+remove_weight_file:
 	device_remove_file(&tz->device, &dev->weight_attr);
 remove_trip_file:
 	device_remove_file(&tz->device, &dev->attr);




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 15/20] thermal: core: Introduce thermal_instance_delete()
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (13 preceding siblings ...)
  2024-09-14 10:43 ` [RFC PATCH for 6.13 v1 14/20] thermal: core: Introduce thermal_instance_add() Rafael J. Wysocki
@ 2024-09-14 10:44 ` Rafael J. Wysocki
  2024-09-14 10:45 ` [RFC PATCH for 6.13 v1 16/20] thermal: core: Introduce thermal_cdev_update_nocheck() Rafael J. Wysocki
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:44 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

It is not necessary to walk the thermal_instances list in a trip
descriptor under a cooling device lock, so acquire that lock only
for deleting the given thermal instance from the list of thermal
instances in the given cdev.

Moreover, in analogy with the previous change that introduced
thermal_instance_add(), put the code deleting the given thermal
instance from the lists it is on into a separate new function
called thermal_instance_delete().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -859,6 +859,17 @@ free_mem:
 	return result;
 }
 
+static void thermal_instance_delete(struct thermal_instance *instance)
+{
+	list_del(&instance->trip_node);
+
+	mutex_lock(&instance->cdev->lock);
+
+	list_del(&instance->cdev_node);
+
+	mutex_unlock(&instance->cdev->lock);
+}
+
 /**
  * thermal_unbind_cdev_from_trip - unbind a cooling device from a thermal zone.
  * @tz:		pointer to a struct thermal_zone_device.
@@ -875,16 +886,12 @@ static void thermal_unbind_cdev_from_tri
 {
 	struct thermal_instance *pos, *next;
 
-	mutex_lock(&cdev->lock);
 	list_for_each_entry_safe(pos, next, &td->thermal_instances, trip_node) {
 		if (pos->cdev == cdev) {
-			list_del(&pos->trip_node);
-			list_del(&pos->cdev_node);
-			mutex_unlock(&cdev->lock);
+			thermal_instance_delete(pos);
 			goto unbind;
 		}
 	}
-	mutex_unlock(&cdev->lock);
 
 	return;
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 16/20] thermal: core: Introduce thermal_cdev_update_nocheck()
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (14 preceding siblings ...)
  2024-09-14 10:44 ` [RFC PATCH for 6.13 v1 15/20] thermal: core: Introduce thermal_instance_delete() Rafael J. Wysocki
@ 2024-09-14 10:45 ` Rafael J. Wysocki
  2024-09-14 10:47 ` [RFC PATCH for 6.13 v1 17/20] thermal: core: Add and use cooling device guard Rafael J. Wysocki
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:45 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Three thermal governors call __thermal_cdev_update() under the
cdev lock without doing any checks, so in order to reduce the
related code duplication, introduce a new helper function called
thermal_cdev_update_nocheck() for them and make them use it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/gov_bang_bang.c       |    4 +---
 drivers/thermal/gov_fair_share.c      |    4 +---
 drivers/thermal/gov_power_allocator.c |    5 ++---
 drivers/thermal/thermal_core.h        |    1 +
 drivers/thermal/thermal_helpers.c     |   13 +++++++++++++
 5 files changed, 18 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/thermal/gov_bang_bang.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_bang_bang.c
+++ linux-pm/drivers/thermal/gov_bang_bang.c
@@ -30,9 +30,7 @@ static void bang_bang_set_instance_targe
 
 	dev_dbg(&instance->cdev->device, "target=%ld\n", instance->target);
 
-	mutex_lock(&instance->cdev->lock);
-	__thermal_cdev_update(instance->cdev);
-	mutex_unlock(&instance->cdev->lock);
+	thermal_cdev_update_nocheck(instance->cdev);
 }
 
 /**
Index: linux-pm/drivers/thermal/gov_fair_share.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_fair_share.c
+++ linux-pm/drivers/thermal/gov_fair_share.c
@@ -89,9 +89,7 @@ static void fair_share_throttle(struct t
 		}
 		instance->target = div_u64(dividend, divisor);
 
-		mutex_lock(&cdev->lock);
-		__thermal_cdev_update(cdev);
-		mutex_unlock(&cdev->lock);
+		thermal_cdev_update_nocheck(cdev);
 	}
 }
 
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
@@ -317,9 +317,8 @@ power_actor_set_power(struct thermal_coo
 		return ret;
 
 	instance->target = clamp_val(state, instance->lower, instance->upper);
-	mutex_lock(&cdev->lock);
-	__thermal_cdev_update(cdev);
-	mutex_unlock(&cdev->lock);
+
+	thermal_cdev_update_nocheck(cdev);
 
 	return 0;
 }
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -212,6 +212,7 @@ static inline bool cdev_is_power_actor(s
 }
 
 void thermal_cdev_update(struct thermal_cooling_device *);
+void thermal_cdev_update_nocheck(struct thermal_cooling_device *cdev);
 void __thermal_cdev_update(struct thermal_cooling_device *cdev);
 
 int get_tz_trend(struct thermal_zone_device *tz, const struct thermal_trip *trip);
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -206,6 +206,19 @@ void thermal_cdev_update(struct thermal_
 }
 
 /**
+ * thermal_cdev_update_nocheck() - Unconditionally update cooling device state
+ * @cdev: Target cooling device.
+ */
+void thermal_cdev_update_nocheck(struct thermal_cooling_device *cdev)
+{
+	mutex_lock(&cdev->lock);
+
+	__thermal_cdev_update(cdev);
+
+	mutex_unlock(&cdev->lock);
+}
+
+/**
  * thermal_zone_get_slope - return the slope attribute of the thermal zone
  * @tz: thermal zone device with the slope attribute
  *




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 17/20] thermal: core: Add and use cooling device guard
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (15 preceding siblings ...)
  2024-09-14 10:45 ` [RFC PATCH for 6.13 v1 16/20] thermal: core: Introduce thermal_cdev_update_nocheck() Rafael J. Wysocki
@ 2024-09-14 10:47 ` Rafael J. Wysocki
  2024-09-14 10:48 ` [RFC PATCH for 6.13 v1 18/20] thermal: core: Call __thermal_cdev_update() on cdev unbind Rafael J. Wysocki
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:47 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Add and use a special guard for cooling devices.

This allows quite a few error code paths to be simplified among
other things and brings in code size reduction for a good measure.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/gov_power_allocator.c |   21 +++++++++----------
 drivers/thermal/gov_step_wise.c       |    6 ++---
 drivers/thermal/thermal_core.c        |   17 ++++-----------
 drivers/thermal/thermal_debugfs.c     |   25 +++++++++++++---------
 drivers/thermal/thermal_helpers.c     |   19 ++++-------------
 drivers/thermal/thermal_sysfs.c       |   37 ++++++++++------------------------
 include/linux/thermal.h               |    3 ++
 7 files changed, 52 insertions(+), 76 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -749,12 +749,10 @@ static int thermal_instance_add(struct t
 
 	list_add_tail(&new_instance->trip_node, &td->thermal_instances);
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	list_add_tail(&new_instance->cdev_node, &cdev->thermal_instances);
 
-	mutex_unlock(&cdev->lock);
-
 	return 0;
 }
 
@@ -863,11 +861,9 @@ static void thermal_instance_delete(stru
 {
 	list_del(&instance->trip_node);
 
-	mutex_lock(&instance->cdev->lock);
+	guard(cooling_dev)(instance->cdev);
 
 	list_del(&instance->cdev_node);
-
-	mutex_unlock(&instance->cdev->lock);
 }
 
 /**
@@ -1230,10 +1226,10 @@ void thermal_cooling_device_update(struc
 	 * Update under the cdev lock to prevent the state from being set beyond
 	 * the new limit concurrently.
 	 */
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	if (cdev->ops->get_max_state(cdev, &cdev->max_state))
-		goto unlock;
+		return;
 
 	thermal_cooling_device_stats_reinit(cdev);
 
@@ -1260,12 +1256,9 @@ void thermal_cooling_device_update(struc
 	}
 
 	if (cdev->ops->get_cur_state(cdev, &state) || state > cdev->max_state)
-		goto unlock;
+		return;
 
 	thermal_cooling_device_stats_update(cdev, state);
-
-unlock:
-	mutex_unlock(&cdev->lock);
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
 
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -137,6 +137,9 @@ struct thermal_cooling_device {
 #endif
 };
 
+DEFINE_GUARD(cooling_dev, struct thermal_cooling_device *, mutex_lock(&_T->lock),
+	     mutex_unlock(&_T->lock))
+
 /* Structure to define Thermal Zone parameters */
 struct thermal_zone_params {
 	const char *governor_name;
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -544,13 +544,12 @@ cur_state_store(struct device *dev, stru
 	if (state > cdev->max_state)
 		return -EINVAL;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	result = cdev->ops->set_cur_state(cdev, state);
 	if (!result)
 		thermal_cooling_device_stats_update(cdev, state);
 
-	mutex_unlock(&cdev->lock);
 	return result ? result : count;
 }
 
@@ -625,21 +624,18 @@ static ssize_t total_trans_show(struct d
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
 	struct cooling_dev_stats *stats;
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return 0;
 
 	spin_lock(&stats->lock);
 	ret = sprintf(buf, "%u\n", stats->total_trans);
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return ret;
 }
 
@@ -652,11 +648,11 @@ time_in_state_ms_show(struct device *dev
 	ssize_t len = 0;
 	int i;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return 0;
 
 	spin_lock(&stats->lock);
 
@@ -668,9 +664,6 @@ time_in_state_ms_show(struct device *dev
 	}
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return len;
 }
 
@@ -682,11 +675,11 @@ reset_store(struct device *dev, struct d
 	struct cooling_dev_stats *stats;
 	int i, states;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return count;
 
 	states = cdev->max_state + 1;
 
@@ -702,9 +695,6 @@ reset_store(struct device *dev, struct d
 
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return count;
 }
 
@@ -716,13 +706,11 @@ static ssize_t trans_table_show(struct d
 	ssize_t len = 0;
 	int i, j;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
-	if (!stats) {
-		len = -ENODATA;
-		goto unlock;
-	}
+	if (!stats)
+		return -ENODATA;
 
 	len += snprintf(buf + len, PAGE_SIZE - len, " From  :    To\n");
 	len += snprintf(buf + len, PAGE_SIZE - len, "       : ");
@@ -760,9 +748,6 @@ static ssize_t trans_table_show(struct d
 		len = -EFBIG;
 	}
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return len;
 }
 
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -58,17 +58,10 @@ bool thermal_trip_is_bound_to_cdev(struc
 				   const struct thermal_trip *trip,
 				   struct thermal_cooling_device *cdev)
 {
-	bool ret;
-
 	guard(thermal_zone)(tz);
+	guard(cooling_dev)(cdev);
 
-	mutex_lock(&cdev->lock);
-
-	ret = thermal_instance_present(tz, cdev, trip);
-
-	mutex_unlock(&cdev->lock);
-
-	return ret;
+	return thermal_instance_present(tz, cdev, trip);
 }
 EXPORT_SYMBOL_GPL(thermal_trip_is_bound_to_cdev);
 
@@ -197,12 +190,12 @@ void __thermal_cdev_update(struct therma
  */
 void thermal_cdev_update(struct thermal_cooling_device *cdev)
 {
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
+
 	if (!cdev->updated) {
 		__thermal_cdev_update(cdev);
 		cdev->updated = true;
 	}
-	mutex_unlock(&cdev->lock);
 }
 
 /**
@@ -211,11 +204,9 @@ void thermal_cdev_update(struct thermal_
  */
 void thermal_cdev_update_nocheck(struct thermal_cooling_device *cdev)
 {
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	__thermal_cdev_update(cdev);
-
-	mutex_unlock(&cdev->lock);
 }
 
 /**
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -516,6 +516,19 @@ void thermal_debug_cdev_add(struct therm
 	cdev->debugfs = thermal_dbg;
 }
 
+static struct thermal_debugfs *thermal_debug_cdev_clear(struct thermal_cooling_device *cdev)
+{
+	struct thermal_debugfs *thermal_dbg;
+
+	guard(cooling_dev)(cdev);
+
+	thermal_dbg = cdev->debugfs;
+	if (thermal_dbg)
+		cdev->debugfs = NULL;
+
+	return thermal_dbg;
+}
+
 /**
  * thermal_debug_cdev_remove - Remove a cooling device debugfs entry
  *
@@ -527,17 +540,9 @@ void thermal_debug_cdev_remove(struct th
 {
 	struct thermal_debugfs *thermal_dbg;
 
-	mutex_lock(&cdev->lock);
-
-	thermal_dbg = cdev->debugfs;
-	if (!thermal_dbg) {
-		mutex_unlock(&cdev->lock);
+	thermal_dbg = thermal_debug_cdev_clear(cdev);
+	if (!thermal_dbg)
 		return;
-	}
-
-	cdev->debugfs = NULL;
-
-	mutex_unlock(&cdev->lock);
 
 	mutex_lock(&thermal_dbg->lock);
 
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
@@ -544,18 +544,17 @@ static void allow_maximum_power(struct t
 		cdev = instance->cdev;
 
 		instance->target = 0;
-		mutex_lock(&cdev->lock);
-		/*
-		 * Call for updating the cooling devices local stats and avoid
-		 * periods of dozen of seconds when those have not been
-		 * maintained.
-		 */
-		cdev->ops->get_requested_power(cdev, &req_power);
+		scoped_guard(cooling_dev, cdev) {
+			/*
+			 * Call for updating the cooling devices local stats and
+			 * avoid periods of dozen of seconds when those have not
+			 * been maintained.
+			 */
+			cdev->ops->get_requested_power(cdev, &req_power);
 
-		if (params->update_cdevs)
-			__thermal_cdev_update(cdev);
-
-		mutex_unlock(&cdev->lock);
+			if (params->update_cdevs)
+				__thermal_cdev_update(cdev);
+		}
 	}
 }
 
Index: linux-pm/drivers/thermal/gov_step_wise.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_step_wise.c
+++ linux-pm/drivers/thermal/gov_step_wise.c
@@ -97,9 +97,9 @@ static void thermal_zone_trip_update(str
 
 		instance->initialized = true;
 
-		mutex_lock(&instance->cdev->lock);
-		instance->cdev->updated = false; /* cdev needs update */
-		mutex_unlock(&instance->cdev->lock);
+		scoped_guard(cooling_dev, instance->cdev) {
+			instance->cdev->updated = false; /* cdev needs update */
+		}
 	}
 }
 




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 18/20] thermal: core: Call __thermal_cdev_update() on cdev unbind
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (16 preceding siblings ...)
  2024-09-14 10:47 ` [RFC PATCH for 6.13 v1 17/20] thermal: core: Add and use cooling device guard Rafael J. Wysocki
@ 2024-09-14 10:48 ` Rafael J. Wysocki
  2024-09-14 10:49 ` [RFC PATCH for 6.13 v1 19/20] thermal: core: Separate thermal zone governor initialization Rafael J. Wysocki
  2024-09-14 10:50 ` [RFC PATCH for 6.13 v1 20/20] thermal: core: Manage thermal_governor_lock using a mutex guard Rafael J. Wysocki
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:48 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

When deleting a thermal instance from a cooling device's list of thermal
instances, update it in case its state has been determined by the thermal
instance going away.

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

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -864,6 +864,12 @@ static void thermal_instance_delete(stru
 	guard(cooling_dev)(instance->cdev);
 
 	list_del(&instance->cdev_node);
+
+	/*
+	 * Update the cdev in case its state has been determined by the thermal
+	 * instance going away.
+	 */
+	__thermal_cdev_update(instance->cdev);
 }
 
 /**




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 19/20] thermal: core: Separate thermal zone governor initialization
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (17 preceding siblings ...)
  2024-09-14 10:48 ` [RFC PATCH for 6.13 v1 18/20] thermal: core: Call __thermal_cdev_update() on cdev unbind Rafael J. Wysocki
@ 2024-09-14 10:49 ` Rafael J. Wysocki
  2024-09-14 10:50 ` [RFC PATCH for 6.13 v1 20/20] thermal: core: Manage thermal_governor_lock using a mutex guard Rafael J. Wysocki
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:49 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

In preparation for a subsequent change that will switch over the thermal
core to using a mutex guard for managing thermal_governor_lock, move
the code running in thermal_zone_device_register_with_trips() under that
lock into a separate function called thermal_zone_init_governor().

While at it, drop a useless comment.

No intentional functional impact.

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

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1342,6 +1342,25 @@ int thermal_zone_get_crit_temp(struct th
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
 
+static int thermal_zone_init_governor(struct thermal_zone_device *tz)
+{
+	struct thermal_governor *governor;
+	int ret;
+
+	mutex_lock(&thermal_governor_lock);
+
+	if (tz->tzp)
+		governor = __find_governor(tz->tzp->governor_name);
+	else
+		governor = def_governor;
+
+	ret = thermal_set_governor(tz, governor);
+
+	mutex_unlock(&thermal_governor_lock);
+
+	return ret;
+}
+
 static void thermal_zone_init_complete(struct thermal_zone_device *tz)
 {
 	struct thermal_cooling_device *cdev;
@@ -1406,7 +1425,6 @@ thermal_zone_device_register_with_trips(
 	struct thermal_trip_desc *td;
 	int id;
 	int result;
-	struct thermal_governor *governor;
 
 	if (!type || strlen(type) == 0) {
 		pr_err("No thermal zone type defined\n");
@@ -1502,21 +1520,9 @@ thermal_zone_device_register_with_trips(
 	if (result)
 		goto release_device;
 
-	/* Update 'this' zone's governor information */
-	mutex_lock(&thermal_governor_lock);
-
-	if (tz->tzp)
-		governor = __find_governor(tz->tzp->governor_name);
-	else
-		governor = def_governor;
-
-	result = thermal_set_governor(tz, governor);
-	if (result) {
-		mutex_unlock(&thermal_governor_lock);
+	result = thermal_zone_init_governor(tz);
+	if (result)
 		goto unregister;
-	}
-
-	mutex_unlock(&thermal_governor_lock);
 
 	if (!tz->tzp || !tz->tzp->no_hwmon) {
 		result = thermal_add_hwmon_sysfs(tz);




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH for 6.13 v1 20/20] thermal: core: Manage thermal_governor_lock using a mutex guard
  2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
                   ` (18 preceding siblings ...)
  2024-09-14 10:49 ` [RFC PATCH for 6.13 v1 19/20] thermal: core: Separate thermal zone governor initialization Rafael J. Wysocki
@ 2024-09-14 10:50 ` Rafael J. Wysocki
  19 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2024-09-14 10:50 UTC (permalink / raw)
  To: Linux PM; +Cc: LKML, Daniel Lezcano, Lukasz Luba, Zhang Rui

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

Switch over the thermal core to using a mutex guard for
thermal_governor_lock management.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -123,7 +123,7 @@ int thermal_register_governor(struct the
 	if (!governor)
 		return -EINVAL;
 
-	mutex_lock(&thermal_governor_lock);
+	guard(mutex)(&thermal_governor_lock);
 
 	err = -EBUSY;
 	if (!__find_governor(governor->name)) {
@@ -162,8 +162,6 @@ int thermal_register_governor(struct the
 		}
 	}
 
-	mutex_unlock(&thermal_governor_lock);
-
 	return err;
 }
 
@@ -174,10 +172,10 @@ void thermal_unregister_governor(struct
 	if (!governor)
 		return;
 
-	mutex_lock(&thermal_governor_lock);
+	guard(mutex)(&thermal_governor_lock);
 
 	if (!__find_governor(governor->name))
-		goto exit;
+		return;
 
 	list_del(&governor->governor_list);
 
@@ -188,9 +186,6 @@ void thermal_unregister_governor(struct
 				 THERMAL_NAME_LENGTH))
 			thermal_set_governor(pos, NULL);
 	}
-
-exit:
-	mutex_unlock(&thermal_governor_lock);
 }
 
 int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
@@ -199,16 +194,13 @@ int thermal_zone_device_set_policy(struc
 	struct thermal_governor *gov;
 	int ret = -EINVAL;
 
-	mutex_lock(&thermal_governor_lock);
-
+	guard(mutex)(&thermal_governor_lock);
 	guard(thermal_zone)(tz);
 
 	gov = __find_governor(strim(policy));
 	if (gov)
 		ret = thermal_set_governor(tz, gov);
 
-	mutex_unlock(&thermal_governor_lock);
-
 	thermal_notify_tz_gov_change(tz, policy);
 
 	return ret;
@@ -219,15 +211,13 @@ int thermal_build_list_of_policies(char
 	struct thermal_governor *pos;
 	ssize_t count = 0;
 
-	mutex_lock(&thermal_governor_lock);
+	guard(mutex)(&thermal_governor_lock);
 
 	list_for_each_entry(pos, &thermal_governor_list, governor_list) {
 		count += sysfs_emit_at(buf, count, "%s ", pos->name);
 	}
 	count += sysfs_emit_at(buf, count, "\n");
 
-	mutex_unlock(&thermal_governor_lock);
-
 	return count;
 }
 
@@ -663,17 +653,18 @@ int for_each_thermal_governor(int (*cb)(
 			      void *data)
 {
 	struct thermal_governor *gov;
-	int ret = 0;
 
-	mutex_lock(&thermal_governor_lock);
+	guard(mutex)(&thermal_governor_lock);
+
 	list_for_each_entry(gov, &thermal_governor_list, governor_list) {
+		int ret;
+
 		ret = cb(gov, data);
 		if (ret)
-			break;
+			return ret;
 	}
-	mutex_unlock(&thermal_governor_lock);
 
-	return ret;
+	return 0;
 }
 
 int for_each_thermal_cooling_device(int (*cb)(struct thermal_cooling_device *,
@@ -1345,20 +1336,15 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
 static int thermal_zone_init_governor(struct thermal_zone_device *tz)
 {
 	struct thermal_governor *governor;
-	int ret;
 
-	mutex_lock(&thermal_governor_lock);
+	guard(mutex)(&thermal_governor_lock);
 
 	if (tz->tzp)
 		governor = __find_governor(tz->tzp->governor_name);
 	else
 		governor = def_governor;
 
-	ret = thermal_set_governor(tz, governor);
-
-	mutex_unlock(&thermal_governor_lock);
-
-	return ret;
+	return thermal_set_governor(tz, governor);
 }
 
 static void thermal_zone_init_complete(struct thermal_zone_device *tz)




^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-09-14 11:16 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-14 10:24 [RFC PATCH for 6.13 v1 00/20] thermal: core: Updates related to thermal zone initialization, suspend and locking Rafael J. Wysocki
2024-09-14 10:25 ` [RFC PATCH for 6.13 v1 01/20] thermal: core: Use the thermal zone guard in more cases Rafael J. Wysocki
2024-09-14 10:27 ` [RFC PATCH for 6.13 v1 02/20] thermal: core: Rearrange PM notification code Rafael J. Wysocki
2024-09-14 10:28 ` [RFC PATCH for 6.13 v1 03/20] thermal: core: Represent suspend-related thermal zone flags as bits Rafael J. Wysocki
2024-09-14 10:30 ` [RFC PATCH for 6.13 v1 04/20] thermal: core: Mark thermal zones as initializing to start with Rafael J. Wysocki
2024-09-14 10:31 ` [RFC PATCH for 6.13 v1 05/20] thermal: core: Fix race between zone registration and system suspend Rafael J. Wysocki
2024-09-14 10:32 ` [RFC PATCH for 6.13 v1 06/20] thermal: core: Consolidate thermal zone locking during initialization Rafael J. Wysocki
2024-09-14 10:34 ` [RFC PATCH for 6.13 v1 07/20] thermal: core: Mark thermal zones as exiting before unregistration Rafael J. Wysocki
2024-09-14 10:35 ` [RFC PATCH for 6.13 v1 08/20] thermal: core: Consolidate thermal zone locking in the exit path Rafael J. Wysocki
2024-09-14 10:36 ` [RFC PATCH for 6.13 v1 09/20] thermal: core: Update thermal zones after cooling device binding Rafael J. Wysocki
2024-09-14 10:37 ` [RFC PATCH for 6.13 v1 10/20] thermal: core: Drop need_update field from struct thermal_zone_device Rafael J. Wysocki
2024-09-14 10:38 ` [RFC PATCH for 6.13 v1 11/20] thermal: core: Separate code running under thermal_list_lock Rafael J. Wysocki
2024-09-14 10:40 ` [RFC PATCH for 6.13 v1 12/20] thermal: core: Manage thermal_list_lock using a mutex guard Rafael J. Wysocki
2024-09-14 10:41 ` [RFC PATCH for 6.13 v1 13/20] thermal: core: Call thermal_governor_update_tz() outside of cdev lock Rafael J. Wysocki
2024-09-14 10:43 ` [RFC PATCH for 6.13 v1 14/20] thermal: core: Introduce thermal_instance_add() Rafael J. Wysocki
2024-09-14 10:44 ` [RFC PATCH for 6.13 v1 15/20] thermal: core: Introduce thermal_instance_delete() Rafael J. Wysocki
2024-09-14 10:45 ` [RFC PATCH for 6.13 v1 16/20] thermal: core: Introduce thermal_cdev_update_nocheck() Rafael J. Wysocki
2024-09-14 10:47 ` [RFC PATCH for 6.13 v1 17/20] thermal: core: Add and use cooling device guard Rafael J. Wysocki
2024-09-14 10:48 ` [RFC PATCH for 6.13 v1 18/20] thermal: core: Call __thermal_cdev_update() on cdev unbind Rafael J. Wysocki
2024-09-14 10:49 ` [RFC PATCH for 6.13 v1 19/20] thermal: core: Separate thermal zone governor initialization Rafael J. Wysocki
2024-09-14 10:50 ` [RFC PATCH for 6.13 v1 20/20] thermal: core: Manage thermal_governor_lock using a mutex guard Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).