linux-kernel - Re: brocken devfreq simple

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]

Message-ID: <c3b4d74e-0e0e-590d-0588-3ff9756f8050@arm.com>
Date:   Wed, 24 Jun 2020 14:42:08 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Kamil Konieczny <k.konieczny@...sung.com>,
        Willy Wolff <willy.mh.wolff.ml@...il.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Kukjin Kim <kgene@...nel.org>, linux-pm@...r.kernel.org,
        "linux-samsung-soc@...r.kernel.org" 
        <linux-samsung-soc@...r.kernel.org>,
        linux-arm-kernel@...ts.infradead.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: brocken devfreq simple_ondemand for Odroid XU3/4?



On 6/24/20 2:13 PM, Krzysztof Kozlowski wrote:
> On Wed, Jun 24, 2020 at 02:03:03PM +0100, Lukasz Luba wrote:
>>
>>
>> On 6/24/20 1:06 PM, Krzysztof Kozlowski wrote:
>>> My case was clearly showing wrong behavior. System was idle but not
>>> sleeping - network working, SSH connection ongoing.  Therefore at least
>>> one CPU was not idle and could adjust the devfreq/DMC... but this did not
>>> happen. The system stayed for like a minute in 633 MHz OPP.
>>>
>>> Not-waking up idle processors - ok... so why not using power efficient
>>> workqueue? It is exactly for this purpose - wake up from time to time on
>>> whatever CPU to do the necessary job.
>>
>> IIRC I've done this experiment, still keeping in devfreq:
>> INIT_DEFERRABLE_WORK()
>> just applying patch [1]. It uses a system_wq which should
>> be the same as system_power_efficient_wq when
>> CONFIG_WQ_POWER_EFFICIENT_DEFAULT is not set (our case).
>> This wasn't solving the issue for the deferred work. That's
>> why the patch 2/2 following patch 1/2 [1] was needed.
>>
>> The deferred work uses TIMER_DEFERRABLE in it's initialization
>> and this is the problem. When the deferred work was queued on a CPU,
>> next that CPU went idle, the work was not migrated to some other CPU.
>> The former cpu is also not woken up according to the documentation [2].
> 
> Yes, you need either workqueue.power_efficient kernel param or CONFIG
> option to actually enable it.  But at least it could then work on any
> CPU.
> 
> Another solution is to use directly WQ_UNBOUND.
> 
>> That's why Kamil's approach should be continue IMHO. It gives more
>> control over important devices like: bus, dmc, gpu, which utilization
>> does not strictly correspond to cpu utilization (which might be low or
>> even 0 and cpu put into idle).
>>
>> I think Kamil was pointing out also some other issues not only dmc
>> (buses probably), but I realized too late to help him.
> 
> This should not be a configurable option. Why someone would prefer to
> use one over another and decide about this during build or run time?
> Instead it should be just *right* all the time. Always.

I had the same opinion, as you can see in my explanation to those
patches, but I failed. That's why I agree with Kamil's approach
because had higher chance to get into mainline and fix at least some
of the use cases.

> 
> Argument that we want to save power so we will not wake up any CPU is
> ridiculous if because of this system stays in high-power mode.
> 
> If system is idle and memory going to be idle, someone should be woken
> up to save more power and slow down memory controller.
> 
> If system is idle but memory going to be busy, the currently busy CPU
> (which performs some memory-intensive job) could do the job and ramp up
> the devfreq performance.

I agree. I think this devfreq mechanism was designed in the times
where there was/were 1 or 2 CPUs in the system. After a while we got ~8
and not all of them are used. This scenario was probably not
experimented widely on mainline platforms.

That is a good material for improvements, for someone who has time and
power.

Regards,
Lukasz

> 
> Best regards,
> Krzysztof
>