RE: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal devices
From: "Zhang, Rui" <rui.zhang@intel.com>
Date: 2020-04-27 14:20:27
Also in:
linux-acpi, linux-arm-kernel, linux-pm, platform-driver-x86
-----Original Message----- From: Zhang, Rui Sent: Friday, April 24, 2020 5:03 PM To: Andrzej Pietrasiewicz <redacted>; linux- pm@vger.kernel.org Cc: Rafael J . Wysocki <redacted>; Len Brown <lenb@kernel.org>; Jiri Pirko [off-list ref]; Ido Schimmel [off-list ref]; David S . Miller [off-list ref]; Peter Kaestle [off-list ref]; Darren Hart [off-list ref]; Andy Shevchenko [off-list ref]; Support Opensource [off-list ref]; Daniel Lezcano [off-list ref]; Amit Kucheria [off-list ref]; Shawn Guo [off-list ref]; Sascha Hauer [off-list ref]; Pengutronix Kernel Team [off-list ref]; Fabio Estevam [off-list ref]; NXP Linux Team [off-list ref]; Heiko Stuebner [off-list ref]; Orson Zhai [off-list ref]; Baolin Wang [off-list ref]; Chunyan Zhang [off-list ref]; linux- acpi@vger.kernel.org; netdev@vger.kernel.org; platform-driver- x86@vger.kernel.org; linux-arm-kernel@lists.infradead.org; kernel@collabora.com; Barlomiej Zolnierkiewicz [off-list ref] Subject: RE: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal devices Hi, Andrzej, Thanks for the patches. My Linux laptop was broken and won't get fixed till next week, so I may lost some of the discussions previously.quoted
-----Original Message----- From: Andrzej Pietrasiewicz <redacted> Sent: Friday, April 24, 2020 12:57 AM To: linux-pm@vger.kernel.org Cc: Zhang, Rui <rui.zhang@intel.com>; Rafael J . Wysocki [off-list ref]; Len Brown [off-list ref]; Jiri Pirko [off-list ref]; Ido Schimmel [off-list ref]; David S . Miller [off-list ref]; Peter Kaestle [off-list ref]; Darren Hart [off-list ref]; Andy Shevchenko [off-list ref]; Support Opensource [off-list ref]; Daniel Lezcano [off-list ref]; Amit Kucheria [off-list ref]; Shawn Guo [off-list ref];Saschaquoted
Hauer [off-list ref]; Pengutronix Kernel Team [off-list ref]; Fabio Estevam [off-list ref]; NXPLinuxquoted
Team [off-list ref]; Heiko Stuebner [off-list ref]; OrsonZhaiquoted
[off-list ref]; Baolin Wang [off-list ref];Chunyanquoted
Zhang [off-list ref]; linux- acpi@vger.kernel.org; netdev@vger.kernel.org; platform-driver- x86@vger.kernel.org; linux-arm-kernel@lists.infradead.org; kernel@collabora.com; Andrzej Pietrasiewicz [off-list ref]; Barlomiej Zolnierkiewicz [off-list ref] Subject: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal devices Importance: High Polling DISABLED devices is not desired, as all such "disabled" devices are meant to be handled by userspace. This patch introduces and uses should_stop_polling() to decide whether the device should be polled ornot.quoted
Thanks for the fix, and IMO, this reveal some more problems. Say, we need to define "DISABLED" thermal zone. Can we read the temperature? Can we trust the trip point value? IMO, a disabled thermal zone does not mean it is handled by userspace, because that is what the userspace governor designed for. Instead, if a thermal zone is disabled, in thermal_zone_device_update(), we should basically skip all the other operations as well.
I overlooked the last line of the patch. So thermal_zone_device_update() returns immediately if the thermal zone is disabled, right? But how can we stop polling in this case? There is no chance to call into monitor_thermal_zone() in thermal_zone_device_update(), or do I miss something?
I'll try your patches and probably make an incremental patch.
I have finished a small patch set to improve this based on my understanding, and will post it tomorrow after testing. Thanks, rui
quoted hunk ↗ jump to hunk
Thanks, ruiquoted
Signed-off-by: Andrzej Pietrasiewicz <redacted> --- drivers/thermal/thermal_core.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)diff --git a/drivers/thermal/thermal_core.cb/drivers/thermal/thermal_core.c index a2a5034f76e7..03c4d8d23284 100644--- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c@@ -305,13 +305,22 @@ static voidthermal_zone_device_set_polling(struct thermal_zone_device *tz, cancel_delayed_work(&tz->poll_queue); } +static inline bool should_stop_polling(struct thermal_zone_device +*tz) { + return thermal_zone_device_get_mode(tz) == THERMAL_DEVICE_DISABLED; } + static void monitor_thermal_zone(struct thermal_zone_device *tz) { + bool stop; + + stop = should_stop_polling(tz); + mutex_lock(&tz->lock); - if (tz->passive) + if (!stop && tz->passive) thermal_zone_device_set_polling(tz, tz->passive_delay); - else if (tz->polling_delay) + else if (!stop && tz->polling_delay) thermal_zone_device_set_polling(tz, tz->polling_delay); else thermal_zone_device_set_polling(tz, 0); @@ -503,6 +512,9@@ voidquoted
thermal_zone_device_update(struct thermal_zone_device *tz, { int count; + if (should_stop_polling(tz)) + return; + if (atomic_read(&in_suspend)) return; -- 2.17.1