Messages in this thread | | | Date | Mon, 11 Dec 2023 18:39:26 +0100 | Subject | Re: [PATCH v1 2/3] thermal: Drop redundant and confusing device_is_registered() checks | From | Daniel Lezcano <> |
| |
On 08/12/2023 20:19, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Multiple places in the thermal subsystem (most importantly, sysfs > attribute callback functions) check if the given thermal zone device is > still registered in order to return early in case the device_del() in > thermal_zone_device_unregister() has run already. > > However, after thermal_zone_device_unregister() has been made wait for > all of the zone-related activity to complete before returning, it is > not necessary to do that any more, because all of the code holding a > reference to the thermal zone device object will be waited for even if > it does not do anything special to enforce this. > > Accordingly, drop all of the device_is_registered() checks that are now > redundant and get rid of the zone locking that is not necessary any more > after dropping them. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > ---
[ ... ]
> @@ -132,11 +120,6 @@ trip_point_temp_store(struct device *dev > > mutex_lock(&tz->lock); > > - if (!device_is_registered(dev)) { > - ret = -ENODEV; > - goto unlock; > - } > - > trip = &tz->trips[trip_id]; > > if (temp != trip->temperature) { > @@ -162,23 +145,12 @@ trip_point_temp_show(struct device *dev, > char *buf) > { > struct thermal_zone_device *tz = to_thermal_zone(dev); > - int trip_id, temp; > + int trip_id; > > if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1) > return -EINVAL; > > - mutex_lock(&tz->lock); > - > - if (!device_is_registered(dev)) { > - mutex_unlock(&tz->lock); > - return -ENODEV; > - } > - > - temp = tz->trips[trip_id].temperature; > - > - mutex_unlock(&tz->lock); > - > - return sprintf(buf, "%d\n", temp); > + return sprintf(buf, "%d\n", tz->trips[trip_id].temperature);
Without the lock, could the trip_temp_store() make the value change while we read it?
[ ... ]
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |