Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : nucleus: Fix interrupt handler tails

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

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

On 2011-06-20 22:52, Gilles Chanteperdrix wrote:
> On 06/20/2011 10:41 PM, Jan Kiszka wrote:
>> xnarch_switch_to is the central entry point for everyone. It may decide
>> to branch to switch_to or __switch_to, or it simply handles all on its
>> own - that's depending on the arch.
> 
> No, the Linux kernel does not know anything about xnarch_switch_to, so
> the schedule() function continues to use switch_to happily.
> xnarch_switch_to is only used to switch from xnthread_t to xnthread_t,
> by __xnpod_schedule(). Now, that some architecture (namely x86) decide
> that xnarch_switch_to should use switch_to (or more likely an inner
> __switch_to) when the xnthread_t has a non NULL user_task member is an
> implementation detail.
> 
>> Can you point out where in those paths irqs are disabled again (after
>> entering xnarch_switch_to)
> 
> They are not disabled again after xnarch_switch_to, they are disabled
> when starting switch_to.
> 
>  and left off for each of the unlocked
>> switching archs? I'm still skeptical that the need for disable irqs
>> during thread switch on some archs also leads to unconditionally
>> disabled hard irqs when returning from xnarch_switch_to.
>>
>> But even if that's all the case today, we would better set this
>> requirement in stone:
>>
>> diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
>> index f2fc2ab..c4c5807 100644
>> --- a/ksrc/nucleus/pod.c
>> +++ b/ksrc/nucleus/pod.c
>> @@ -2273,6 +2273,8 @@ reschedule:
>>
>>  	xnpod_switch_to(sched, prev, next);
>>
>> +	XENO_BUGON(NUCLEUS, !irqs_disabled_hw());
>> +
> 
> You misunderstand me: only after the second half context switch in the
> case of xnshadow_relax are the interrupts disabled. Because this second
> half-switch started as a switch_to and not an xnarch_switch_to, so,
> started as:
> 
> #define switch_to(prev,next,last)                                       \
> do {                                                                    \
>         local_irq_disable_hw_cond();                                    \
>         last = __switch_to(prev,task_thread_info(prev),
> task_thread_info(next));        \
>         local_irq_enable_hw_cond();                                     \
> } while (0)
> 
> (On ARM for instance).

OK, that's now clear, thanks.

> 
> But that is true, we could assert this in the shadow epilogue case.
> 

I've queued a patch.

Jan


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