Re: [Adeos-main] Fasteoi unmasking issue
On 29/11/11 23:25, Wolfgang Mauerer wrote:
> On 29/11/11 15:13, Philippe Gerum wrote:
>> On 11/28/2011 02:05 PM, Wolfgang Mauerer wrote:
>>> we are facing some difficulties with GSI interrupt storms
>>> originating from a PCI card that seem to be caused by
>>> ipipe: The card is passed through to qemu-kvm (the setup
>>> is based on the patches sent by Jan some time ago). Once
>>> the card becomes active, we are hit by a tremendous amount
>>> of interrupts (> 10/s) that keep ipipe fully occupied.
>>> The observed pattern is (excerpt from the ipipe tracer)
>>>
>>> :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62)
>>> :| __ipipe_handle_irq+0x11 (common_interrupt+0x27)
>>> (...)
>>> : handle_irq+0x9 (do_IRQ+0x66)
>>> : irq_to_desc+0x4 (handle_irq+0x15)
>>> : handle_fasteoi_irq+0x14 (handle_irq+0x22)
>>> (...)
>>> : unmask_ioapic_irq+0x4 (handle_fasteoi_irq+0x94)
>>> : unmask_ioapic+0xd (unmask_ioapic_irq+0x14)
>>> : __ipipe_spin_lock_irqsave+0x7 (unmask_ioapic+0x23)
>>> :| __ipipe_spin_lock_irqsave+0x93 (unmask_ioapic+0x23)
>>> :| __io_apic_modify_irq+0x4 (unmask_ioapic+0x41)
>>> :| __ipipe_unlock_irq+0x11 (unmask_ioapic+0x66)
>>> :| __ipipe_spin_unlock_irqrestore+0x9 (unmask_ioapic+0x75)
>>> :| __ipipe_spin_unlock_irqrestore+0x60 (unmask_ioapic+0x75)
>>> :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62)
>>>
>>> That is, as soon as the IRQ in question is unmasked, the
>>> next one is immediately received, and the interrupt handler
>>> in non-RT context never gets a chance to actually service
>>> the interrupt.
>>>
>>> The problem seems to be caused by unmasking the IRQ in
>>> handle_fasteoi_irq(), and with a hack along the lines of
>>>
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -586,7 +586,8 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc
>>> *desc)
>>> raw_spin_lock(&desc->lock);
>>> desc->status&= ~IRQ_INPROGRESS;
>>> #ifdef CONFIG_IPIPE
>>> - desc->irq_data.chip->irq_unmask(&desc->irq_data);
>>> + if (irq != WHICHEVER_IRQ_CAUSES_THE_STORM)
>>> + desc->irq_data.chip->irq_unmask(&desc->irq_data);
>>> out:
>>> #else
>>> out:
>>>
>>> the issue is solved.
>>>
>>> So the question is: Why is it okay to unconditionally unmask
>>> all interrupts in the fasteoi handler? All cards that re-send
>>> interrupts at high frequencies unless they are properly handled
>>> by their device driver should cause the same problem.
>>> I take the early unmasking is an optimisation, or are there any
>>> further reasons for the unconditional unmasking in
>>> handle_fasteoi_irq()?
>>
>> This is not an optimization, the flow for which this code was designed
>> for is:
>>
>> hw IRQ receipt
>> chip->eoi()
>> must mask the IRQ line
>> ...
>> real-time or Linux handling, clear device interrupt
>> ...
>> handle_fasteoi()
>> unmask previous masking
>>
>> It does not cope well with the recent threaded interrupt model addition
>> in the vanilla kernel. So it will likely break for any device with
>> threaded level IRQ handling.
> ah true, with non-threaded IRQ handlers, the unmasking happens
> after the handler is finished.
>
> Modulo any polishing etc., how about something like:
Too invasive ;) It turns out there's some Linux fasteoi
bug present in 2.6.38, patch follows in separate Mail. --Wolfgang
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index eca7fa8..e28745f 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -240,7 +240,8 @@ static inline void move_masked_irq(int irq) { }
> extern int no_irq_affinity;
>
> /* Handle irq action chains: */
> -extern irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction
> *action);
> +extern irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction
> *action,
> + short *threaded);
>
> /*
> * Built-in IRQ handlers for various IRQ types,
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 9671529..d36e327 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -479,7 +479,7 @@ handle_simple_irq(unsigned int irq, struct irq_desc *desc)
> desc->status |= IRQ_INPROGRESS;
> raw_spin_unlock(&desc->lock);
>
> - action_ret = handle_IRQ_event(irq, action);
> + action_ret = handle_IRQ_event(irq, action, NULL);
> if (!noirqdebug)
> note_interrupt(irq, desc, action_ret);
>
> @@ -526,7 +526,7 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
> desc->status |= IRQ_INPROGRESS;
> raw_spin_unlock(&desc->lock);
>
> - action_ret = handle_IRQ_event(irq, action);
> + action_ret = handle_IRQ_event(irq, action, NULL);
> if (!noirqdebug)
> note_interrupt(irq, desc, action_ret);
>
> @@ -555,6 +555,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc
> *desc)
> {
> struct irqaction *action;
> irqreturn_t action_ret;
> + short threaded = 0;
>
> raw_spin_lock(&desc->lock
Re: [Adeos-main] Fasteoi unmasking issue
On 29/11/11 15:13, Philippe Gerum wrote:
> On 11/28/2011 02:05 PM, Wolfgang Mauerer wrote:
>> we are facing some difficulties with GSI interrupt storms
>> originating from a PCI card that seem to be caused by
>> ipipe: The card is passed through to qemu-kvm (the setup
>> is based on the patches sent by Jan some time ago). Once
>> the card becomes active, we are hit by a tremendous amount
>> of interrupts (> 10/s) that keep ipipe fully occupied.
>> The observed pattern is (excerpt from the ipipe tracer)
>>
>> :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62)
>> :| __ipipe_handle_irq+0x11 (common_interrupt+0x27)
>> (...)
>> : handle_irq+0x9 (do_IRQ+0x66)
>> : irq_to_desc+0x4 (handle_irq+0x15)
>> : handle_fasteoi_irq+0x14 (handle_irq+0x22)
>> (...)
>> : unmask_ioapic_irq+0x4 (handle_fasteoi_irq+0x94)
>> : unmask_ioapic+0xd (unmask_ioapic_irq+0x14)
>> : __ipipe_spin_lock_irqsave+0x7 (unmask_ioapic+0x23)
>> :| __ipipe_spin_lock_irqsave+0x93 (unmask_ioapic+0x23)
>> :| __io_apic_modify_irq+0x4 (unmask_ioapic+0x41)
>> :| __ipipe_unlock_irq+0x11 (unmask_ioapic+0x66)
>> :| __ipipe_spin_unlock_irqrestore+0x9 (unmask_ioapic+0x75)
>> :| __ipipe_spin_unlock_irqrestore+0x60 (unmask_ioapic+0x75)
>> :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62)
>>
>> That is, as soon as the IRQ in question is unmasked, the
>> next one is immediately received, and the interrupt handler
>> in non-RT context never gets a chance to actually service
>> the interrupt.
>>
>> The problem seems to be caused by unmasking the IRQ in
>> handle_fasteoi_irq(), and with a hack along the lines of
>>
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -586,7 +586,8 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc
>> *desc)
>> raw_spin_lock(&desc->lock);
>> desc->status&= ~IRQ_INPROGRESS;
>> #ifdef CONFIG_IPIPE
>> - desc->irq_data.chip->irq_unmask(&desc->irq_data);
>> + if (irq != WHICHEVER_IRQ_CAUSES_THE_STORM)
>> + desc->irq_data.chip->irq_unmask(&desc->irq_data);
>> out:
>> #else
>> out:
>>
>> the issue is solved.
>>
>> So the question is: Why is it okay to unconditionally unmask
>> all interrupts in the fasteoi handler? All cards that re-send
>> interrupts at high frequencies unless they are properly handled
>> by their device driver should cause the same problem.
>> I take the early unmasking is an optimisation, or are there any
>> further reasons for the unconditional unmasking in
>> handle_fasteoi_irq()?
>
> This is not an optimization, the flow for which this code was designed
> for is:
>
> hw IRQ receipt
> chip->eoi()
> must mask the IRQ line
> ...
> real-time or Linux handling, clear device interrupt
> ...
> handle_fasteoi()
> unmask previous masking
>
> It does not cope well with the recent threaded interrupt model addition
> in the vanilla kernel. So it will likely break for any device with
> threaded level IRQ handling.
ah true, with non-threaded IRQ handlers, the unmasking happens
after the handler is finished.
Modulo any polishing etc., how about something like:
diff --git a/include/linux/irq.h b/include/linux/irq.h
index eca7fa8..e28745f 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -240,7 +240,8 @@ static inline void move_masked_irq(int irq) { }
extern int no_irq_affinity;
/* Handle irq action chains: */
-extern irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction
*action);
+extern irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action,
+ short *threaded);
/*
* Built-in IRQ handlers for various IRQ types,
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 9671529..d36e327 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -479,7 +479,7 @@ handle_simple_irq(unsigned int irq, struct irq_desc *desc)
desc->status |= IRQ_INPROGRESS;
raw_spin_unlock(&desc->lock);
- action_ret = handle_IRQ_event(irq, action);
+ action_ret = handle_IRQ_event(irq, action, NULL);
if (!noirqdebug)
note_interrupt(irq, desc, action_ret);
@@ -526,7 +526,7 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
desc->status |= IRQ_INPROGRESS;
raw_spin_unlock(&desc->lock);
- action_ret = handle_IRQ_event(irq, action);
+ action_ret = handle_IRQ_event(irq, action, NULL);
if (!noirqdebug)
note_interrupt(irq, desc, action_ret);
@@ -555,6 +555,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
{
struct irqaction *action;
irqreturn_t action_ret;
+ short threaded = 0;
raw_spin_lock(&desc->lock);
@@ -579,14 +580,15 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc
*desc)
desc->status &= ~IRQ_PENDING;
raw_spin_unlock(&desc->lock);
- action_ret = handle_IRQ_event(irq, action);
+ action_ret = handle_IRQ_event(irq, action, &threaded);
if (!noirqdebug)
Re: [Adeos-main] Fasteoi unmasking issue
On 11/28/2011 02:05 PM, Wolfgang Mauerer wrote: Dear all, we are facing some difficulties with GSI interrupt storms originating from a PCI card that seem to be caused by ipipe: The card is passed through to qemu-kvm (the setup is based on the patches sent by Jan some time ago). Once the card becomes active, we are hit by a tremendous amount of interrupts (> 10/s) that keep ipipe fully occupied. The observed pattern is (excerpt from the ipipe tracer) :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62) :| __ipipe_handle_irq+0x11 (common_interrupt+0x27) (...) : handle_irq+0x9 (do_IRQ+0x66) : irq_to_desc+0x4 (handle_irq+0x15) : handle_fasteoi_irq+0x14 (handle_irq+0x22) (...) : unmask_ioapic_irq+0x4 (handle_fasteoi_irq+0x94) : unmask_ioapic+0xd (unmask_ioapic_irq+0x14) : __ipipe_spin_lock_irqsave+0x7 (unmask_ioapic+0x23) :| __ipipe_spin_lock_irqsave+0x93 (unmask_ioapic+0x23) :| __io_apic_modify_irq+0x4 (unmask_ioapic+0x41) :| __ipipe_unlock_irq+0x11 (unmask_ioapic+0x66) :| __ipipe_spin_unlock_irqrestore+0x9 (unmask_ioapic+0x75) :| __ipipe_spin_unlock_irqrestore+0x60 (unmask_ioapic+0x75) :| common_interrupt+0x20 (__ipipe_spin_unlock_irqrestore+0x62) That is, as soon as the IRQ in question is unmasked, the next one is immediately received, and the interrupt handler in non-RT context never gets a chance to actually service the interrupt. The problem seems to be caused by unmasking the IRQ in handle_fasteoi_irq(), and with a hack along the lines of --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -586,7 +586,8 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc) raw_spin_lock(&desc->lock); desc->status&= ~IRQ_INPROGRESS; #ifdef CONFIG_IPIPE - desc->irq_data.chip->irq_unmask(&desc->irq_data); + if (irq != WHICHEVER_IRQ_CAUSES_THE_STORM) + desc->irq_data.chip->irq_unmask(&desc->irq_data); out: #else out: the issue is solved. So the question is: Why is it okay to unconditionally unmask all interrupts in the fasteoi handler? All cards that re-send interrupts at high frequencies unless they are properly handled by their device driver should cause the same problem. I take the early unmasking is an optimisation, or are there any further reasons for the unconditional unmasking in handle_fasteoi_irq()? This is not an optimization, the flow for which this code was designed for is: hw IRQ receipt chip->eoi() must mask the IRQ line ... real-time or Linux handling, clear device interrupt ... handle_fasteoi() unmask previous masking It does not cope well with the recent threaded interrupt model addition in the vanilla kernel. So it will likely break for any device with threaded level IRQ handling. Thanks& best regards, Wolfgang -- Siemens AG, Open Source Platforms, Corporate Competence Centre Embedded Linux -- Philippe. ___ Adeos-main mailing list [email protected] https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] Fasteoi unmasking issue
[ Sorry in advance, only have webmail access ATM. ] > The problem seems to be caused by unmasking the IRQ in > handle_fasteoi_irq(), and with a hack along the lines of > > --- a/kernel/irq/chip.c > +++ b/kernel/irq/chip.c > @@ -586,7 +586,8 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc > *desc) > raw_spin_lock(&desc->lock); > desc->status &= ~IRQ_INPROGRESS; > #ifdef CONFIG_IPIPE > - desc->irq_data.chip->irq_unmask(&desc->irq_data); > + if (irq != WHICHEVER_IRQ_CAUSES_THE_STORM) > + desc->irq_data.chip->irq_unmask(&desc->irq_data); > out: > #else > out: > > the issue is solved. > > So the question is: Why is it okay to unconditionally unmask > all interrupts in the fasteoi handler? All cards that re-send > interrupts at high frequencies unless they are properly handled > by their device driver should cause the same problem. > I take the early unmasking is an optimisation, or are there any > further reasons for the unconditional unmasking in > handle_fasteoi_irq()? > It think the proper fix is to check for IRQD_IRQ_MASKED and only unmask the line if it isn't masked from Linux' perspective. That should be a long pending i-pipe bug, surfaced by the IRQ thread mask flow that (not only) KVM's device assignment code depends on. Can you give this a try as well? Thanks for digging out the reason! Cheers from Schiphol, Jan ___ SMS schreiben mit WEB.DE FreeMail - einfach, schnell und kostenguenstig. Jetzt gleich testen! http://f.web.de/?mc=021192 ___ Adeos-main mailing list [email protected] https://mail.gna.org/listinfo/adeos-main
