[Adeos-main] [PATCH] x86: Move ipipe_suspend_domain out of IRQ-disabled section

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 3461 bytes --]

Philippe Gerum wrote:
> On Wed, 2009-11-18 at 18:37 +0100, Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Philippe Gerum wrote:
>>>> On Wed, 2009-11-18 at 18:12 +0100, Jan Kiszka wrote:
>>>>> This fixes the valid complaint about safe_halt being called with the
>>>>> root domain unstalled.
>>>> The fix should go to the caller. ipipe_suspend_domain() acts as a
>>>> logical barrier: after that point, you may assume that the current
>>>> domain is unstalled.
>>> The caller so far expect to find no interruption window between return
>>> from ipipe_suspend_domain and yet another local_irq_disable. It expects
>>> to remain stalled all the time until safe_halt.
>> Checked again: Opening the IRQ window here is bogus, may cause
>> rescheduling delays to Linux (if not much worse things).
>>
>> I suppose it's better to adjust the assumption that ipipe_suspend_domain
>> behaves like "sti; hlt". Are there users that rely on this?
>> __ipipe_walk_pipeline does not look like it would.
> 
> I chose to never apply the mantra "never care for out of tree code" for
> Adeos, granted, at the expense of quite a lot of headaches, but that
> layer is a standalone building block which exports a stable API since
> years.
> 
> I may revert that decision in a foreseeable future, when X3 starts
> notably. But I'm still reluctant to break such a significant assumption
> in the current patch series.
> 
> I would rather move ipipe_suspend_domain() out of the atomic section on
> x86 (granted, this should be done carefully if ever possible).

Likely feasible. The good news is I missed the fact that there is
another check for needs_resched later on, right before the actual halt.
So the problem should be curable by moving ipipe_suspend_domain, at
least on x86.

Jan

-------->

x86: Move ipipe_suspend_domain out of IRQ-disabled section

ipipe_suspend_domain reenables IRQs on return, so we have to move it
before the point where the kernel disables it for halt.

This fixes the valid complaint about safe_halt being called with the
root domain unstalled.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 arch/x86/kernel/process_32.c |    3 ++-
 arch/x86/kernel/process_64.c |    4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index a8a5cd1..e31de9e 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -111,10 +111,11 @@ void cpu_idle(void)
 			if (cpu_is_offline(cpu))
 				play_dead();

+			ipipe_suspend_domain();
+
 			local_irq_disable();
 			/* Don't trace irqs off for idle */
 			stop_critical_timings();
-  			ipipe_suspend_domain();
 			pm_idle();
 			start_critical_timings();
 		}
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 02efd18..b30dc4d 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -136,6 +136,9 @@ void cpu_idle(void)

 			if (cpu_is_offline(smp_processor_id()))
 				play_dead();
+
+			ipipe_suspend_domain();
+
 			/*
 			 * Idle routines should keep interrupts disabled
 			 * from here on, until they go to idle.
@@ -145,7 +148,6 @@ void cpu_idle(void)
 			enter_idle();
 			/* Don't trace irqs off for idle */
 			stop_critical_timings();
-  			ipipe_suspend_domain();
 			pm_idle();
 			start_critical_timings();
 			/* In many cases the interrupt that ended idle


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]