2004-09-25 11:47:42

by Zwane Mwaikambo

Subject: [email protected], [email protected]: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

Hello Kenji,

On Fri, 24 Sep 2004, Kenji Kaneshige wrote:

> + /*
> + * If interrupt handlers still exists on the irq
> + * associated with the gsi, don't unregister the
> + * interrupt.
> + */
> + if (unlikely(idesc->action)) {
> + iosapic_intr_info[vector].refcnt++;
> + spin_unlock_irqrestore(&idesc->lock, flags);
> + printk(KERN_WARNING "Cannot unregister GSI. IRQ %u is still in use.\n", irq);
> + return;
> + }
> +
> + /* Clear the interrupt controller descriptor. */
> + idesc->handler = &no_irq_type;

Hmm, what happens here if that vector was queued just before the local irq
disable in spin_lock_irqsave(idesc->lock...) ? Then when we unlock we'll
call do_IRQ to handle the irq associated with that vector. I haven't seen
the usage but it appears that iosapic_unregister_intr requires some
serialisation.

Thanks,
Zwane

2004-09-25 15:01:10

by Zwane Mwaikambo

Subject: Re: [email protected], [email protected]: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

On Sat, 25 Sep 2004, Zwane Mwaikambo wrote:

> Hmm, what happens here if that vector was queued just before the local irq
> disable in spin_lock_irqsave(idesc->lock...) ? Then when we unlock we'll
> call do_IRQ to handle the irq associated with that vector. I haven't seen
> the usage but it appears that iosapic_unregister_intr requires some
> serialisation.

Ignore this, i misread some of the code.

Thanks Kenji,
Zwane

2004-09-27 06:06:26

by Kenji Kaneshige

Subject: Re: [email protected], [email protected]: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

Zwane Mwaikambo wrote:

> On Sat, 25 Sep 2004, Zwane Mwaikambo wrote:
>
>> Hmm, what happens here if that vector was queued just before the local irq
>> disable in spin_lock_irqsave(idesc->lock...) ? Then when we unlock we'll
>> call do_IRQ to handle the irq associated with that vector. I haven't seen
>> the usage but it appears that iosapic_unregister_intr requires some
>> serialisation.
>
> Ignore this, i misread some of the code.
>
> Thanks Kenji,
> Zwane
>

OK.
BTW, I was able to find a bug thanks to your comment :-)
The following 'spin_lock(&iosapic_lock)' was missing. I'll update
the patch.

if (unlikely(idesc->action)) {
iosapic_intr_info[vector].refcnt++;
MISSING => spin_unlock(&iosapic_lock);
spin_unlock_irqrestore(&idesc->lock, flags);
printk(KERN_WARNING "Cannot unregister GSI. IRQ %u is still in use.\n", irq);
return;
}

Thanks,
Kenji Kaneshige