acpi-devel@lists.sourceforge.net, linux-ia64@vger.kernel.orgRe: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

acpi-devel@lists.sourceforge.net, linux-ia64@vger.kernel.orgRe: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

[Zwane Mwaikambo]

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

Re: acpi-devel@lists.sourceforge.net, linux-ia64@vger.kernel.orgRe: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

[Zwane Mwaikambo]

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

Re: acpi-devel@lists.sourceforge.net, linux-ia64@vger.kernel.orgRe: [PATCH] Updated patches for PCI IRQ resource deallocation support [3/3]

[Kenji Kaneshige]

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