Jan Kiszka wrote:
> 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 <[email protected]>
> ---
>  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
> 

Oops, forgot to switch off line wrapping. You can pull from

git://git.kiszka.org/ipipe-2.6.git queues/2.6.31-x86

instead.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main

Reply via email to