From cba00d16a2b0b0cfc8266124266ed23ab9eae918 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 11 Oct 2024 00:05:25 +0200 Subject: [PATCH] thermal: core: Add and use thermal zone guard Add and use a guard for thermal zone locking. This allows quite a few error code paths to be simplified among other things and brings in a noticeable code size reduction for a good measure. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Link: https://patch.msgid.link/1930069.tdWV9SEqCh@rjwysocki.net Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_core.c | 61 +++++++---------------- drivers/thermal/thermal_core.h | 4 ++ drivers/thermal/thermal_debugfs.c | 25 ++++++---- drivers/thermal/thermal_helpers.c | 17 +++---- drivers/thermal/thermal_hwmon.c | 5 +- drivers/thermal/thermal_netlink.c | 21 +++----- drivers/thermal/thermal_sysfs.c | 81 +++++++++++++------------------ drivers/thermal/thermal_trip.c | 8 +-- 8 files changed, 86 insertions(+), 136 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 4ea304e3ceb7..d1038d33f17e 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -202,16 +202,13 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz, int ret = -EINVAL; mutex_lock(&thermal_governor_lock); - mutex_lock(&tz->lock); + + guard(thermal_zone)(tz); gov = __find_governor(strim(policy)); - if (!gov) - goto exit; + if (gov) + ret = thermal_set_governor(tz, gov); - ret = thermal_set_governor(tz, gov); - -exit: - mutex_unlock(&tz->lock); mutex_unlock(&thermal_governor_lock); thermal_notify_tz_gov_change(tz, policy); @@ -617,26 +614,18 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz, { int ret; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); /* do nothing if mode isn't changing */ - if (mode == tz->mode) { - mutex_unlock(&tz->lock); - + if (mode == tz->mode) return 0; - } ret = __thermal_zone_device_set_mode(tz, mode); - if (ret) { - mutex_unlock(&tz->lock); - + if (ret) return ret; - } __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); - mutex_unlock(&tz->lock); - if (mode == THERMAL_DEVICE_ENABLED) thermal_notify_tz_enable(tz); else @@ -665,10 +654,10 @@ static bool thermal_zone_is_present(struct thermal_zone_device *tz) void thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); + if (thermal_zone_is_present(tz)) __thermal_zone_device_update(tz, event); - mutex_unlock(&tz->lock); } EXPORT_SYMBOL_GPL(thermal_zone_device_update); @@ -972,12 +961,10 @@ static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz, static void thermal_zone_cdev_bind(struct thermal_zone_device *tz, struct thermal_cooling_device *cdev) { - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); if (__thermal_zone_cdev_bind(tz, cdev)) __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); - - mutex_unlock(&tz->lock); } /** @@ -1284,11 +1271,9 @@ 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) { - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); __thermal_zone_cdev_unbind(tz, cdev); - - mutex_unlock(&tz->lock); } /** @@ -1334,7 +1319,7 @@ int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp) if (tz->ops.get_crit_temp) return tz->ops.get_crit_temp(tz, temp); - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); for_each_trip_desc(tz, td) { const struct thermal_trip *trip = &td->trip; @@ -1346,8 +1331,6 @@ int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp) } } - mutex_unlock(&tz->lock); - return ret; } EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp); @@ -1360,7 +1343,7 @@ static void thermal_zone_init_complete(struct thermal_zone_device *tz) list_add_tail(&tz->node, &thermal_tz_list); - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); /* Bind cooling devices for this zone. */ list_for_each_entry(cdev, &thermal_cdev_list, node) @@ -1377,8 +1360,6 @@ static void thermal_zone_init_complete(struct thermal_zone_device *tz) __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); - mutex_unlock(&tz->lock); - mutex_unlock(&thermal_list_lock); } @@ -1615,7 +1596,7 @@ static bool thermal_zone_exit(struct thermal_zone_device *tz) goto unlock; } - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); tz->state |= TZ_STATE_FLAG_EXIT; list_del_init(&tz->node); @@ -1624,8 +1605,6 @@ static bool thermal_zone_exit(struct thermal_zone_device *tz) list_for_each_entry(cdev, &thermal_cdev_list, node) __thermal_zone_cdev_unbind(tz, cdev); - mutex_unlock(&tz->lock); - unlock: mutex_unlock(&thermal_list_lock); @@ -1710,7 +1689,7 @@ static void thermal_zone_device_resume(struct work_struct *work) tz = container_of(work, struct thermal_zone_device, poll_queue.work); - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); tz->state &= ~(TZ_STATE_FLAG_SUSPENDED | TZ_STATE_FLAG_RESUMING); @@ -1720,13 +1699,11 @@ static void thermal_zone_device_resume(struct work_struct *work) __thermal_zone_device_update(tz, THERMAL_TZ_RESUME); complete(&tz->resume); - - mutex_unlock(&tz->lock); } static void thermal_zone_pm_prepare(struct thermal_zone_device *tz) { - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); if (tz->state & TZ_STATE_FLAG_RESUMING) { /* @@ -1742,13 +1719,11 @@ static void thermal_zone_pm_prepare(struct thermal_zone_device *tz) } tz->state |= TZ_STATE_FLAG_SUSPENDED; - - mutex_unlock(&tz->lock); } static void thermal_zone_pm_complete(struct thermal_zone_device *tz) { - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); cancel_delayed_work(&tz->poll_queue); @@ -1762,8 +1737,6 @@ static void thermal_zone_pm_complete(struct thermal_zone_device *tz) 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); } static int thermal_pm_notify(struct notifier_block *nb, diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 19a698bdbfba..09f796fb1278 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -9,6 +9,7 @@ #ifndef __THERMAL_CORE_H__ #define __THERMAL_CORE_H__ +#include #include #include @@ -146,6 +147,9 @@ struct thermal_zone_device { struct thermal_trip_desc trips[] __counted_by(num_trips); }; +DEFINE_GUARD(thermal_zone, struct thermal_zone_device *, mutex_lock(&_T->lock), + mutex_unlock(&_T->lock)) + /* Initial thermal zone temperature. */ #define THERMAL_TEMP_INIT INT_MIN diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c index 939d3e5f1817..d67021aeb136 100644 --- a/drivers/thermal/thermal_debugfs.c +++ b/drivers/thermal/thermal_debugfs.c @@ -885,6 +885,19 @@ void thermal_debug_tz_add(struct thermal_zone_device *tz) 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 thermal_zone_device *tz) 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; diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index 403d62d3ce77..f2baadd8f8da 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -60,13 +60,13 @@ bool thermal_trip_is_bound_to_cdev(struct thermal_zone_device *tz, { bool ret; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); + mutex_lock(&cdev->lock); ret = thermal_instance_present(tz, cdev, trip); mutex_unlock(&cdev->lock); - mutex_unlock(&tz->lock); return ret; } @@ -138,19 +138,14 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp) if (IS_ERR_OR_NULL(tz)) return -EINVAL; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); - if (!tz->ops.get_temp) { - ret = -EINVAL; - goto unlock; - } + if (!tz->ops.get_temp) + return -EINVAL; ret = __thermal_zone_get_temp(tz, temp); if (!ret && *temp <= THERMAL_TEMP_INVALID) - ret = -ENODATA; - -unlock: - mutex_unlock(&tz->lock); + return -ENODATA; return ret; } diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index f0e504fd866a..37da7a8ea948 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -78,12 +78,9 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) 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; diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c index f3c58c708969..91f3fe8c8f07 100644 --- a/drivers/thermal/thermal_netlink.c +++ b/drivers/thermal/thermal_netlink.c @@ -459,7 +459,7 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) 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; @@ -469,19 +469,12 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) 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) @@ -512,7 +505,7 @@ static int thermal_genl_cmd_tz_get_temp(struct param *p) static int thermal_genl_cmd_tz_get_gov(struct param *p) { struct sk_buff *msg = p->msg; - int id, ret = 0; + int id; if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID]) return -EINVAL; @@ -523,16 +516,14 @@ static int thermal_genl_cmd_tz_get_gov(struct param *p) 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; + return -EMSGSIZE; - mutex_unlock(&tz->lock); - - return ret; + return 0; } static int __thermal_genl_cmd_cdev_get(struct thermal_cooling_device *cdev, diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 1838aa729bb5..701607a953ff 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -50,13 +50,13 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - int enabled; - mutex_lock(&tz->lock); - enabled = tz->mode == THERMAL_DEVICE_ENABLED; - mutex_unlock(&tz->lock); + guard(thermal_zone)(tz); - return sprintf(buf, "%s\n", enabled ? "enabled" : "disabled"); + if (tz->mode == THERMAL_DEVICE_ENABLED) + return sprintf(buf, "enabled\n"); + + return sprintf(buf, "disabled\n"); } static ssize_t @@ -103,38 +103,34 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, { struct thermal_trip *trip = thermal_trip_of_attr(attr, temp); struct thermal_zone_device *tz = to_thermal_zone(dev); - int ret, temp; + int temp; - ret = kstrtoint(buf, 10, &temp); - if (ret) + if (kstrtoint(buf, 10, &temp)) return -EINVAL; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); if (temp == trip->temperature) - goto unlock; + return count; /* Arrange the condition to avoid integer overflows. */ if (temp != THERMAL_TEMP_INVALID && - temp <= trip->hysteresis + THERMAL_TEMP_INVALID) { - ret = -EINVAL; - goto unlock; - } + temp <= trip->hysteresis + THERMAL_TEMP_INVALID) + return -EINVAL; if (tz->ops.set_trip_temp) { + int ret; + ret = tz->ops.set_trip_temp(tz, trip, temp); if (ret) - goto unlock; + return ret; } thermal_zone_set_trip_temp(tz, trip, temp); __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); -unlock: - mutex_unlock(&tz->lock); - - return ret ? ret : count; + return count; } static ssize_t @@ -152,16 +148,15 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, { struct thermal_trip *trip = thermal_trip_of_attr(attr, hyst); struct thermal_zone_device *tz = to_thermal_zone(dev); - int ret, hyst; + int hyst; - ret = kstrtoint(buf, 10, &hyst); - if (ret || hyst < 0) + if (kstrtoint(buf, 10, &hyst) || hyst < 0) return -EINVAL; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); if (hyst == trip->hysteresis) - goto unlock; + return count; /* * Allow the hysteresis to be updated when the temperature is invalid @@ -171,22 +166,17 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, */ if (trip->temperature == THERMAL_TEMP_INVALID) { WRITE_ONCE(trip->hysteresis, hyst); - goto unlock; + return count; } - if (trip->temperature - hyst <= THERMAL_TEMP_INVALID) { - ret = -EINVAL; - goto unlock; - } + if (trip->temperature - hyst <= THERMAL_TEMP_INVALID) + return -EINVAL; thermal_zone_set_trip_hyst(tz, trip, hyst); __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); -unlock: - mutex_unlock(&tz->lock); - - return ret ? ret : count; + return count; } static ssize_t @@ -236,25 +226,26 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct thermal_zone_device *tz = to_thermal_zone(dev); - int ret = 0; int temperature; if (kstrtoint(buf, 10, &temperature)) return -EINVAL; - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); + + if (tz->ops.set_emul_temp) { + int ret; - if (!tz->ops.set_emul_temp) - tz->emul_temperature = temperature; - else ret = tz->ops.set_emul_temp(tz, temperature); + if (ret) + return ret; + } else { + tz->emul_temperature = temperature; + } - if (!ret) - __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); + __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); - mutex_unlock(&tz->lock); - - return ret ? ret : count; + return count; } static DEVICE_ATTR_WO(emul_temp); #endif @@ -894,13 +885,11 @@ ssize_t weight_store(struct device *dev, struct device_attribute *attr, instance = container_of(attr, struct thermal_instance, weight_attr); /* Don't race with governors using the 'weight' value */ - mutex_lock(&tz->lock); + guard(thermal_zone)(tz); instance->weight = weight; thermal_governor_update_tz(tz, THERMAL_INSTANCE_WEIGHT_CHANGED); - mutex_unlock(&tz->lock); - return count; } diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index b53fac333ec5..09474453baa5 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -45,13 +45,9 @@ int thermal_zone_for_each_trip(struct thermal_zone_device *tz, int (*cb)(struct thermal_trip *, void *), void *data) { - int ret; + guard(thermal_zone)(tz); - mutex_lock(&tz->lock); - ret = for_each_thermal_trip(tz, cb, data); - mutex_unlock(&tz->lock); - - return ret; + return for_each_thermal_trip(tz, cb, data); } EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);