On Fri, Oct 19, 2012 at 3:41 AM, Chuansheng Liu
<chuansheng....@intel.com> wrote:
>
> When debugging our system issues related with __setup_vector_irq(),
> found there is a real wrong code that:
> for_each_active_irq(irq) {
> cfg = irq_get_chip_data(irq);
> if (!cfg)
> continue;
>
> These codes presume all allocated irqs are ioapic irqs, but it is not
> like that, in our system there are many GPIO interrupts also.
>
> When one irq is not ioapic type irq, the chip_data will not be the
> type of struct irq_cfg in most cases.
impossible !
where is the irq_desc coming from. ?
after alloc_irq_from, alloc_irq_cfg get called too.
>
> So in function __setup_vector_irq(), it will cause some strange issues,
> moreover, if I added some prints(cfg->...) inside it, it can always
> cause system panic.
>
> Here using the struct irq_chip->flags to help identify if the irq
> is ioapic type or not.
>
> Looked forward all codes with for_each_active_irq(), found there is
> a commit 6fd36ba02 indicates the similar case in print_IO_APICs().
that is for not printing wrong for MSI etc.
>
> Signed-off-by: liu chuansheng <chuansheng....@intel.com>
> ---
> arch/x86/kernel/apic/io_apic.c | 25 ++++++++++++++++++++++---
> 1 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index c265593..f0355e6 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -68,6 +68,18 @@
> #define for_each_irq_pin(entry, head) \
> for (entry = head; entry; entry = entry->next)
>
> +/* need more thoughts ... */
> +#define CHIP_FLAG_IOAPIC 0x1000
> +static inline bool is_ioapic_irq(int irq)
> +{
> + struct irq_chip *chip;
> + chip = irq_get_chip(irq);
> + if ((chip) && (chip->flags == CHIP_FLAG_IOAPIC))
> + return true;
> +
> + return false;
> +}
> +
> #ifdef CONFIG_IRQ_REMAP
> static void irq_remap_modify_chip_defaults(struct irq_chip *chip);
> static inline bool irq_remapped(struct irq_cfg *cfg)
> @@ -1238,6 +1250,9 @@ void __setup_vector_irq(int cpu)
> raw_spin_lock(&vector_lock);
> /* Mark the inuse vectors */
> for_each_active_irq(irq) {
> + if (!is_ioapic_irq(irq))
> + continue;
> +
> cfg = irq_get_chip_data(irq);
> if (!cfg)
> continue;
> @@ -1641,7 +1656,6 @@ __apicdebuginit(void) print_IO_APICs(void)
> int ioapic_idx;
> struct irq_cfg *cfg;
> unsigned int irq;
> - struct irq_chip *chip;
>
> printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
> for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
> @@ -1662,8 +1676,7 @@ __apicdebuginit(void) print_IO_APICs(void)
> for_each_active_irq(irq) {
> struct irq_pin_list *entry;
>
> - chip = irq_get_chip(irq);
> - if (chip != &ioapic_chip)
> + if (!is_ioapic_irq(irq))
> continue;
>
> cfg = irq_get_chip_data(irq);
if the irq is not using ioapic_chip such as msi_chip etc, that irq
should be skip.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
