On 2013-04-02 23:44, Gilles Chanteperdrix wrote:
> On 03/27/2013 02:30 PM, Jan Kiszka wrote:
> 
>> I'm wondering now why we need this differently for the I-pipe case.
>>
>> Let's revisit what happens with a fasteoi normally:
>>
>> - it's masked only if it's a oneshot IRQ before calling the flow handler
>> - it's unmasked after the flow handling if the thread was not woken up
>>
>> With I-pipe we already enter handle_fasteoi_irq with the IRQ masked. The
>> conditions and spots to unmask are:
>> - from handle_fasteoi_irq if the thread wasn't woken up or we have
>>   non-threaded or non-oneshot handling
>> - otherwise on end_irq from the handler thread
>>
>> Do you think this is correct? If so, I do not think it matches this
>> patch yet.
> 
> 
> Hi,
> 
> here is a much simpler patch:
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 11e75d1..a1c9918 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -421,6 +421,13 @@ static inline void preflow_handler(struct irq_desc *desc)
>  static inline void preflow_handler(struct irq_desc *desc) { }
>  #endif
>  
> +static void cond_release_fasteoi_irq(struct irq_desc *desc)
> +{
> +     if (desc->irq_data.chip->irq_release && 
> +         !irqd_irq_disabled(&desc->irq_data) && !desc->threads_oneshot)
> +             desc->irq_data.chip->irq_release(&desc->irq_data);
> +}
> +
>  /**
>   *   handle_fasteoi_irq - irq handler for transparent controllers
>   *   @irq:   the interrupt number
> @@ -463,8 +470,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc 
> *desc)
>  
>  #ifdef CONFIG_IPIPE
>       /* XXX: IRQCHIP_EOI_IF_HANDLED is ignored. */

Makes me wonder what this comment wants to tell us. That there is an
unhandled corner case or that this is intentionally ignored? I support
the latter as I-pipe already does EOI when accepting the IRQ, no? Maybe
you can clarify that line at this chance.

> -     if (desc->irq_data.chip->irq_release)
> -             desc->irq_data.chip->irq_release(&desc->irq_data);
> +     cond_release_fasteoi_irq(desc);
>  out_eoi:
>  #else  /* !CONFIG_IPIPE */
>       if (desc->istate & IRQS_ONESHOT)
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index e49a288..e7ae8bd 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -715,9 +715,14 @@ again:
>  
>       desc->threads_oneshot &= ~action->thread_mask;
>  
> +#ifndef CONFIG_IPIPE
>       if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
>           irqd_irq_masked(&desc->irq_data))
>               unmask_irq(desc);
> +#else /* CONFIG_IPIPE */
> +     if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data))
> +             desc->ipipe_end(desc->irq_data.irq, desc);
> +#endif /* CONFIG_IPIPE */
>  
>  out_unlock:
>       raw_spin_unlock_irq(&desc->lock);
> 
> 

Looks good. I should dig out our test case again and check it on x86. I
think it was PCI device assignment with KVM where the IRQ of the
assigned device is handled by an IRQ thread of KVM.

Jan


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: 
<http://www.xenomai.org/pipermail/xenomai/attachments/20130406/ef69ff66/attachment.pgp>
_______________________________________________
Xenomai mailing list
[email protected]
http://www.xenomai.org/mailman/listinfo/xenomai

Reply via email to