[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <744357E9AAD1214791ACBA4B0B90926377CFA057@SHSMSX108.ccr.corp.intel.com>
Date: Tue, 28 Apr 2020 13:55:19 +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
The patch is on top of this patch set.
Run into an issue during test today, will send out after the issue resolved.
Thanks,
rui
> -----Original Message-----
> From: linux-acpi-owner@...r.kernel.org <linux-acpi-owner@...r.kernel.org>
> On Behalf Of Andrzej Pietrasiewicz
> Sent: Tuesday, April 28, 2020 2:35 AM
> To: Zhang, Rui <rui.zhang@...el.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
> Importance: High
>
> Hi,
>
> W dniu 27.04.2020 o 16:20, Zhang, Rui pisze:
> >
> >
> >> -----Original Message-----
> >> From: Zhang, Rui
> >> Sent: Friday, April 24, 2020 5:03 PM
> >> To: Andrzej Pietrasiewicz <andrzej.p@...labora.com>; 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; netdev@...r.kernel.org; platform-driver-
> >> x86@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> >> 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 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?
>
> It does stop. However, I indeed observe an extra call to
> thermal_zone_device_update() before it fully stops.
> I think what happens is this:
>
> - storing "disabled" in mode ends up in thermal_zone_device_set_mode(),
> which calls driver's ->set_mode() and then calls
> thermal_zone_device_update(), which returns immediately and does not
> touch the tz->poll_queue delayed work
>
> - thermal_zone_device_update() is called from the delayed work when its
> time comes and this time it also returns immediately, not modifying the said
> delayed work, so polling effectively stops now.
>
> > There is no chance to call into monitor_thermal_zone() in
> > thermal_zone_device_update(), or do I miss something?
>
> Without the last "if" statement in this patch polling stops with the first call to
> thermal_zone_device_update() because it indeed disables the delayed work.
>
> So you are probably right - that last "if" should not be introduced.
>
> >
> >> 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.
> >
>
> Is your small patchset based on top of this series or is it a completely
> rewritten version?
>
> Andrzej
Powered by blists - more mailing lists