Message ID | 1880915.tdWV9SEqCh@kreacher |
---|---|
Headers | show |
Series | thermal: core: Remove thermal zones during unregistration | expand |
Hi Lukasz, On Mon, Dec 11, 2023 at 2:37 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > Hi Rafael, > > On 12/8/23 19:11, Rafael J. Wysocki wrote: > > Hi All, > > > > This patch series adds a mechanism to guarantee that > > thermal_zone_device_unregister() will not return until all of the active > > references to the thermal zone device object in question have been dropped > > and it has been deleted (patch [1/3]). > > > > This supersedes the approach used so far in which all thermal zone sysfs > > attribute callbacks check if the zone device is still registered under the > > zone lock, so as to return early if that is not the case, as it means that > > device_del() has been called for the thermal zone in question (and returned). > > It is not necessary to do that any more after patch [1/3], so patch [2/3] > > removes those checks from the code and drops zone locking that is not > > necessary any more either. > > > > Patch [3/3] uses the observation that the thermal subsystem does not need to > > check if a thermal zone device is registered at all, because it can use its > > own data to determine whether or not the thermal zone is going away and so > > it may not be worth updating it, for example. > > > > Please refer to the patch changelogs for details. > > > > The series depends on new thermal material in linux-next, but it should not > > substantially depend on any changes that have not made it into linux-next yet. > > > > Thanks! > > > > > > > > I like the concept with completion thing for this. > I have tired to stress test these patches with my mock > thermal zone module load/unload and it works good. > > The test was doing the these bits: > for i in $(seq 1 1000000) ; do cat > /sys/class/thermal/thermal_zone2/trip_point_0_temp > /dev/null 2>&1 ; done & > for i in $(seq 1 10000) ; do insmod /data/selftest_ipa.ko ; rmmod > selftest_ipa ; done & > > I couldn't trigger any issues in reading from this > trip temp file in background, which should go now w/o the > locking. I thought it would be nice test, since we have > direct call to trips array 'tz->trips[trip_id].temperature'. > Let me know if you think about other scenario for stress testing it. > (I have also checked the 'temp' sysfs read, where the mutex for > tz is used - also no issues). > > Feel free to add to all patches: > > Reviewed-and-tested-by: Lukasz Luba <lukasz.luba@arm.com> Thank you!