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@xxxxxxxxxxxxxxx <linux-acpi-owner@xxxxxxxxxxxxxxx> > On Behalf Of Andrzej Pietrasiewicz > Sent: Tuesday, April 28, 2020 2:35 AM > To: Zhang, Rui <rui.zhang@xxxxxxxxx>; 'linux-pm@xxxxxxxxxxxxxxx' <linux- > pm@xxxxxxxxxxxxxxx> > Cc: 'Rafael J . Wysocki' <rjw@xxxxxxxxxxxxx>; 'Len Brown' <lenb@xxxxxxxxxx>; > 'Jiri Pirko' <jiri@xxxxxxxxxxxx>; 'Ido Schimmel' <idosch@xxxxxxxxxxxx>; > 'David S . Miller' <davem@xxxxxxxxxxxxx>; 'Peter Kaestle' <peter@xxxxxxxx>; > 'Darren Hart' <dvhart@xxxxxxxxxxxxx>; 'Andy Shevchenko' > <andy@xxxxxxxxxxxxx>; 'Support Opensource' > <support.opensource@xxxxxxxxxxx>; 'Daniel Lezcano' > <daniel.lezcano@xxxxxxxxxx>; 'Amit Kucheria' > <amit.kucheria@xxxxxxxxxxxxx>; 'Shawn Guo' <shawnguo@xxxxxxxxxx>; > 'Sascha Hauer' <s.hauer@xxxxxxxxxxxxxx>; 'Pengutronix Kernel Team' > <kernel@xxxxxxxxxxxxxx>; 'Fabio Estevam' <festevam@xxxxxxxxx>; 'NXP > Linux Team' <linux-imx@xxxxxxx>; 'Heiko Stuebner' <heiko@xxxxxxxxx>; > 'Orson Zhai' <orsonzhai@xxxxxxxxx>; 'Baolin Wang' > <baolin.wang7@xxxxxxxxx>; 'Chunyan Zhang' <zhang.lyra@xxxxxxxxx>; > 'linux-acpi@xxxxxxxxxxxxxxx' <linux-acpi@xxxxxxxxxxxxxxx>; > 'netdev@xxxxxxxxxxxxxxx' <netdev@xxxxxxxxxxxxxxx>; 'platform-driver- > x86@xxxxxxxxxxxxxxx' <platform-driver-x86@xxxxxxxxxxxxxxx>; 'linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx' <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; > 'kernel@xxxxxxxxxxxxx' <kernel@xxxxxxxxxxxxx>; 'Barlomiej Zolnierkiewicz' > <b.zolnierkie@xxxxxxxxxxx> > 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@xxxxxxxxxxxxx>; linux- > >> pm@xxxxxxxxxxxxxxx > >> Cc: Rafael J . Wysocki <rjw@xxxxxxxxxxxxx>; Len Brown > >> <lenb@xxxxxxxxxx>; Jiri Pirko <jiri@xxxxxxxxxxxx>; Ido Schimmel > >> <idosch@xxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; > Peter > >> Kaestle <peter@xxxxxxxx>; Darren Hart <dvhart@xxxxxxxxxxxxx>; Andy > >> Shevchenko <andy@xxxxxxxxxxxxx>; Support Opensource > >> <support.opensource@xxxxxxxxxxx>; Daniel Lezcano > >> <daniel.lezcano@xxxxxxxxxx>; Amit Kucheria > >> <amit.kucheria@xxxxxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; > >> Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team > >> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; NXP > >> Linux Team <linux-imx@xxxxxxx>; Heiko Stuebner <heiko@xxxxxxxxx>; > >> Orson Zhai <orsonzhai@xxxxxxxxx>; Baolin Wang > >> <baolin.wang7@xxxxxxxxx>; Chunyan Zhang <zhang.lyra@xxxxxxxxx>; > >> linux- acpi@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; platform-driver- > >> x86@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > >> kernel@xxxxxxxxxxxxx; Barlomiej Zolnierkiewicz > >> <b.zolnierkie@xxxxxxxxxxx> > >> 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@xxxxxxxxxxxxx> > >>> Sent: Friday, April 24, 2020 12:57 AM > >>> To: linux-pm@xxxxxxxxxxxxxxx > >>> Cc: Zhang, Rui <rui.zhang@xxxxxxxxx>; Rafael J . Wysocki > >>> <rjw@xxxxxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>; Jiri Pirko > >>> <jiri@xxxxxxxxxxxx>; Ido Schimmel <idosch@xxxxxxxxxxxx>; David S . > >>> Miller <davem@xxxxxxxxxxxxx>; Peter Kaestle <peter@xxxxxxxx>; > Darren > >>> Hart <dvhart@xxxxxxxxxxxxx>; Andy Shevchenko <andy@xxxxxxxxxxxxx>; > >>> Support Opensource <support.opensource@xxxxxxxxxxx>; Daniel > Lezcano > >>> <daniel.lezcano@xxxxxxxxxx>; Amit Kucheria > >>> <amit.kucheria@xxxxxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; > >> Sascha > >>> Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team > >>> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; NXP > >> Linux > >>> Team <linux-imx@xxxxxxx>; Heiko Stuebner <heiko@xxxxxxxxx>; > Orson > >> Zhai > >>> <orsonzhai@xxxxxxxxx>; Baolin Wang <baolin.wang7@xxxxxxxxx>; > >> Chunyan > >>> Zhang <zhang.lyra@xxxxxxxxx>; linux- acpi@xxxxxxxxxxxxxxx; > >>> netdev@xxxxxxxxxxxxxxx; platform-driver- x86@xxxxxxxxxxxxxxx; > >>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > >>> kernel@xxxxxxxxxxxxx; Andrzej Pietrasiewicz > >>> <andrzej.p@xxxxxxxxxxxxx>; Barlomiej Zolnierkiewicz > >>> <b.zolnierkie@xxxxxxxxxxx> > >>> 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
