Messages in this thread | | | Date | Thu, 14 Feb 2019 08:59:47 -0800 | From | Matthias Kaehlcke <> | Subject | Re: [PATCH 1/4] PM / devfreq: Track overall load monitor state instead of 'stop_polling' |
| |
Hi Chanwoo,
On Thu, Feb 14, 2019 at 11:25:52PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > 2019년 2월 14일 (목) 오후 7:16, Matthias Kaehlcke <mka@chromium.org>님이 작성: > > > > The field ->stop_polling indicates whether load monitoring should be/is > > stopped, it is set in devfreq_monitor_suspend(). Change the variable to > > hold the general state of load monitoring (stopped, running, suspended). > > Besides improving readability of conditions involving the field and this > > prepares the terrain for moving some duplicated code from the governors > > into the devfreq core. > > > > Hold the devfreq lock in devfreq_monitor_start/stop() to ensure proper > > synchronization. > > IMHO, I'm not sure that there are any benefits changing > from 'stop_polling' to 'monitor_state'. I have no objections > if Myungjoo confirms it.
I agree that as an isolated change there isn't a clear benefit. However in the context of the series the change is needed to avoid resuming a load monitor that wasn't even started.
In case this series isn't accepted I'd still suggest to change the name from 'stop_polling' to 'suspended'. I read 'stop_polling' as a call for action, while 'suspended' is a state. IMO at least in some contexts conditions using a state is clearer.
Cheers
Matthias
| |