linux-kernel - Re: [PATCH v3 5/7] drivers: devfreq: add longer polling interval in idle

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 linux-cve-announce 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:   Tue, 19 Feb 2019 09:33:46 +0100
From:   Lukasz Luba <l.luba@...tner.samsung.com>
To:     myungjoo.ham@...sung.com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Cc:     Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        "tkjos@...gle.com" <tkjos@...gle.com>,
        "joel@...lfernandes.org" <joel@...lfernandes.org>,
        "chris.diamand@....com" <chris.diamand@....com>,
        "mka@...omium.org" <mka@...omium.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "mingo@...hat.com" <mingo@...hat.com>
Subject: Re: [PATCH v3 5/7] drivers: devfreq: add longer polling interval in
 idle

Hi MyungJoo,

Thank you for taking part in the discussion.
Please check my comments below.

On 2/18/19 5:33 AM, MyungJoo Ham wrote:
>> This patch adds new mechanism for devfreq devices which changes polling
>> interval. The system should sleep longer when the devfreq device is almost
>> not used. The devfreq framework will not schedule the work too often.
>> This low-load state is recognised when the device is operating at the lowest
>> frequency and has low busy_time/total_time factor (< 30%).
>> When the frequency is different then min, the device is under normal polling
>> which is the value defined in driver's 'polling_ms'.
>> When the device is getting more pressure, the framework is able to catch it
>> based on 'load' in lowest frequency and will start polling more frequently.
>> The second scenario is when the governor recognised heavy load at minimum
>> frequency and increases the frequency. The devfreq framework will start
>> polling in shorter intervals.
>> The polling interval, when the device is not heavily, can also be changed
>> from userspace of defined by the driver's author.
>>
>> Signed-off-by: Lukasz Luba <l.luba@...tner.samsung.com>
>> ---
>>   drivers/devfreq/devfreq.c                 | 151 +++++++++++++++++++++++++++---
>>   drivers/devfreq/governor.h                |   3 +-
>>   drivers/devfreq/governor_simpleondemand.c |   6 +-
>>   3 files changed, 145 insertions(+), 15 deletions(-)
>>
> 
> There are some requirements that you need to consider:
> 
> Is 30% really applicable to ALL devfreq devices?
The 30% load while the device is on lowest OPP is to filter some noise.
It might be tunable over sysfs for each device if you like.
>      - What if some devices do not want such behaviors?
They can set polling_idle_ms and polling_ms the same value.
>      - What if some devices want different values (change behavors)?
Need of sysfs tunable here.
>      - What if some manufactures want different default values?
Like above (sysfs).
>      - What if some devices want to let the framework know that it's in idle?
There might be a filed in devfreq->state which could handle this.
>      - What if some other kernel context, device (drivers),
>      or userspace process want to notify that it's no more idling?This issue is more related to the new movement in the 'interconnect'
development. They have a goal for this kind of interactions and QoS
between devices or their clients. In devfreq it would be possible
to tackle this, but would require a lot of changes (notification chain,
state machines in devices,

> 
> As mentioned in the internal thread (tizen.org),
> I'm not convinced by the idea of assuming that a device can be considered "idling"
> if it has simply "low" utilization.
> 
> You are going to deteriorate the UI response time of mobile devices significantly.
Current devfreq wake-up also does not guarantee that, maybe on a single
CPU platform does.

I will try to address your and Chanwoo's comments that the devfreq still
needs deferred polling in some platforms.
Would it be OK if we have two options: deferred and delayed work while
registering a wakeup for a device?
That would be a function like: polling_mode_init(devfreq) instead of
simple INIT_DEFERRED_WORK(), which will check the device's preference.
The device driver could set a filed in 'polling_mode' to enum:
POWER_EFFICIENT or RELIABLE_INTERVAL. For compatibility with old drivers
where the polling_mode = 0, SYSTEM_DEFAULT_POLLING_MODE (which is one
of these two) would be used.
Then the two-phase-polling-interval from this patch could only be used
for the RELIABLE_INTERVAL configuration or even dropped.

Regards,
Lukasz
> 
> Cheers,
> MyungJoo.
> 
>