RE: [PATCH v3 2/2] thermal: core: Stop polling DISABLED thermal devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux