linux-kernel - Re: [RFC PATCH 2/2] sched: idle: IRQ based next prediction for idle period

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, 12 Jan 2016 17:04:36 +0100
From:	Daniel Lezcano <daniel.lezcano@...aro.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	peterz@...radead.org, rafael@...nel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, nicolas.pitre@...aro.org,
	vincent.guittot@...aro.org
Subject: Re: [RFC PATCH 2/2] sched: idle: IRQ based next prediction for idle
 period

On 01/12/2016 04:12 PM, Thomas Gleixner wrote:
> On Tue, 12 Jan 2016, Daniel Lezcano wrote:
>> On 01/12/2016 03:26 PM, Thomas Gleixner wrote:
>>> You better implement the switching part in the cpuidle core first, i.e.
>>> proper
>>> callbacks when a governor is switched in/out. Then make use of this
>>> switcheroo
>>> right away. Doing it the other way round is just wrong.
>>
>> The problem is this code is not another governor but a 'predictor' where the
>> scheduler will use the information to ask the cpuidle to go to a specific idle
>> state without going through the governor code, so into the governor's
>> callbacks. It is on top of cpuidle. The scheduler will become the governor.
>>
>> The current straightforward code, does the switch in the cpu_idle_loop
>> idle_task's function:
>>
>> [ ... ]
>>
>> if (cpu_idle_force_poll || tick_check_broadcast_expired())
>> 	cpu_idle_poll();
>> else {
>> 	if (sched_idle_enabled()) {
>> 		int latency = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
>> 		s64 duration = sched_idle_next_wakeup();
>> 		sched_idle(duration, latency);
>> 	} else {
>> 		cpuidle_idle_call();
>> 	}
>> }
>>
>> Due to the complexity of the code, this first step introduce a mechanism to
>> predict the next event and re-use it trivially in the idle task.
>
> This looks really wrong. Why on earth don't you implement a proper governor
> and just get rid of this extra hackery?

That is part of the ongoing work where we are integrating the different 
PM subsystems with the scheduler in order have them collaborating 
together as asked by Ingo [1].

The idea is to get rid of the governors and let the scheduler to tell 
the Cpuidle framework : "I expect to sleep <x> nsec and I have a <y>
nsec latency requirement" as stated by Peter Zijlstra [2].

The code above could be not nice but I think it begins to do what is 
expecting Peter. It is an embryonic code and in order to prevent too 
much different topics for a single series, I posted the two first 
patches which provide the next event API as the foundations for the next 
changes. How we integrate the 'next event' is a question I wanted to 
postpone in a different series of RFC patches.

   -- Daniel

[1] http://lwn.net/Articles/552885/
[2] https://lkml.org/lkml/2013/11/11/353 (broken here is another archive [3]
[3] http://lkml.iu.edu/hypermail/linux/kernel/1311.1/01360.html


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog