Re: [PATCH] irq: Exclude percpu IRQs from being fixed up

["Jan Beulich" <JBeulich@novell.com>]

>>> On 17.02.11 at 01:15, Fengzhe Zhang <fengzhe.zhang@intel.com> wrote:
> On 2011/2/16 22:53, Jan Beulich wrote:
>>>>> On 16.02.11 at 15:26, "Zhang, Fengzhe"<fengzhe.zhang@intel.com>  wrote:
>>> irq: Exclude percpu IRQs from being fixed up
>>>
>>> Xen spin unlock uses spurious ipi "lock_kicker_irq" to wake up blocked vCPUs
>>> waiting on that lock. This irq should always be disabled. However, when Dom0
>>> is shuting down, function fixup_irqs is called which unmasks all irqs.
>>> Function unmask_irq effectively re-enables lock_kicker_irq and its irq 
> handler
>>> is invoked which reports bug and crashes Dom0.
>>>
>>> This patch sets IRQ_PER_CPU flag in irq desc and excludes percpu IRQs from
>>> being fixed up when taking CPUs down.
>>>
>>> Signed-off-by: Fengzhe Zhang<fengzhe.zhang@intel.com>
>>>
>>> diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
>>> index 977d8b4..f0f9450 100644
>>> --- a/arch/x86/kernel/irq_64.c
>>> +++ b/arch/x86/kernel/irq_64.c
>>> @@ -80,6 +80,9 @@ void fixup_irqs(void)
>>>   		if (irq == 2)
>>>   			continue;
>>>
>>> +		if (desc->status&  IRQ_PER_CPU)
>>> +			continue;
>>> +
>>>   		/* interrupt's are disabled at this point */
>>>   		spin_lock(&desc->lock);
>>>
>>> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>>> index f34e231..26bc55a 100644
>>> --- a/kernel/irq/manage.c
>>> +++ b/kernel/irq/manage.c
>>> @@ -727,10 +727,9 @@ __setup_irq(unsigned int irq, struct irq_desc *desc,
>>> struct irqaction *new)
>>>   				goto out_thread;
>>>   		} else
>>>   			compat_irq_chip_set_default_handler(desc);
>>> -#if defined(CONFIG_IRQ_PER_CPU)
>>
>> Why? XEN should select IRQ_PER_CPU instead in its Kconfig.
>>
>> Jan
>>
> IRQ_PER_CPU switch is not found in current Kconfig. I'm not sure if this 

kernel/irq/Kconfig (introduced as a generic option in 2.6.38-rc2). In
prior kernel you'd have to add a respective Kconfig item in
drivers/xen/Kconfig.

> feature is going to be brought back in the short term. I remove the 
> ifdef to set IRQ_PER_CPU flag in desc by default but still leave the IRQ 
> handling logic unchanged. This is a temporary solution to fix system 
> crash on poweroff. And this is the fix with minimum impact among the 
> several solutions we tried.

But it's more a hack than a fix. And making per-CPU IRQs properly
treated as such isn't a bad idea in any case, I would say.

Jan