| Message ID | 2010397.PYKUYFuaPT@rjwysocki.net (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | thermal: core: Reimplement locking through guards | expand |
On 10/10/24 23:10, Rafael J. Wysocki wrote: > 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> > --- > > This is a resend of > > https://lore.kernel.org/linux-pm/2018087.usQuhbGJ8B@rjwysocki.net/ > > --- > drivers/thermal/thermal_core.c | 148 ++++++++++++++++++----------------------- > 1 file changed, 68 insertions(+), 80 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -140,7 +140,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) { > /* > @@ -163,7 +163,6 @@ int thermal_register_governor(struct the > } > } > > - mutex_unlock(&thermal_list_lock); > mutex_unlock(&thermal_governor_lock); > > return err; > @@ -181,7 +180,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, > @@ -189,8 +190,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); > } > @@ -686,51 +685,52 @@ 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) { > get_device(&tz->device); > - match = tz; > - break; > + return tz; > } > } > - mutex_unlock(&thermal_list_lock); > > - return match; > + return NULL; > } > > /* > @@ -969,14 +969,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); > } > > /** > @@ -1210,10 +1208,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 > @@ -1255,9 +1253,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); > > @@ -1281,24 +1276,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; > } > > /** > @@ -1345,7 +1334,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); > > @@ -1365,8 +1354,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); > } > > /** > @@ -1587,14 +1574,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); > > @@ -1605,10 +1589,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; > } > > /** > @@ -1660,24 +1641,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); > @@ -1718,6 +1698,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); > @@ -1736,35 +1728,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; > > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -140,7 +140,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) { /* @@ -163,7 +163,6 @@ int thermal_register_governor(struct the } } - mutex_unlock(&thermal_list_lock); mutex_unlock(&thermal_governor_lock); return err; @@ -181,7 +180,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, @@ -189,8 +190,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); } @@ -686,51 +685,52 @@ 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) { get_device(&tz->device); - match = tz; - break; + return tz; } } - mutex_unlock(&thermal_list_lock); - return match; + return NULL; } /* @@ -969,14 +969,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); } /** @@ -1210,10 +1208,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 @@ -1255,9 +1253,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); @@ -1281,24 +1276,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; } /** @@ -1345,7 +1334,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); @@ -1365,8 +1354,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); } /** @@ -1587,14 +1574,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); @@ -1605,10 +1589,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; } /** @@ -1660,24 +1641,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); @@ -1718,6 +1698,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); @@ -1736,35 +1728,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;