RE: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal devices
From: "Zhang, Rui" <rui.zhang@intel.com>
Date: 2020-04-24 09:04:26
Also in:
linux-acpi, linux-arm-kernel, linux-pm, platform-driver-x86
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.
-----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]; 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; 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 or not.
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'll try your patches and probably make an incremental patch. Thanks, rui
quoted hunk ↗ jump to hunk
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 void thermal_zone_device_set_polling(structthermal_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@@ void 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