lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 24 Apr 2020 09:03:29 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
CC:     "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, Jiri Pirko <jiri@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        "David S . Miller" <davem@...emloft.net>,
        Peter Kaestle <peter@...e.net>,
        Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        "Support Opensource" <support.opensource@...semi.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amit.kucheria@...durent.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Heiko Stuebner <heiko@...ech.de>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang7@...il.com>,
        Chunyan Zhang <zhang.lyra@...il.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "kernel@...labora.com" <kernel@...labora.com>,
        Barlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
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.

> -----Original Message-----
> From: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
> Sent: Friday, April 24, 2020 12:57 AM
> To: linux-pm@...r.kernel.org
> Cc: Zhang, Rui <rui.zhang@...el.com>; Rafael J . Wysocki
> <rjw@...ysocki.net>; Len Brown <lenb@...nel.org>; Jiri Pirko
> <jiri@...lanox.com>; Ido Schimmel <idosch@...lanox.com>; David S .
> Miller <davem@...emloft.net>; Peter Kaestle <peter@...e.net>; Darren
> Hart <dvhart@...radead.org>; Andy Shevchenko <andy@...radead.org>;
> Support Opensource <support.opensource@...semi.com>; Daniel Lezcano
> <daniel.lezcano@...aro.org>; Amit Kucheria
> <amit.kucheria@...durent.com>; Shawn Guo <shawnguo@...nel.org>;
> Sascha Hauer <s.hauer@...gutronix.de>; Pengutronix Kernel Team
> <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>; NXP
> Linux Team <linux-imx@....com>; Heiko Stuebner <heiko@...ech.de>;
> Orson Zhai <orsonzhai@...il.com>; Baolin Wang
> <baolin.wang7@...il.com>; Chunyan Zhang <zhang.lyra@...il.com>; linux-
> acpi@...r.kernel.org; netdev@...r.kernel.org; platform-driver-
> x86@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> kernel@...labora.com; Andrzej Pietrasiewicz <andrzej.p@...labora.com>;
> Barlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> 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

> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
> ---
>  drivers/thermal/thermal_core.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/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(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
> @@ 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

Powered by blists - more mailing lists