linux-kernel - Re: [PATCH V2 4/5] idle: Move idle conditions in cpuidle

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: <530B97B9.8020502@linaro.org>
Date:	Mon, 24 Feb 2014 20:04:25 +0100
From:	Daniel Lezcano <daniel.lezcano@...aro.org>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	mingo@...nel.org, tglx@...utronix.de, rjw@...ysocki.net,
	nicolas.pitre@...aro.org, preeti@...ux.vnet.ibm.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 4/5] idle: Move idle conditions in cpuidle_idle main
 function

On 02/24/2014 06:22 PM, Peter Zijlstra wrote:
> On Mon, Feb 24, 2014 at 06:03:10PM +0100, Daniel Lezcano wrote:
>> Well there is the polling idle state for the x86 and ppc cpuidle drivers.
>> Except that, I think we have something more or less clean.
>
> Yeah, they have to set it back to polling again :/
>
> Ideally we'd sweep the entire tree and switch the default polling state
> to polling and add a current_clr_polling_and_test() to all WFI/HLT like
> ones that need the interrupt.
>
> But lots of work that.
>
>>> -	if (need_resched()) {
>>
>> Ok. The need_resched is now replaced by 'current_clr_polling_and_test', with
>> a call to '__current_set_polling()' to set the flag back, right ?
>
> Yah.
>
>> For my personal information, what is the subtlety with:
>>
>> if (tif_need_resched())
>> 	set_preempt_need_resched();
>>
>> ?
>
> Urgh, looks like something went wrong with: cf37b6b48428d
>
> That commit doesn't actually remove kernel/cpu/idle.c nor is the new
> code an exact replica of the old one.
>
> Ingo, any chance we can get that fixed?
>
> Daniel; does the below change/comment clarify?

Yes, I think so.

[ ... ]

> +
> +		/*
> +		 * Since we fell out of the loop above, we know
> +		 * TIF_NEED_RESCHED must be set, propagate it into
> +		 * PREEMPT_NEED_RESCHED.
> +		 *
> +		 * This is required because for polling idle loops we will
> +		 * not have had an IPI to fold the state for us.
> +		 */
> +		preempt_set_need_resched();
>   		tick_nohz_idle_exit();
>   		schedule_preempt_disabled();

So IIUC, the mainloop has two states: one where it is blocked on a 
HLT/WFI instruction (or about to enter/ exit this state) and another one 
outside of this blocking section.

When the idle task is blocked on HLT/WFI, it needs the IPI-reschedule in 
order to be woken up and rescheduled. But if it is outside this section, 
the idle task is not waiting for an interrupt and an expensive IPI can 
be saved by just setting the TS_POLLING flag, the scheduler will check 
this flag and won't send the IPI.

But 'set_preempt_need_resched' is called from the IPI handler. So if no 
IPI is sent because the idle task is in polling state, we have to set it 
ourself.

Now, the difference between the old code with 'tif_need_resched()' is 
because we don't need to check it because it is always true.

Am I right ?




-- 
  <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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/