sparse_irq aka dyn_irq

Quoted 40 lines · Andrew Morton

Quoted 2 lines · On Thu, 13 Nov 2008 12:16:56 -0800 Yinghai Lu <[email protected]> wrote:
From: Yinghai Lu <[email protected]>
Subject: sparseirq v13
My overall view on this is that it takes some of the kernel's most fragile and most problem-dense code and makes it much more complex, and by adding new configuration options it significantly worsens our testing coverage.
The patch is HHHHHUUUUUUUUUUGGGGGEEE! Did it really need to be a single megapatch?
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
I don't have a clue what all this does. I hope those who will work on this code are sufficiently familiar with it all to be able to maintain it when there are close to zero comments in some of our most tricky and problem-prone code.
Quoted 18 lines
...
arch/x86/Kconfig +15 -0
+config SPARSE_IRQ
+     bool "Support sparse irq numbering"
+     depends on PCI_MSI || HT_IRQ
+     default y
+     help
+       This enables support for sparse irq, esp for msi/msi-x. the irq
+       number will be bus/dev/fn + 12bit. You may need if you have lots of
+       cards supports msi-x installed.
+
+       If you don't know what to do here, say Y.
+
+config MOVE_IRQ_DESC
+     bool "Move irq desc when changing irq smp_affinity"
+     depends on SPARSE_IRQ && SMP
+     default y

Quoted 8 lines

arch/x86/Kconfig config X86_HAS_BOOT_CPU_ID · line 261
Quoted 4 lines
arch/x86/Kconfig +4 -0
+     help
+       This enables moving irq_desc to cpu/node that irq will use handled.
+
+       If you don't know what to do here, say Y.
Do these reeeealy have to exist? How are users to know which to choose? Which option will distros choose and why did we make them have to decide?

want to use that as marker, so later could split the patch to small ones by enabling by steps.

Quoted 4 lines

Quoted 4 lines
...

Quoted 12 lines

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 133
Quoted 9 lines
arch/x86/kernel/io_apic.c +9 -0
+{
+     struct irq_pin_list *pin;
+     int node;
+
+     if (cpu < 0)
+             cpu = smp_processor_id();
+     node = cpu_to_node(cpu);
+
+     pin = kzalloc_node(sizeof(*pin), GFP_KERNEL, node);
It's a bug to call smp_processor_id() from preemptible code and it's a bug to use GFP_KERNEL in non-preemptible code. How can this be?

the could should be executed in boot stage, only bsp is running, or interrupt conext
via irq_complete_move

Quoted 29 lines

Quoted 11 lines

arch/x86/kernel/io_apic.c +5 -1

+     printk(KERN_DEBUG "  alloc irq_2_pin on cpu %d node %d\n", cpu, node);
+
+     return pin;
+}
 
 
...
 
-static struct irq_cfg *irq_cfg_alloc(unsigned int irq)
+static struct irq_cfg *get_one_free_irq_cfg(int cpu)

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 220

Quoted 10 lines

arch/x86/kernel/io_apic.c +8 -1

 {
-     return irq_cfg(irq);
+     struct irq_cfg *cfg;
+     int node;
+
+     if (cpu < 0)
+             cpu = smp_processor_id();
+     node = cpu_to_node(cpu);
+
+     cfg = kzalloc_node(sizeof(*cfg), GFP_KERNEL, node);

See above.

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 224

Quoted 4 lines

arch/x86/kernel/io_apic.c +3 -0

+     printk(KERN_DEBUG "  alloc irq_cfg on cpu %d node %d\n", cpu, node);
+
+     return cfg;
 }

Quoted 8 lines

So all callers of this function must test the return value and if it is NULL, take appropriate action.
That is something which should have been documented in this function's interface description. Only that doesn't exist.
The one caller which I checked (arch_init_copy_chip_data()) fails to check for this and will oops.

will add one function to wrap it.

Quoted 19 lines

Quoted 6 lines

...

arch/x86/kernel/io_apic.c +2 -0

+static void set_extra_move_desc(struct irq_desc *desc, cpumask_t mask)
+{

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 324

Quoted 10 lines

arch/x86/kernel/io_apic.c +10 -0

+     struct irq_cfg *cfg = desc->chip_data;
+
+     if (!cfg->move_in_progress) {
+             /* it means that domain is not changed */
+             cpumask_t tmp;
+
+             cpus_and(tmp, desc->affinity, mask);
+             if (cpus_empty(tmp))
+                     cfg->move_desc_in_progress_in_same_domain = 1;
+     }

Aren't we trying to avoid on-stack cpumask_t's?

Quoted 2 lines

I'd have though that this one could be eliminated via the use of cpus_intersects()?

will change to that.

there some other in __assign_irq_vector...

Quoted 6 lines

Quoted 6 lines
}
+#endif
...

Quoted 8 lines

arch/x86/kernel/io_apic.c int timer_through_8259 __initdata; · line 2835
Quoted 6 lines
static struct irq_chip lapic_chip __read_mostly = {
.name = "local-APIC",
.mask = mask_lapic_irq,
arch/x86/kernel/io_apic.c +0 -0
@@ -2574,7 +2834,9 @@ int timer_through_8259 __initdata;
  */
 static inline void __init check_timer(void)
An inlined __init function makes little sense.

should be done other patch...?

Quoted 9 lines

Review static void dmar_msi_set_affinity(unsign · line 3591
Quoted 7 lines
...
(quoted diff) +0 -0
@@ -3306,10 +3590,13 @@ static void dmar_msi_set_affinity(unsign
      if (cpus_empty(tmp))
              return;
`tmp' is always a bad choice of identifier.

other patch?

Quoted 9 lines

...

arch/x86/mm/init_32.c +0 -0

@@ -66,6 +66,7 @@ static unsigned long __meminitdata table
 static unsigned long __meminitdata table_top;

Quoted 4 lines

arch/x86/mm/init_32.c static unsigned long __meminitdata table · line 69
Quoted 2 lines
arch/x86/mm/init_32.c +1 -0
 static int __initdata after_init_bootmem;
+int after_bootmem;
This isn't a very well-chosen identifier for an x86-specific global.

it is not used, will remove that.

Quoted 68 lines

Quoted 13 lines

...

arch/x86/mm/init_32.c +1 -0

@@ -98,6 +126,7 @@ int __ht_create_irq(struct pci_dev *dev,
      int max_irq;
      int pos;
      int irq;
+     unsigned int irq_want;
 
      pos = pci_find_ht_capability(dev, HT_CAPTYPE_IRQ);
      if (!pos)

arch/x86/mm/init_32.c int __ht_create_irq(struct pci_dev *dev, · line 162

Quoted 9 lines

arch/x86/mm/init_32.c +5 -0

@@ -125,7 +154,12 @@ int __ht_create_irq(struct pci_dev *dev,
      cfg->msg.address_lo = 0xffffffff;
      cfg->msg.address_hi = 0xffffffff;
 
+     irq_want = build_irq_for_pci_dev(dev);
+#ifdef CONFIG_SPARSE_IRQ
+     irq = create_irq_nr(irq_want + idx);
+#else
      irq = create_irq();
+#endif

irq_want is unused if CONFIG_SPARSE_IRQ=n.

Quoted 12 lines

arch/x86/mm/init_32.c +0 -0
      if (irq <= 0) {
              kfree(cfg);
Index: linux-2.6/drivers/pci/intr_remapping.c
===================================================================
drivers/pci/intr_remapping.c +1 -1
@@ -19,17 +19,73 @@ struct irq_2_iommu {
      u8  irte_mask;
 };
 
-static struct irq_2_iommu irq_2_iommuX[NR_IRQS];
+#ifdef CONFIG_SPARSE_IRQ

drivers/pci/intr_remapping.c struct irq_2_iommu { · line 32

Quoted 10 lines

drivers/pci/intr_remapping.c +10 -0

+static struct irq_2_iommu *get_one_free_irq_2_iommu(int cpu)
+{
+     struct irq_2_iommu *iommu;
+     int node;
+
+     if (cpu < 0)
+             cpu = smp_processor_id();
+     node = cpu_to_node(cpu);
+
+     iommu = kzalloc_node(sizeof(*iommu), GFP_KERNEL, node);

See above.

Quoted 18 lines

drivers/pci/intr_remapping.c +4 -0

+     printk(KERN_DEBUG "alloc irq_2_iommu on cpu %d node %d\n", cpu, node);
+
+     return iommu;
+}
 
 
...

fs/proc/stat.c +3 -0

@@ -27,6 +27,9 @@ static int show_stat(struct seq_file *p,
      u64 sum = 0;
      struct timespec boottime;
      unsigned int per_irq_sum;
+#ifdef CONFIG_GENERIC_HARDIRQS
+     struct irq_desc *desc;
+#endif

Quoted 12 lines

fs/proc/stat.c static int show_stat(struct seq_file *p, · line 50

Quoted 9 lines

fs/proc/stat.c +1 -2

      user = nice = system = idle = iowait =
              irq = softirq = steal = cputime64_zero;

@@ -44,10 +47,9 @@ static int show_stat(struct seq_file *p,
              softirq = cputime64_add(softirq, kstat_cpu(i).cpustat.softirq);
              steal = cputime64_add(steal, kstat_cpu(i).cpustat.steal);
              guest = cputime64_add(guest, kstat_cpu(i).cpustat.guest);
-
-             for_each_irq_nr(j)
+             for_each_irq_desc(j, desc) {

This won't compile if CONFIG_GENERIC_HARDIRQS=n, I suspect.

we have

#ifndef CONFIG_GENERIC_HARDIRQS
#include <asm/irq.h>

# define nr_irqs                NR_IRQS

# define for_each_irq_desc(irq, desc)           \
        for (irq = 0; irq < nr_irqs; irq++)
# define end_for_each_irq_desc()

Quoted 18 lines

fs/proc/stat.c static int show_stat(struct seq_file *p, · line 95

Quoted 12 lines

fs/proc/stat.c +2 -2

                      sum += kstat_irqs_cpu(j, i);
-
+             } end_for_each_irq_desc();
              sum += arch_irq_stat_cpu(i);
      }
      sum += arch_irq_stat();

@@ -90,14 +92,17 @@ static int show_stat(struct seq_file *p,
      seq_printf(p, "intr %llu", (unsigned long long)sum);
 
      /* sum again ? it could be updated? */
-     for_each_irq_nr(j) {
+     for_each_irq_desc(j, desc) {

ditto.

Quoted 3 lines

...

Quoted 10 lines

fs/proc/stat.c

Quoted 8 lines

fs/proc/stat.c +8 -0

+#define end_for_each_irq_desc()
+#endif
+
+#define desc_chip_ack(irq, descp) desc->chip->ack(irq)
+#define desc_chip_mask(irq, descp) desc->chip->mask(irq)
+#define desc_chip_mask_ack(irq, descp) desc->chip->mask_ack(irq)
+#define desc_chip_unmask(irq, descp) desc->chip->unmask(irq)
+#define desc_chip_eoi(irq, descp) desc->chip->eoi(irq)

Was it necessary to implement these as macros?

try to avoid bunch of #idef with different parameters that need to be passed.

Quoted 8 lines

...

fs/proc/stat.c +0 -0

@@ -49,6 +56,299 @@ void handle_bad_irq(unsigned int irq, st
 int nr_irqs = NR_IRQS;
 EXPORT_SYMBOL_GPL(nr_irqs);

Quoted 12 lines

fs/proc/stat.c void handle_bad_irq(unsigned int irq, st · line 65
Quoted 7 lines
fs/proc/stat.c +7 -0
+void __init __attribute__((weak)) arch_early_irq_init_work(void)
+{
+}
+
+#ifdef CONFIG_SPARSE_IRQ
+static struct irq_desc irq_desc_init = {
+     .irq        = -1U,
Plain old `-1' would be better here. It works in all cases and the reader doesn't need to go and check that this field really is an unsigned int and it won't need editing if that field gets changed to long

.
ok.

Quoted 36 lines

Quoted 15 lines

fs/proc/stat.c +14 -0

+     .status     = IRQ_DISABLED,
+     .chip       = &no_irq_chip,
+     .handle_irq = handle_bad_irq,
+     .depth      = 1,
+     .lock       = __SPIN_LOCK_UNLOCKED(irq_desc_init.lock),
+#ifdef CONFIG_SMP
+     .affinity   = CPU_MASK_ALL
+#endif
+};
+
+static void init_kstat_irqs(struct irq_desc *desc, int cpu, int nr)
+{
+     unsigned long bytes;
+     char *ptr;

fs/proc/stat.c void handle_bad_irq(unsigned int irq, st · line 89

Quoted 10 lines

fs/proc/stat.c +10 -0

+     int node;
+
+     /* Compute how many bytes we need per irq and allocate them */
+     bytes = nr * sizeof(unsigned int);
+
+     if (cpu < 0)
+             cpu = smp_processor_id();
+
+     node = cpu_to_node(cpu);
+     ptr = kzalloc_node(bytes, GFP_KERNEL, node);

See above.

Quoted 8 lines

fs/proc/stat.c +5 -0

+     printk(KERN_DEBUG "  alloc kstat_irqs on cpu %d node %d\n", cpu, node);
+
+     desc->kstat_irqs = (unsigned int *)ptr;
+}
+
 
...

Quoted 9 lines

fs/proc/stat.c
Quoted 6 lines
fs/proc/stat.c +6 -0
+#endif
+/*
+ * Protect the sparse_irqs_free freelist:
+ */
+static DEFINE_SPINLOCK(sparse_irq_lock);
+LIST_HEAD(sparse_irqs_head);
It's strange that the list is global and is accessed from other .c files, but the lock which protects it is static

.
only protect that when try to append the list. with rcu add tail.

Quoted 10 lines

fs/proc/stat.c

Quoted 8 lines

fs/proc/stat.c +7 -0

+/*
+ * The sparse irqs are in a hash-table as well, for fast lookup:
+ */
+#define SPARSEIRQHASH_BITS          (13 - 1)
+#define SPARSEIRQHASH_SIZE          (1UL << SPARSEIRQHASH_BITS)
+#define __sparseirqhashfn(key)      hash_long((unsigned long)key, SPARSEIRQHASH_BITS)
+#define sparseirqhashentry(key)     (sparseirqhash_table + __sparseirqhashfn((key)))

Why implement these via macros?

copied from sched.c

Quoted 13 lines

fs/proc/stat.c +12 -0

+static struct list_head sparseirqhash_table[SPARSEIRQHASH_SIZE];
+
+static struct irq_desc irq_desc_legacy[NR_IRQS_LEGACY] __cacheline_aligned_in_smp = {
+     [0 ... NR_IRQS_LEGACY-1] = {
+             .irq        = -1U,
+             .status     = IRQ_DISABLED,
+             .chip       = &no_irq_chip,
+             .handle_irq = handle_bad_irq,
+             .depth      = 1,
+             .lock       = __SPIN_LOCK_UNLOCKED(irq_desc_init.lock),
+#ifdef CONFIG_SMP
+             .affinity   = CPU_MASK_ALL

Quoted 9 lines

fs/proc/stat.c
Quoted 6 lines
fs/proc/stat.c +6 -0
+#endif
+     }
+};
+
+/* FIXME: use bootmem alloc ...*/
+static unsigned int kstat_irqs_legacy[NR_IRQS_LEGACY][NR_CPUS];
Do these need to be 32-bit? Maybe they'll fit in 16-bit, dunno.

struct irq_desc {
        unsigned int            irq;
#ifdef CONFIG_SPARSE_IRQ
        struct list_head        list;
        struct list_head        hash_entry;
        struct timer_rand_state *timer_rand_state;
        unsigned int            *kstat_irqs;

Quoted 4 lines

Quoted 1 line
+void __init early_irq_init_work(void)
The use of "_work" implies that this function is invoked by schedule_work(). But it isn't

ok, will remove it.

Quoted 52 lines

fs/proc/stat.c +48 -0

+{
+     struct irq_desc *desc;
+     int legacy_count;
+     int i;
+
+     /* init_work to init list for sparseirq */
+     for (i = 0; i < SPARSEIRQHASH_SIZE; i++)
+             INIT_LIST_HEAD(sparseirqhash_table + i);
+
+     desc = irq_desc_legacy;
+     legacy_count = ARRAY_SIZE(irq_desc_legacy);
+
+     for (i = 0; i < legacy_count; i++) {
+             struct list_head *hash_head;
+
+             hash_head = sparseirqhashentry(i);
+             desc[i].irq = i;
+             desc[i].kstat_irqs = kstat_irqs_legacy[i];
+             list_add_tail(&desc[i].hash_entry, hash_head);
+             list_add_tail(&desc[i].list, &sparse_irqs_head);
+     }
+
+     arch_early_irq_init_work();
+}
+
 
...
 
+struct irq_desc *irq_to_desc_alloc_cpu(unsigned int irq, int cpu)
+{
+     struct irq_desc *desc;
+     struct list_head *hash_head;
+     unsigned long flags;
+     int node;
+
+     desc = irq_to_desc(irq);
+     if (desc)
+             return desc;
+
+     hash_head = sparseirqhashentry(irq);
+
+     spin_lock_irqsave(&sparse_irq_lock, flags);
+
+     /*
+      * We have to do the hash-walk again, to avoid races
+      * with another CPU:
+      */
+     list_for_each_entry(desc, hash_head, hash_entry)
+             if (desc->irq == irq)
+                     goto out_unlock;
+

Quoted 8 lines

fs/proc/stat.c
Quoted 5 lines
fs/proc/stat.c +5 -0
+     if (cpu < 0)
+             cpu = smp_processor_id();
+
+     node = cpu_to_node(cpu);
+     desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);
Oh for gawd's sake. PLEASE read Documentation/SubmitChecklist. Carefully. We've already discussed this.

there are 13 errors with checkpatch scripts. seems all about macro definition.

Quoted 89 lines

You cannot do a GFP_KERNEL allocation under spin_lock_irqsave().

Quoted 43 lines

fs/proc/stat.c +43 -0

+     printk(KERN_DEBUG "  alloc irq_desc for %d aka %#x on cpu %d node %d\n",
+              irq, irq, cpu, node);
+     init_one_irq_desc(irq, desc, cpu);
+
+     /*
+      * We use RCU's safe list-add method to make
+      * parallel walking of the hash-list safe:
+      */
+     list_add_tail_rcu(&desc->hash_entry, hash_head);
+     /*
+      * Add it to the global list:
+      */
+     list_add_tail_rcu(&desc->list, &sparse_irqs_head);
+
+out_unlock:
+     spin_unlock_irqrestore(&sparse_irq_lock, flags);
+
+     return desc;
+}
+
+struct irq_desc *irq_to_desc_alloc(unsigned int irq)
+{
+     return irq_to_desc_alloc_cpu(irq, -1);
+}
+
+#ifdef CONFIG_MOVE_IRQ_DESC
+static struct irq_desc *__real_move_irq_desc(struct irq_desc *old_desc,
+                                             int cpu)
+{
+     struct irq_desc *desc;
+     unsigned int irq;
+     struct list_head *hash_head;
+     unsigned long flags;
+     int node;
+
+     irq = old_desc->irq;
+
+     hash_head = sparseirqhashentry(irq);
+
+     spin_lock_irqsave(&sparse_irq_lock, flags);
+     /*
+      * We have to do the hash-walk again, to avoid races
+      * with another CPU:

fs/proc/stat.c

Quoted 10 lines

fs/proc/stat.c +10 -0

+      */
+     list_for_each_entry(desc, hash_head, hash_entry)
+             if (desc->irq == irq && old_desc != desc)
+                     goto out_unlock;
+
+     if (cpu < 0)
+             cpu = smp_processor_id();
+
+     node = cpu_to_node(cpu);
+     desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);

Ditto.

Also, the return value from the memory allocation attempt is not checked.

Quoted 27 lines

fs/proc/stat.c +18 -0

+     printk(KERN_DEBUG "  move irq_desc for %d aka %#x to cpu %d node %d\n",
+              irq, irq, cpu, node);
+
+     init_copy_one_irq_desc(irq, old_desc, desc, cpu);
+
+     list_replace_rcu(&old_desc->hash_entry, &desc->hash_entry);
+     list_replace_rcu(&old_desc->list, &desc->list);
+
+     /* free the old one */
+     free_one_irq_desc(old_desc);
+     kfree(old_desc);
+
+out_unlock:
+     spin_unlock_irqrestore(&sparse_irq_lock, flags);
+
+     return desc;
+}
+
 
...

init/main.c +0 -0

@@ -542,6 +542,15 @@ void __init __weak thread_info_cache_ini
 {
 }

Quoted 10 lines

init/main.c void __init __weak thread_info_cache_ini · line 552
Quoted 8 lines
init/main.c +8 -0
+void __init __attribute__((weak)) arch_early_irq_init_work(void)
+{
+}
+
+void __init __attribute__((weak)) early_irq_init_work(void)
+{
+     arch_early_irq_init_work();
+}
Please use __weak

thanks for reviewing.

Ingo Molnar

2008-11-13 22:05 UTC #35 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 7 lines · Yinghai Lu

Quoted 7 lines

kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 248

Quoted 7 lines

kernel/irq/handle.c +7 -0

+     /*
+      * We have to do the hash-walk again, to avoid races
+      * with another CPU:
+      */
+     list_for_each_entry(desc, hash_head, hash_entry)
+             if (desc->irq == irq)
+                     goto out_unlock;

should be:

Quoted 4 lines

Quoted 4 lines
kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 273
Quoted 4 lines
kernel/irq/handle.c +4 -0
+     list_for_each_entry(desc, hash_head, hash_entry) {
+             if (desc->irq == irq)
+                     goto out_unlock;
+     }

and:

Quoted 6 lines

Quoted 6 lines
Quoted 1 line
+ desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);
Oh for gawd's sake. PLEASE read Documentation/SubmitChecklist. Carefully. We've already discussed this.
You cannot do a GFP_KERNEL allocation under spin_lock_irqsave().

yes.

Ingo

Andrew Morton

2008-11-13 22:16 UTC #36 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 32 lines · On Thu, 13 Nov 2008 14:01:06 -0800 "Yinghai Lu" <[email protected]> wrote:

Quoted 22 lines · Andrew Morton
Quoted 2 lines · On Thu, 13 Nov 2008 12:16:56 -0800 Yinghai Lu <[email protected]> wrote:
From: Yinghai Lu <[email protected]>
Subject: sparseirq v13
My overall view on this is that it takes some of the kernel's most fragile and most problem-dense code and makes it much more complex, and by adding new configuration options it significantly worsens our testing coverage.
The patch is HHHHHUUUUUUUUUUGGGGGEEE! Did it really need to be a single megapatch?
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
I don't have a clue what all this does. I hope those who will work on this code are sufficiently familiar with it all to be able to maintain it when there are close to zero comments in some of our most tricky and problem-prone code.
...
Quoted 4 lines
Quoted 1 line
+static unsigned int kstat_irqs_legacy[NR_IRQS_LEGACY][NR_CPUS];
Do these need to be 32-bit? Maybe they'll fit in 16-bit, dunno.

Quoted 7 lines

struct irq_desc {
        unsigned int            irq;
#ifdef CONFIG_SPARSE_IRQ
        struct list_head        list;
        struct list_head        hash_entry;
        struct timer_rand_state *timer_rand_state;
        unsigned int            *kstat_irqs;

That doesn't address my question.

The above array can be very large. Can we halve its size by using 16-bit quantities? Will this code ever encounter IRQ numbers larger than 65536?

Quoted 31 lines

Quoted 23 lines

kernel/irq/handle.c +23 -0

+struct irq_desc *irq_to_desc_alloc_cpu(unsigned int irq, int cpu)
+{
+     struct irq_desc *desc;
+     struct list_head *hash_head;
+     unsigned long flags;
+     int node;
+
+     desc = irq_to_desc(irq);
+     if (desc)
+             return desc;
+
+     hash_head = sparseirqhashentry(irq);
+
+     spin_lock_irqsave(&sparse_irq_lock, flags);
+
+     /*
+      * We have to do the hash-walk again, to avoid races
+      * with another CPU:
+      */
+     list_for_each_entry(desc, hash_head, hash_entry)
+             if (desc->irq == irq)
+                     goto out_unlock;
+

Quoted 8 lines

kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 254
Quoted 5 lines
kernel/irq/handle.c +5 -0
+     if (cpu < 0)
+             cpu = smp_processor_id();
+
+     node = cpu_to_node(cpu);
+     desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);
Oh for gawd's sake. PLEASE read Documentation/SubmitChecklist. Carefully. We've already discussed this.

Quoted 1 line

there are 13 errors with checkpatch scripts. seems all about macro definition.

This has nothing to do with checkpatch. Documentation/SubmitChecklist covers much more than that. In particular it descripbes various steps which should be taken when runtime testing new code subissions.

Quoted 2 lines

Quoted 2 lines
You cannot do a GFP_KERNEL allocation under spin_lock_irqsave().

Steps which would have detected this bug.

Yinghai Lu

author

2008-11-13 22:42 UTC #37 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 13 lines · Andrew Morton

Quoted 5 lines
...
Quoted 3 lines
Quoted 1 line
+static unsigned int kstat_irqs_legacy[NR_IRQS_LEGACY][NR_CPUS];
Do these need to be 32-bit? Maybe they'll fit in 16-bit, dunno.
Quoted 7 lines
struct irq_desc {
        unsigned int            irq;
#ifdef CONFIG_SPARSE_IRQ
        struct list_head        list;
        struct list_head        hash_entry;
        struct timer_rand_state *timer_rand_state;
        unsigned int            *kstat_irqs;

Quoted 6 lines

That doesn't address my question.
The above array can be very large. Can we halve its size by using 16-bit quantities? Will this code ever encounter IRQ numbers larger than 65536?

NR_CPUS=4096, NR_IRQS_LEGACY=16, and that array will be 256k bytes

later could change that alloc_bootmem, so NR_CPUS will be replaced to nr_cpu_ids

Andrew Morton

2008-11-13 22:59 UTC #38 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 17 lines · On Thu, 13 Nov 2008 14:41:36 -0800 Yinghai Lu <[email protected]> wrote:

Quoted 15 lines · Andrew Morton
Quoted 5 lines
...
Quoted 3 lines
Quoted 1 line
+static unsigned int kstat_irqs_legacy[NR_IRQS_LEGACY][NR_CPUS];
Do these need to be 32-bit? Maybe they'll fit in 16-bit, dunno.
Quoted 7 lines
struct irq_desc {
        unsigned int            irq;
#ifdef CONFIG_SPARSE_IRQ
        struct list_head        list;
        struct list_head        hash_entry;
        struct timer_rand_state *timer_rand_state;
        unsigned int            *kstat_irqs;
That doesn't address my question.

Quoted 8 lines

Quoted 4 lines
The above array can be very large. Can we halve its size by using 16-bit quantities? Will this code ever encounter IRQ numbers larger than 65536?
NR_CPUS=4096, NR_IRQS_LEGACY=16, and that array will be 256k bytes
later could change that alloc_bootmem, so NR_CPUS will be replaced to nr_cpu_ids

Do the entries in that array need to be 32-bit?

Mike Travis

2008-11-13 23:15 UTC #39 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 23 lines · Andrew Morton

Quoted 20 lines · On Thu, 13 Nov 2008 14:41:36 -0800 Yinghai Lu <[email protected]> wrote:
Quoted 18 lines · Andrew Morton
Quoted 5 lines
...
Quoted 3 lines
Quoted 1 line
+static unsigned int kstat_irqs_legacy[NR_IRQS_LEGACY][NR_CPUS];
Do these need to be 32-bit? Maybe they'll fit in 16-bit, dunno.
Quoted 7 lines
struct irq_desc {
        unsigned int            irq;
#ifdef CONFIG_SPARSE_IRQ
        struct list_head        list;
        struct list_head        hash_entry;
        struct timer_rand_state *timer_rand_state;
        unsigned int            *kstat_irqs;
That doesn't address my question.
The above array can be very large. Can we halve its size by using 16-bit quantities? Will this code ever encounter IRQ numbers larger than 65536?

Quoted 5 lines

Quoted 3 lines
NR_CPUS=4096, NR_IRQS_LEGACY=16, and that array will be 256k bytes
later could change that alloc_bootmem, so NR_CPUS will be replaced to nr_cpu_ids
Do the entries in that array need to be 32-bit?

That's a good question. If there are 4k or 16k cpus, would the number of irq's being "kstat'd" need to be > 64k? (I'm not exactly sure why there are NR_CPUS * NR_IRQS_LEGACY kstat entries per IRQ?)

Yinghai Lu

author

2008-11-13 23:25 UTC #40 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · Mike Travis

Quoted 3 lines · Andrew Morton
On Thu, 13 Nov 2008 14:41:36 -0800
Yinghai Lu <[email protected]> wrote:
That's a good question. If there are 4k or 16k cpus, would the number of irq's being "kstat'd" need to be > 64k? (I'm not exactly sure why there are NR_CPUS * NR_IRQS_LEGACY kstat entries per IRQ?)

can be replaced with nr_cpu_ids * NR_IRQS_LEGACY...

Mike Travis

2008-11-14 00:21 UTC #41 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 5 lines · Yinghai Lu

Quoted 4 lines · Mike Travis
Quoted 3 lines · Andrew Morton
On Thu, 13 Nov 2008 14:41:36 -0800
Yinghai Lu <[email protected]> wrote:

Quoted 7 lines

Quoted 3 lines
That's a good question. If there are 4k or 16k cpus, would the number of irq's being "kstat'd" need to be > 64k? (I'm not exactly sure why there are NR_CPUS * NR_IRQS_LEGACY kstat entries per IRQ?)
can be replaced with nr_cpu_ids * NR_IRQS_LEGACY...
YH

Ok, but why does each irq need a list of all other irq's in the system? Can they be collapsed into one array or percpu list?

Thanks,
Mike

Yinghai Lu

author

2008-11-14 00:30 UTC #42 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 9 lines · Mike Travis

Quoted 8 lines · Yinghai Lu
Quoted 7 lines · Mike Travis
Quoted 3 lines · Andrew Morton
On Thu, 13 Nov 2008 14:41:36 -0800
Yinghai Lu <[email protected]> wrote:
That's a good question. If there are 4k or 16k cpus, would the number of irq's being "kstat'd" need to be > 64k? (I'm not exactly sure why there are NR_CPUS * NR_IRQS_LEGACY kstat entries per IRQ?)

Quoted 6 lines

Quoted 3 lines
can be replaced with nr_cpu_ids * NR_IRQS_LEGACY...
YH
Ok, but why does each irq need a list of all other irq's in the system? Can they be collapsed into one array or percpu list?

use a big pointer array like struct irq_desc *desc[1<<32] ? that means 32g bytes... irq = bus/dev/fn/idx

or struct irq_desc *desc[NR_CPUS * NR_VECTORS] or per_cpu etc, it mean 4096*256*8 ... irq = cpu/idx

Yinghai Lu

author

2008-11-14 06:30 UTC #43 lore raw

PATCH 1/1 sparse_irq aka dyn_irq v14

address some Andrew's concerns.

also according to ingo, change _irqx to _irq_desc etc.

Thanks

Yinghai Lu

-----

From: Yinghai Lu <[email protected]>
Subject: sparseirq v14

impact: new feature sparseirq

add some kind of hash table as Ingo suggesting.
remove dyna_array

when sparse_irq is used (CONFIG_SPARSE_IRQ), use kzalloc_node to get irq_desc, irq_cfg
use desc->chip_data for x86 to store irq_cfg

for logical apic mode, need to add move_desc_in_progress_in_same_domain. otherwise it will not get moved. ==> also could need two phase to get irq_desc moved.

	for example: 0xff is old affinity, and need to set 0xf, and then set to 0xf0.
	[ or we need to change domain definition to cpus on the same node ? ]

Oct 27 12:35:34 LBSuse kernel: klogd 1.4.1, log source = /proc/kmsg started.
Oct 27 12:35:34 LBSuse kernel: eth0: no IPv6 routers present
LBSuse:~ # echo f0 > /proc/irq/22/smp_affinity
LBSuse:~ # tail /var/log/messages
Oct 27 12:35:34 LBSuse kernel: klogd 1.4.1, log source = /proc/kmsg started.
Oct 27 12:35:34 LBSuse kernel: eth0: no IPv6 routers present
Oct 27 12:36:46 LBSuse kernel:   move irq_desc for 22 aka 0x16 to cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc kstat_irqs on cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc irq_cfg on cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc irq_2_pin on cpu 7 node 1

so assume the user space program should update /proc/irq/XX/smp_affinity to 03 or 0f at first on boot or we change irq_default_affinity ?

for physical apic is much simple
on 4 sockets 16 cores system
irq_desc is moving..
when

# echo 10 > /proc/irq/134483967/smp_affinity
# echo 100 > /proc/irq/134483967/smp_affinity
# echo 1000 > /proc/irq/134483967/smp_affinity
got
Nov  9 21:39:51 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 4 node 1
Nov  9 21:39:51 LBSuse kernel:   alloc kstat_irqs on cpu 4 node 1
Nov  9 21:39:51 LBSuse kernel:   alloc irq_cfg on cpu 4 node 1
Nov  9 21:40:05 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 8 node 2
Nov  9 21:40:05 LBSuse kernel:   alloc kstat_irqs on cpu 8 node 2
Nov  9 21:40:05 LBSuse kernel:   alloc irq_cfg on cpu 8 node 2
Nov  9 21:40:18 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 12 node 3
Nov  9 21:40:18 LBSuse kernel:   alloc kstat_irqs on cpu 12 node 3
Nov  9 21:40:18 LBSuse kernel:   alloc irq_cfg on cpu 12 node 3

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/Kconfig                   |   20 
 arch/x86/include/asm/hpet.h        |    7 
 arch/x86/include/asm/irq_vectors.h |    2 
 arch/x86/kernel/hpet.c             |   23 -
 arch/x86/kernel/i8259.c            |   29 +
 arch/x86/kernel/io_apic.c          |  846 ++++++++++++++++++++++++++-----------
 arch/x86/kernel/irq.c              |   24 -
 arch/x86/kernel/irq_32.c           |    4 
 arch/x86/kernel/irq_64.c           |    8 
 arch/x86/kernel/irqinit_32.c       |    3 
 arch/x86/kernel/irqinit_64.c       |    3 
 arch/x86/kernel/uv_irq.c           |   27 +
 arch/x86/mm/init_32.c              |    3 
 drivers/char/random.c              |   31 +
 drivers/pci/htirq.c                |   44 +
 drivers/pci/intel-iommu.c          |   23 -
 drivers/pci/intr_remapping.c       |   60 ++
 drivers/pci/msi.c                  |   71 ++-
 drivers/xen/events.c               |    9 
 fs/proc/interrupts.c               |   18 
 fs/proc/stat.c                     |   17 
 include/linux/dmar.h               |    7 
 include/linux/htirq.h              |    8 
 include/linux/interrupt.h          |    2 
 include/linux/irq.h                |   95 +++-
 include/linux/irqnr.h              |   15 
 include/linux/kernel_stat.h        |   14 
 include/linux/msi.h                |   10 
 init/main.c                        |   11 
 kernel/irq/autoprobe.c             |   10 
 kernel/irq/chip.c                  |   40 -
 kernel/irq/handle.c                |  369 +++++++++++++++-
 kernel/irq/manage.c                |    6 
 kernel/irq/migration.c             |   34 +
 kernel/irq/proc.c                  |    3 
 kernel/irq/spurious.c              |    4 
 36 files changed, 1527 insertions(+), 373 deletions(-)

arch/x86/Kconfig +20 -0

Index: linux-2.6/arch/x86/Kconfig
===================================================================
@@ -240,6 +240,26 @@ config X86_HAS_BOOT_CPU_ID
 	def_bool y
 	depends on X86_VOYAGER
 
+config SPARSE_IRQ
+	bool "Support sparse irq numbering"
+	depends on PCI_MSI || HT_IRQ
+	default y
+	help
+	  This enables support for sparse irq, esp for msi/msi-x. the irq
+	  number will be bus/dev/fn + 12bit. You may need if you have lots of
+	  cards supports msi-x installed.
+
+	  If you don't know what to do here, say Y.
+
+config MOVE_IRQ_DESC
+	bool "Move irq desc when changing irq smp_affinity"
+	depends on SPARSE_IRQ && SMP
+	default y
+	help
+	  This enables moving irq_desc to cpu/node that irq will use handled.
+
+	  If you don't know what to do here, say Y.
+
 config X86_FIND_SMP_CONFIG
 	def_bool y
 	depends on X86_MPPARSE || X86_VOYAGER

arch/x86/kernel/io_apic.c +603 ~~-243~~

1,686 lines not on this page · raw message

arch/x86/kernel/irqinit_32.c +1 -2

Index: linux-2.6/arch/x86/kernel/irqinit_32.c
===================================================================
@@ -68,8 +68,7 @@ void __init init_ISA_irqs (void)
 	/*
 	 * 16 old-style INTA-cycle interrupts:
 	 */
-	for (i = 0; i < 16; i++) {
-		/* first time call this irq_desc */
+	for (i = 0; i < NR_IRQS_LEGACY; i++) {
 		struct irq_desc *desc = irq_to_desc(i);
 
 		desc->status = IRQ_DISABLED;

arch/x86/kernel/irqinit_64.c +1 -2

Index: linux-2.6/arch/x86/kernel/irqinit_64.c
===================================================================
@@ -142,8 +142,7 @@ void __init init_ISA_irqs(void)
 	init_bsp_APIC();
 	init_8259A(0);
 
-	for (i = 0; i < 16; i++) {
-		/* first time call this irq_desc */
+	for (i = 0; i < NR_IRQS_LEGACY; i++) {
 		struct irq_desc *desc = irq_to_desc(i);
 
 		desc->status = IRQ_DISABLED;

arch/x86/mm/init_32.c +3 -0

Index: linux-2.6/arch/x86/mm/init_32.c
===================================================================
@@ -66,6 +66,7 @@ static unsigned long __meminitdata table
 static unsigned long __meminitdata table_top;
 
 static int __initdata after_init_bootmem;
+int after_bootmem;
 
 static __init void *alloc_low_page(unsigned long *phys)
 {

@@ -987,6 +988,8 @@ void __init mem_init(void)
 
 	set_highmem_pages_init();
 
+	after_bootmem = 1;
+
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;
 	datasize =  (unsigned long) &_edata - (unsigned long) &_etext;
 	initsize =  (unsigned long) &__init_end - (unsigned long) &__init_begin;

drivers/char/random.c +31 -0

Index: linux-2.6/drivers/char/random.c
===================================================================
@@ -558,6 +558,8 @@ struct timer_rand_state {
 	unsigned dont_count_entropy:1;
 };
 
+#ifndef CONFIG_SPARSE_IRQ
+
 static struct timer_rand_state *irq_timer_state[NR_IRQS];
 
 static struct timer_rand_state *get_timer_rand_state(unsigned int irq)

@@ -576,6 +578,33 @@ static void set_timer_rand_state(unsigne
 	irq_timer_state[irq] = state;
 }
 
+#else
+
+static struct timer_rand_state *get_timer_rand_state(unsigned int irq)
+{
+	struct irq_desc *desc;
+
+	desc = irq_to_desc(irq);
+
+	if (!desc)
+		return NULL;
+
+	return desc->timer_rand_state;
+}
+
+static void set_timer_rand_state(unsigned int irq, struct timer_rand_state *state)
+{
+	struct irq_desc *desc;
+
+	desc = irq_to_desc(irq);
+
+	if (!desc)
+		return;
+
+	desc->timer_rand_state = state;
+}
+#endif
+
 static struct timer_rand_state input_timer_state;
 
 /*

@@ -933,8 +962,10 @@ void rand_initialize_irq(int irq)
 {
 	struct timer_rand_state *state;
 
+#ifndef CONFIG_SPARSE_IRQ
 	if (irq >= nr_irqs)
 		return;
+#endif
 
 	state = get_timer_rand_state(irq);

drivers/pci/htirq.c +40 -4

77 lines not on this page · raw message

drivers/pci/intr_remapping.c +58 -2

84 lines not on this page · raw message

drivers/xen/events.c +5 -4

Index: linux-2.6/drivers/xen/events.c
===================================================================
@@ -141,8 +141,9 @@ static void init_evtchn_cpu_bindings(voi
 	int i;
 
 	/* By default all event channels notify CPU#0. */
-	for_each_irq_desc(i, desc)
+	for_each_irq_desc(i, desc) {
 		desc->affinity = cpumask_of_cpu(0);
+	} end_for_each_irq_desc();
 #endif
 
 	memset(cpu_evtchn, 0, sizeof(cpu_evtchn));

@@ -231,7 +232,7 @@ static int find_unbound_irq(void)
 	int irq;
 
 	/* Only allocate from dynirq range */
-	for_each_irq_nr(irq)
+	for (irq = 0; irq < nr_irqs; irq++)
 		if (irq_bindcount[irq] == 0)
 			break;

@@ -792,7 +793,7 @@ void xen_irq_resume(void)
 		mask_evtchn(evtchn);
 
 	/* No IRQ <-> event-channel mappings. */
-	for_each_irq_nr(irq)
+	for (irq = 0; irq < nr_irqs; irq++)
 		irq_info[irq].evtchn = 0; /* zap event-channel binding */
 
 	for (evtchn = 0; evtchn < NR_EVENT_CHANNELS; evtchn++)

@@ -824,7 +825,7 @@ void __init xen_init_IRQ(void)
 		mask_evtchn(i);
 
 	/* Dynamic IRQ space is currently unbound. Zero the refcnts. */
-	for_each_irq_nr(i)
+	for (i = 0; i < nr_irqs; i++)
 		irq_bindcount[i] = 0;
 
 	irq_ctx_init(smp_processor_id());

fs/proc/stat.c +11 -6

Index: linux-2.6/fs/proc/stat.c
===================================================================
@@ -27,6 +27,9 @@ static int show_stat(struct seq_file *p,
 	u64 sum = 0;
 	struct timespec boottime;
 	unsigned int per_irq_sum;
+#ifdef CONFIG_GENERIC_HARDIRQS
+	struct irq_desc *desc;
+#endif
 
 	user = nice = system = idle = iowait =
 		irq = softirq = steal = cputime64_zero;

@@ -44,10 +47,9 @@ static int show_stat(struct seq_file *p,
 		softirq = cputime64_add(softirq, kstat_cpu(i).cpustat.softirq);
 		steal = cputime64_add(steal, kstat_cpu(i).cpustat.steal);
 		guest = cputime64_add(guest, kstat_cpu(i).cpustat.guest);
-
-		for_each_irq_nr(j)
+		for_each_irq_desc(j, desc) {
 			sum += kstat_irqs_cpu(j, i);
-
+		} end_for_each_irq_desc();
 		sum += arch_irq_stat_cpu(i);
 	}
 	sum += arch_irq_stat();

@@ -90,14 +92,17 @@ static int show_stat(struct seq_file *p,
 	seq_printf(p, "intr %llu", (unsigned long long)sum);
 
 	/* sum again ? it could be updated? */
-	for_each_irq_nr(j) {
+	for_each_irq_desc(j, desc) {
 		per_irq_sum = 0;
-
 		for_each_possible_cpu(i)
 			per_irq_sum += kstat_irqs_cpu(j, i);
 
+#ifdef CONFIG_SPARSE_IRQ
+		seq_printf(p, " %#x:%u", j, per_irq_sum);
+#else
 		seq_printf(p, " %u", per_irq_sum);
-	}
+#endif
+	} end_for_each_irq_desc();
 
 	seq_printf(p,
 		"\nctxt %llu\n"

fs/proc/interrupts.c +18 -0

Index: linux-2.6/fs/proc/interrupts.c
===================================================================
@@ -8,6 +8,23 @@
 /*
  * /proc/interrupts
  */
+#ifdef CONFIG_SPARSE_IRQ
+static void *int_seq_start(struct seq_file *f, loff_t *pos)
+{
+	rcu_read_lock();
+	return seq_list_start(&sparse_irqs_head, *pos);
+}
+
+static void *int_seq_next(struct seq_file *f, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &sparse_irqs_head, pos);
+}
+
+static void int_seq_stop(struct seq_file *f, void *v)
+{
+	rcu_read_unlock();
+}
+#else
 static void *int_seq_start(struct seq_file *f, loff_t *pos)
 {
 	return (*pos <= nr_irqs) ? pos : NULL;

@@ -25,6 +42,7 @@ static void int_seq_stop(struct seq_file
 {
 	/* Nothing to do */
 }
+#endif
 
 static const struct seq_operations int_seq_ops = {
 	.start = int_seq_start,

include/linux/interrupt.h +2 -0

Index: linux-2.6/include/linux/interrupt.h
===================================================================
@@ -18,6 +18,8 @@
 #include <asm/ptrace.h>
 #include <asm/system.h>
 
+extern int nr_irqs;
+
 /*
  * These correspond to the IORESOURCE_IRQ_* defines in
  * linux/ioport.h to select the interrupt line behaviour.  When

include/linux/irq.h +90 -5

143 lines not on this page · raw message

include/linux/kernel_stat.h +13 -1

Index: linux-2.6/include/linux/kernel_stat.h
===================================================================
@@ -28,7 +28,9 @@ struct cpu_usage_stat {
 
 struct kernel_stat {
 	struct cpu_usage_stat	cpustat;
-	unsigned int irqs[NR_IRQS];
+#ifndef CONFIG_SPARSE_IRQ
+       unsigned int irqs[NR_IRQS];
+#endif
 };
 
 DECLARE_PER_CPU(struct kernel_stat, kstat);

@@ -39,6 +41,10 @@ DECLARE_PER_CPU(struct kernel_stat, ksta
 
 extern unsigned long long nr_context_switches(void);
 
+#ifndef CONFIG_SPARSE_IRQ
+#define kstat_irqs_this_cpu(irq) \
+	(kstat_this_cpu.irqs[irq])
+
 struct irq_desc;
 
 static inline void kstat_incr_irqs_this_cpu(unsigned int irq,

@@ -46,11 +52,17 @@ static inline void kstat_incr_irqs_this_
 {
 	kstat_this_cpu.irqs[irq]++;
 }
+#endif
+
 
+#ifndef CONFIG_SPARSE_IRQ
 static inline unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
 {
        return kstat_cpu(cpu).irqs[irq];
 }
+#else
+extern unsigned int kstat_irqs_cpu(unsigned int irq, int cpu);
+#endif
 
 /*
  * Number of interrupts per specific IRQ source, since bootup

kernel/irq/autoprobe.c +5 -5

Index: linux-2.6/kernel/irq/autoprobe.c
===================================================================
@@ -57,7 +57,7 @@ unsigned long probe_irq_on(void)
 			desc->chip->startup(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	/* Wait for longstanding interrupts to trigger. */
 	msleep(20);

@@ -75,7 +75,7 @@ unsigned long probe_irq_on(void)
 				desc->status |= IRQ_PENDING;
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	/*
 	 * Wait for spurious interrupts to trigger

@@ -99,7 +99,7 @@ unsigned long probe_irq_on(void)
 					mask |= 1 << i;
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	return mask;
 }

@@ -135,7 +135,7 @@ unsigned int probe_irq_mask(unsigned lon
 			desc->chip->shutdown(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 	mutex_unlock(&probing_active);
 
 	return mask & val;

@@ -179,7 +179,7 @@ int probe_irq_off(unsigned long val)
 			desc->chip->shutdown(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 	mutex_unlock(&probing_active);
 
 	if (nr_of_irqs > 1)

kernel/irq/chip.c +22 ~~-18~~

Index: linux-2.6/kernel/irq/chip.c
===================================================================
@@ -24,9 +24,11 @@
  */
 void dynamic_irq_init(unsigned int irq)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_desc *desc;
 	unsigned long flags;
 
+	/* first time to use this irq_desc */
+	desc = irq_to_desc_alloc(irq);
 	if (!desc) {
 		WARN(1, KERN_ERR "Trying to initialize invalid IRQ%d\n", irq);
 		return;

@@ -223,7 +225,7 @@ static void default_enable(unsigned int
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
-	desc->chip->unmask(irq);
+	desc_chip_unmask(irq, &desc);
 	desc->status &= ~IRQ_MASKED;
 }

@@ -252,7 +254,7 @@ static void default_shutdown(unsigned in
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
-	desc->chip->mask(irq);
+	desc_chip_mask(irq, &desc);
 	desc->status |= IRQ_MASKED;
 }

@@ -282,13 +284,15 @@ void irq_chip_set_defaults(struct irq_ch
 		chip->end = dummy_irq_chip.end;
 }
 
-static inline void mask_ack_irq(struct irq_desc *desc, int irq)
+static inline void mask_ack_irq(struct irq_desc **descp, int irq)
 {
+	struct irq_desc *desc = *descp;
+
 	if (desc->chip->mask_ack)
-		desc->chip->mask_ack(irq);
+		desc_chip_mask_ack(irq, descp);
 	else {
-		desc->chip->mask(irq);
-		desc->chip->ack(irq);
+		desc_chip_mask(irq, descp);
+		desc_chip_ack(irq, descp);
 	}
 }

@@ -351,7 +355,7 @@ handle_level_irq(unsigned int irq, struc
 	irqreturn_t action_ret;
 
 	spin_lock(&desc->lock);
-	mask_ack_irq(desc, irq);
+	mask_ack_irq(&desc, irq);
 
 	if (unlikely(desc->status & IRQ_INPROGRESS))
 		goto out_unlock;

@@ -376,7 +380,7 @@ handle_level_irq(unsigned int irq, struc
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
 	if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
-		desc->chip->unmask(irq);
+		desc_chip_unmask(irq, &desc);
 out_unlock:
 	spin_unlock(&desc->lock);
 }

@@ -413,7 +417,7 @@ handle_fasteoi_irq(unsigned int irq, str
 	if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
 		desc->status |= IRQ_PENDING;
 		if (desc->chip->mask)
-			desc->chip->mask(irq);
+			desc_chip_mask(irq, &desc);
 		goto out;
 	}

@@ -428,7 +432,7 @@ handle_fasteoi_irq(unsigned int irq, str
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
 out:
-	desc->chip->eoi(irq);
+	desc_chip_eoi(irq, &desc);
 
 	spin_unlock(&desc->lock);
 }

@@ -464,13 +468,13 @@ handle_edge_irq(unsigned int irq, struct
 	if (unlikely((desc->status & (IRQ_INPROGRESS | IRQ_DISABLED)) ||
 		    !desc->action)) {
 		desc->status |= (IRQ_PENDING | IRQ_MASKED);
-		mask_ack_irq(desc, irq);
+		mask_ack_irq(&desc, irq);
 		goto out_unlock;
 	}
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	/* Start handling the irq */
-	desc->chip->ack(irq);
+	desc_chip_ack(irq, &desc);
 
 	/* Mark the IRQ currently in progress.*/
 	desc->status |= IRQ_INPROGRESS;

@@ -480,7 +484,7 @@ handle_edge_irq(unsigned int irq, struct
 		irqreturn_t action_ret;
 
 		if (unlikely(!action)) {
-			desc->chip->mask(irq);
+			desc_chip_mask(irq, &desc);
 			goto out_unlock;
 		}

@@ -492,7 +496,7 @@ handle_edge_irq(unsigned int irq, struct
 		if (unlikely((desc->status &
 			       (IRQ_PENDING | IRQ_MASKED | IRQ_DISABLED)) ==
 			      (IRQ_PENDING | IRQ_MASKED))) {
-			desc->chip->unmask(irq);
+			desc_chip_unmask(irq, &desc);
 			desc->status &= ~IRQ_MASKED;
 		}

@@ -525,14 +529,14 @@ handle_percpu_irq(unsigned int irq, stru
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	if (desc->chip->ack)
-		desc->chip->ack(irq);
+		desc_chip_ack(irq, &desc);
 
 	action_ret = handle_IRQ_event(irq, desc->action);
 	if (!noirqdebug)
 		note_interrupt(irq, desc, action_ret);
 
 	if (desc->chip->eoi)
-		desc->chip->eoi(irq);
+		desc_chip_eoi(irq, &desc);
 }
 
 void

@@ -568,7 +572,7 @@ __set_irq_handler(unsigned int irq, irq_
 	/* Uninstall? */
 	if (handle == handle_bad_irq) {
 		if (desc->chip != &no_irq_chip)
-			mask_ack_irq(desc, irq);
+			mask_ack_irq(&desc, irq);
 		desc->status |= IRQ_DISABLED;
 		desc->depth = 1;
 	}

kernel/irq/handle.c +356 ~~-13~~

434 lines not on this page · raw message

arch/x86/kernel/irq.c +21 -3

Index: linux-2.6/arch/x86/kernel/irq.c
===================================================================
@@ -99,25 +99,37 @@ static int show_other_interrupts(struct
 int show_interrupts(struct seq_file *p, void *v)
 {
 	unsigned long flags, any_count = 0;
-	int i = *(loff_t *) v, j;
+	int i, j;
 	struct irqaction *action;
 	struct irq_desc *desc;
+	int head = 0;
 
+#ifdef CONFIG_SPARSE_IRQ
+	desc = list_entry(v, struct irq_desc, list);
+	i = desc->irq;
+	if (&desc->list == sparse_irqs_head.next)
+		head = 1;
+#else
+	i = *(loff_t *) v;
 	if (i > nr_irqs)
 		return 0;
 
 	if (i == nr_irqs)
 		return show_other_interrupts(p);
+	if (i == 0)
+		head = 1;
+
+	desc = irq_to_desc(i);
+#endif
 
 	/* print header */
-	if (i == 0) {
+	if (head) {
 		seq_printf(p, "           ");
 		for_each_online_cpu(j)
 			seq_printf(p, "CPU%-8d", j);
 		seq_putc(p, '\n');
 	}
 
-	desc = irq_to_desc(i);
 	spin_lock_irqsave(&desc->lock, flags);
 #ifndef CONFIG_SMP
 	any_count = kstat_irqs(i);

@@ -148,6 +160,12 @@ int show_interrupts(struct seq_file *p,
 	seq_putc(p, '\n');
 out:
 	spin_unlock_irqrestore(&desc->lock, flags);
+
+#ifdef CONFIG_SPARSE_IRQ
+	if (&desc->list == sparse_irqs_head.prev)
+		show_other_interrupts(p);
+#endif
+
 	return 0;
 }

include/linux/irqnr.h +4 ~~-11~~

Index: linux-2.6/include/linux/irqnr.h
===================================================================
@@ -7,18 +7,11 @@
 
 # define for_each_irq_desc(irq, desc)		\
 	for (irq = 0; irq < nr_irqs; irq++)
-#else
-extern int nr_irqs;
+# define end_for_each_irq_desc()
 
-# define for_each_irq_desc(irq, desc)		\
-	for (irq = 0, desc = irq_desc; irq < nr_irqs; irq++, desc++)
-
-# define for_each_irq_desc_reverse(irq, desc)				\
-	for (irq = nr_irqs - 1, desc = irq_desc + (nr_irqs - 1);	\
-	     irq >= 0; irq--, desc--)
+static inline early_sparse_irq_init(void)
+{
+}
 #endif
 
-#define for_each_irq_nr(irq)			\
-	for (irq = 0; irq < nr_irqs; irq++)
-
 #endif

arch/x86/kernel/irq_32.c +2 -2

Index: linux-2.6/arch/x86/kernel/irq_32.c
===================================================================
@@ -251,10 +251,10 @@ void fixup_irqs(cpumask_t map)
 			mask = map;
 		}
 		if (desc->chip->set_affinity)
-			desc->chip->set_affinity(irq, mask);
+			desc_chip_set_affinity(irq, desc, mask);
 		else if (desc->action && !(warned++))
 			printk("Cannot set affinity for irq %i\n", irq);
-	}
+	} end_for_each_irq_desc();
 
 #if 0
 	barrier();

arch/x86/kernel/irq_64.c +4 -4

Index: linux-2.6/arch/x86/kernel/irq_64.c
===================================================================
@@ -113,15 +113,15 @@ void fixup_irqs(cpumask_t map)
 		}
 
 		if (desc->chip->mask)
-			desc->chip->mask(irq);
+			desc_chip_mask(irq, &desc);
 
 		if (desc->chip->set_affinity)
-			desc->chip->set_affinity(irq, mask);
+			desc_chip_set_affinity(irq, desc, mask);
 		else if (!(warned++))
 			set_affinity = 0;
 
 		if (desc->chip->unmask)
-			desc->chip->unmask(irq);
+			desc_chip_unmask(irq, &desc);
 
 		spin_unlock(&desc->lock);

@@ -129,7 +129,7 @@ void fixup_irqs(cpumask_t map)
 			printk("Broke affinity for irq %i\n", irq);
 		else if (!set_affinity)
 			printk("Cannot set affinity for irq %i\n", irq);
-	}
+	} end_for_each_irq_desc();
 
 	/* That doesn't seem sufficient.  Give it 1ms. */
 	local_irq_enable();

kernel/irq/proc.c +2 -1

Index: linux-2.6/kernel/irq/proc.c
===================================================================
@@ -243,7 +243,8 @@ void init_irq_proc(void)
 	/*
 	 * Create entries for all existing IRQs.
 	 */
-	for_each_irq_desc(irq, desc)
+	for_each_irq_desc(irq, desc) {
 		register_irq_proc(irq, desc);
+	} end_for_each_irq_desc();
 }

kernel/irq/spurious.c +2 -2

Index: linux-2.6/kernel/irq/spurious.c
===================================================================
@@ -99,7 +99,7 @@ static int misrouted_irq(int irq)
 
 		if (try_one_irq(i, desc))
 			ok = 1;
-	}
+	} end_for_each_irq_desc();
 	/* So the caller can adjust the irq error counts */
 	return ok;
 }

@@ -122,7 +122,7 @@ static void poll_spurious_irqs(unsigned
 			continue;
 
 		try_one_irq(i, desc);
-	}
+	} end_for_each_irq_desc();
 
 	mod_timer(&poll_spurious_irq_timer,
 		  jiffies + POLL_SPURIOUS_IRQ_INTERVAL);

init/main.c +11 -0

Index: linux-2.6/init/main.c
===================================================================
@@ -542,6 +542,15 @@ void __init __weak thread_info_cache_ini
 {
 }
 
+void __init __weak arch_early_irq_init(void)
+{
[… more lines in the raw message]

Lines of this file are not shown. View the raw message.

arch/x86/include/asm/irq_vectors.h +2 -0

Index: linux-2.6/arch/x86/include/asm/irq_vectors.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/i8259.c +24 -5

Index: linux-2.6/arch/x86/kernel/i8259.c
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/uv_irq.c +23 -4

Index: linux-2.6/arch/x86/kernel/uv_irq.c
===================================================================

Lines of this file are not shown. View the raw message.

drivers/pci/msi.c +54 ~~-17~~

71 lines not on this page · raw message

include/linux/msi.h +10 -0

Index: linux-2.6/include/linux/msi.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/include/asm/hpet.h +7 -0

Index: linux-2.6/arch/x86/include/asm/hpet.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/hpet.c +19 -4

Index: linux-2.6/arch/x86/kernel/hpet.c
===================================================================

Lines of this file are not shown. View the raw message.

include/linux/htirq.h +8 -0

Index: linux-2.6/include/linux/htirq.h
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/migration.c +26 -8

Index: linux-2.6/kernel/irq/migration.c
===================================================================

Lines of this file are not shown. View the raw message.

drivers/pci/intel-iommu.c +19 -4

Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================

Lines of this file are not shown. View the raw message.

include/linux/dmar.h +7 -0

Index: linux-2.6/include/linux/dmar.h
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/manage.c +3 -3

Index: linux-2.6/kernel/irq/manage.c
===================================================================

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

Andrew Morton

2008-11-14 06:47 UTC #44 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v14

Quoted 7 lines · On Thu, 13 Nov 2008 22:29:21 -0800 Yinghai Lu <[email protected]> wrote:

address some Andrew's concerns.
...
arch/x86/kernel/io_apic.c +3 -0
+static struct irq_pin_list *get_one_free_irq_2_pin(int cpu)
+{
+	struct irq_pin_list *pin;

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 136

Quoted 10 lines

arch/x86/kernel/io_apic.c +10 -0

+	int node;
+
+	node = cpu_to_node(cpu);
+
+	pin = kzalloc_node(sizeof(*pin), GFP_ATOMIC, node);
+	printk(KERN_DEBUG "  alloc irq_2_pin on cpu %d node %d\n", cpu, node);
+	BUG_ON(!pin);
+
+	return pin;
+}

GFP_ATOMIC allocation attempts are unreliable - much more so than GFP_KERNEL. GFP_ATOMIC allocations can and do fail. With the above code, such a failure will crash the machine.

The code should handle this error and recover gracefully.

Quoted 6 lines

...

arch/x86/kernel/io_apic.c +3 -0

+static struct irq_cfg *get_one_free_irq_cfg(int cpu)
+{
+	struct irq_cfg *cfg;

arch/x86/kernel/io_apic.c static int __init parse_noapic(char *str · line 222

Quoted 10 lines

arch/x86/kernel/io_apic.c +9 -0

+	int node;
+
+	node = cpu_to_node(cpu);
+
+	cfg = kzalloc_node(sizeof(*cfg), GFP_ATOMIC, node);
+	printk(KERN_DEBUG "  alloc irq_cfg on cpu %d node %d\n", cpu, node);
+	BUG_ON(!cfg);
+
+	return cfg;
 }

Ditto

Quoted 4 lines

...
drivers/pci/intr_remapping.c +2 -0
+static struct irq_2_iommu *get_one_free_irq_2_iommu(int cpu)
+{

drivers/pci/intr_remapping.c struct irq_2_iommu { · line 34

Quoted 10 lines

drivers/pci/intr_remapping.c +10 -0

+	struct irq_2_iommu *iommu;
+	int node;
+
+	node = cpu_to_node(cpu);
+
+	iommu = kzalloc_node(sizeof(*iommu), GFP_ATOMIC, node);
+	printk(KERN_DEBUG "alloc irq_2_iommu on cpu %d node %d\n", cpu, node);
+
+	return iommu;
+}

I spent some time trying to work out whether the callers handle failure here but I got lost in a twisty maze.

Quoted 8 lines

...

kernel/irq/handle.c +6 -0

+static void init_kstat_irqs(struct irq_desc *desc, int cpu, int nr)
+{
+	unsigned long bytes;
+	char *ptr;
+	int node;
+

kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 91

Quoted 10 lines

kernel/irq/handle.c +10 -0

+	/* Compute how many bytes we need per irq and allocate them */
+	bytes = nr * sizeof(unsigned int);
+
+	node = cpu_to_node(cpu);
+	ptr = kzalloc_node(bytes, GFP_ATOMIC, node);
+	printk(KERN_DEBUG "  alloc kstat_irqs on cpu %d node %d\n", cpu, node);
+	BUG_ON(!ptr);
+
+	desc->kstat_irqs = (unsigned int *)ptr;
+}

Ditto.

Quoted 22 lines

...

kernel/irq/handle.c +19 -0

+struct irq_desc *irq_to_desc_alloc_cpu(unsigned int irq, int cpu)
+{
+	struct irq_desc *desc;
+	struct list_head *hash_head;
+	unsigned long flags;
+	int node;
+
+	desc = irq_to_desc(irq);
+	if (desc)
+		return desc;
+
+	hash_head = sparseirqhashentry(irq);
+
+	spin_lock_irqsave(&sparse_irq_lock, flags);
+
+	/*
+	 * We have to do the hash-walk again, to avoid races
+	 * with another CPU:
+	 */

kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 253

Quoted 10 lines

kernel/irq/handle.c +10 -0

+	list_for_each_entry(desc, hash_head, hash_entry) {
+		if (desc->irq == irq)
+			goto out_unlock;
+	}
+
+	node = cpu_to_node(cpu);
+	desc = kzalloc_node(sizeof(*desc), GFP_ATOMIC, node);
+	printk(KERN_DEBUG "  alloc irq_desc for %d aka %#x on cpu %d node %d\n",
+		 irq, irq, cpu, node);
+	BUG_ON(!desc);

Ditto.

Quoted 39 lines

kernel/irq/handle.c +36 -0

+	init_one_irq_desc(irq, desc, cpu);
+
+	/*
+	 * We use RCU's safe list-add method to make
+	 * parallel walking of the hash-list safe:
+	 */
+	list_add_tail_rcu(&desc->hash_entry, hash_head);
+	/*
+	 * Add it to the global list:
+	 */
+	list_add_tail_rcu(&desc->list, &sparse_irqs_head);
+
+out_unlock:
+	spin_unlock_irqrestore(&sparse_irq_lock, flags);
+
+	return desc;
+}
+
 
...
 
+static struct irq_desc *__real_move_irq_desc(struct irq_desc *old_desc,
+						int cpu)
+{
+	struct irq_desc *desc;
+	unsigned int irq;
+	struct list_head *hash_head;
+	unsigned long flags;
+	int node;
+
+	irq = old_desc->irq;
+
+	hash_head = sparseirqhashentry(irq);
+
+	spin_lock_irqsave(&sparse_irq_lock, flags);
+	/*
+	 * We have to do the hash-walk again, to avoid races
+	 * with another CPU:
+	 */

kernel/irq/handle.c void handle_bad_irq(unsigned int irq, st · line 305

Quoted 10 lines

kernel/irq/handle.c +10 -0

+	list_for_each_entry(desc, hash_head, hash_entry) {
+		if (desc->irq == irq && old_desc != desc)
+			goto out_unlock;
+	}
+
+	node = cpu_to_node(cpu);
+	desc = kzalloc_node(sizeof(*desc), GFP_ATOMIC, node);
+	printk(KERN_DEBUG "  move irq_desc for %d aka %#x to cpu %d node %d\n",
+		 irq, irq, cpu, node);
+	BUG_ON(!desc);

Ditto.

Quoted 15 lines

kernel/irq/handle.c +15 -0

+	init_copy_one_irq_desc(irq, old_desc, desc, cpu);
+
+	list_replace_rcu(&old_desc->hash_entry, &desc->hash_entry);
+	list_replace_rcu(&old_desc->list, &desc->list);
+
+	/* free the old one */
+	free_one_irq_desc(old_desc);
+	kfree(old_desc);
+
+out_unlock:
+	spin_unlock_irqrestore(&sparse_irq_lock, flags);
+
+	return desc;
+}
+

Yinghai Lu

author

2008-11-15 09:07 UTC #45 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v14

please check it. address kzalloc_node return handling.

---

From: Yinghai Lu <[email protected]>
Subject: sparseirq v15

impact: new feature sparseirq

add some kind of hash table as Ingo suggesting.
remove dyna_array

when sparse_irq is used (CONFIG_SPARSE_IRQ), use kzalloc_node to get irq_desc, irq_cfg
use desc->chip_data for x86 to store irq_cfg

for logical apic mode, need to add move_desc_in_progress_in_same_domain. otherwise it will not get moved. ==> also could need two phase to get irq_desc moved.

	for example: 0xff is old affinity, and need to set 0xf, and then set to 0xf0.
	[ or we need to change domain definition to cpus on the same node ? ]

Oct 27 12:35:34 LBSuse kernel: klogd 1.4.1, log source = /proc/kmsg started.
Oct 27 12:35:34 LBSuse kernel: eth0: no IPv6 routers present
LBSuse:~ # echo f0 > /proc/irq/22/smp_affinity
LBSuse:~ # tail /var/log/messages
Oct 27 12:35:34 LBSuse kernel: klogd 1.4.1, log source = /proc/kmsg started.
Oct 27 12:35:34 LBSuse kernel: eth0: no IPv6 routers present
Oct 27 12:36:46 LBSuse kernel:   move irq_desc for 22 aka 0x16 to cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc kstat_irqs on cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc irq_cfg on cpu 7 node 1
Oct 27 12:36:46 LBSuse kernel:   alloc irq_2_pin on cpu 7 node 1

so assume the user space program should update /proc/irq/XX/smp_affinity to 03 or 0f at first on boot or we change irq_default_affinity ?

for physical apic is much simple
on 4 sockets 16 cores system
irq_desc is moving..
when

# echo 10 > /proc/irq/134483967/smp_affinity
# echo 100 > /proc/irq/134483967/smp_affinity
# echo 1000 > /proc/irq/134483967/smp_affinity
got
Nov  9 21:39:51 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 4 node 1
Nov  9 21:39:51 LBSuse kernel:   alloc kstat_irqs on cpu 4 node 1
Nov  9 21:39:51 LBSuse kernel:   alloc irq_cfg on cpu 4 node 1
Nov  9 21:40:05 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 8 node 2
Nov  9 21:40:05 LBSuse kernel:   alloc kstat_irqs on cpu 8 node 2
Nov  9 21:40:05 LBSuse kernel:   alloc irq_cfg on cpu 8 node 2
Nov  9 21:40:18 LBSuse kernel:   move irq_desc for 134483967 aka 0x8040fff to cpu 12 node 3
Nov  9 21:40:18 LBSuse kernel:   alloc kstat_irqs on cpu 12 node 3
Nov  9 21:40:18 LBSuse kernel:   alloc irq_cfg on cpu 12 node 3

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/Kconfig                   |   20 
 arch/x86/include/asm/hpet.h        |    7 
 arch/x86/include/asm/irq_vectors.h |    2 
 arch/x86/kernel/hpet.c             |   23 
 arch/x86/kernel/i8259.c            |   29 +
 arch/x86/kernel/io_apic.c          |  880 ++++++++++++++++++++++++++-----------
 arch/x86/kernel/irq.c              |   24 -
 arch/x86/kernel/irq_32.c           |    4 
 arch/x86/kernel/irq_64.c           |    8 
 arch/x86/kernel/irqinit_32.c       |    3 
 arch/x86/kernel/irqinit_64.c       |    3 
 arch/x86/kernel/uv_irq.c           |   27 -
 arch/x86/mm/init_32.c              |    3 
 drivers/char/random.c              |   31 +
 drivers/pci/htirq.c                |   44 +
 drivers/pci/intel-iommu.c          |   23 
 drivers/pci/intr_remapping.c       |   72 ++-
 drivers/pci/msi.c                  |   71 ++
 drivers/xen/events.c               |    9 
 fs/proc/interrupts.c               |   18 
 fs/proc/stat.c                     |   17 
 include/linux/dmar.h               |    7 
 include/linux/htirq.h              |    8 
 include/linux/interrupt.h          |    2 
 include/linux/irq.h                |   95 +++
 include/linux/irqnr.h              |   15 
 include/linux/kernel_stat.h        |   14 
 include/linux/msi.h                |   10 
 init/main.c                        |   11 
 kernel/irq/autoprobe.c             |   10 
 kernel/irq/chip.c                  |   40 -
 kernel/irq/handle.c                |  386 +++++++++++++++-
 kernel/irq/manage.c                |    6 
 kernel/irq/migration.c             |   34 +
 kernel/irq/proc.c                  |    3 
 kernel/irq/spurious.c              |    4 
 36 files changed, 1590 insertions(+), 373 deletions(-)

arch/x86/Kconfig +20 -0

Index: linux-2.6/arch/x86/Kconfig
===================================================================
@@ -240,6 +240,26 @@ config X86_HAS_BOOT_CPU_ID
 	def_bool y
 	depends on X86_VOYAGER
 
+config SPARSE_IRQ
+	bool "Support sparse irq numbering"
+	depends on PCI_MSI || HT_IRQ
+	default y
+	help
+	  This enables support for sparse irq, esp for msi/msi-x. the irq
+	  number will be bus/dev/fn + 12bit. You may need if you have lots of
+	  cards supports msi-x installed.
+
+	  If you don't know what to do here, say Y.
+
+config MOVE_IRQ_DESC
+	bool "Move irq desc when changing irq smp_affinity"
+	depends on SPARSE_IRQ && SMP
+	default y
+	help
+	  This enables moving irq_desc to cpu/node that irq will use handled.
+
+	  If you don't know what to do here, say Y.
+
 config X86_FIND_SMP_CONFIG
 	def_bool y
 	depends on X86_MPPARSE || X86_VOYAGER

arch/x86/kernel/io_apic.c +637 ~~-243~~

1,720 lines not on this page · raw message

arch/x86/kernel/irqinit_32.c +1 -2

Index: linux-2.6/arch/x86/kernel/irqinit_32.c
===================================================================
@@ -68,8 +68,7 @@ void __init init_ISA_irqs (void)
 	/*
 	 * 16 old-style INTA-cycle interrupts:
 	 */
-	for (i = 0; i < 16; i++) {
-		/* first time call this irq_desc */
+	for (i = 0; i < NR_IRQS_LEGACY; i++) {
 		struct irq_desc *desc = irq_to_desc(i);
 
 		desc->status = IRQ_DISABLED;

arch/x86/kernel/irqinit_64.c +1 -2

Index: linux-2.6/arch/x86/kernel/irqinit_64.c
===================================================================
@@ -142,8 +142,7 @@ void __init init_ISA_irqs(void)
 	init_bsp_APIC();
 	init_8259A(0);
 
-	for (i = 0; i < 16; i++) {
-		/* first time call this irq_desc */
+	for (i = 0; i < NR_IRQS_LEGACY; i++) {
 		struct irq_desc *desc = irq_to_desc(i);
 
 		desc->status = IRQ_DISABLED;

arch/x86/mm/init_32.c +3 -0

Index: linux-2.6/arch/x86/mm/init_32.c
===================================================================
@@ -66,6 +66,7 @@ static unsigned long __meminitdata table
 static unsigned long __meminitdata table_top;
 
 static int __initdata after_init_bootmem;
+int after_bootmem;
 
 static __init void *alloc_low_page(unsigned long *phys)
 {

@@ -987,6 +988,8 @@ void __init mem_init(void)
 
 	set_highmem_pages_init();
 
+	after_bootmem = 1;
+
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;
 	datasize =  (unsigned long) &_edata - (unsigned long) &_etext;
 	initsize =  (unsigned long) &__init_end - (unsigned long) &__init_begin;

drivers/char/random.c +31 -0

Index: linux-2.6/drivers/char/random.c
===================================================================
@@ -558,6 +558,8 @@ struct timer_rand_state {
 	unsigned dont_count_entropy:1;
 };
 
+#ifndef CONFIG_SPARSE_IRQ
+
 static struct timer_rand_state *irq_timer_state[NR_IRQS];
 
 static struct timer_rand_state *get_timer_rand_state(unsigned int irq)

@@ -576,6 +578,33 @@ static void set_timer_rand_state(unsigne
 	irq_timer_state[irq] = state;
 }
 
+#else
+
+static struct timer_rand_state *get_timer_rand_state(unsigned int irq)
+{
+	struct irq_desc *desc;
+
+	desc = irq_to_desc(irq);
+
+	if (!desc)
+		return NULL;
+
+	return desc->timer_rand_state;
+}
+
+static void set_timer_rand_state(unsigned int irq, struct timer_rand_state *state)
+{
+	struct irq_desc *desc;
+
+	desc = irq_to_desc(irq);
+
+	if (!desc)
+		return;
+
+	desc->timer_rand_state = state;
+}
+#endif
+
 static struct timer_rand_state input_timer_state;
 
 /*

@@ -933,8 +962,10 @@ void rand_initialize_irq(int irq)
 {
 	struct timer_rand_state *state;
 
+#ifndef CONFIG_SPARSE_IRQ
 	if (irq >= nr_irqs)
 		return;
+#endif
 
 	state = get_timer_rand_state(irq);

drivers/pci/htirq.c +40 -4

77 lines not on this page · raw message

drivers/pci/intr_remapping.c +70 -2

108 lines not on this page · raw message

drivers/xen/events.c +5 -4

Index: linux-2.6/drivers/xen/events.c
===================================================================
@@ -141,8 +141,9 @@ static void init_evtchn_cpu_bindings(voi
 	int i;
 
 	/* By default all event channels notify CPU#0. */
-	for_each_irq_desc(i, desc)
+	for_each_irq_desc(i, desc) {
 		desc->affinity = cpumask_of_cpu(0);
+	} end_for_each_irq_desc();
 #endif
 
 	memset(cpu_evtchn, 0, sizeof(cpu_evtchn));

@@ -231,7 +232,7 @@ static int find_unbound_irq(void)
 	int irq;
 
 	/* Only allocate from dynirq range */
-	for_each_irq_nr(irq)
+	for (irq = 0; irq < nr_irqs; irq++)
 		if (irq_bindcount[irq] == 0)
 			break;

@@ -792,7 +793,7 @@ void xen_irq_resume(void)
 		mask_evtchn(evtchn);
 
 	/* No IRQ <-> event-channel mappings. */
-	for_each_irq_nr(irq)
+	for (irq = 0; irq < nr_irqs; irq++)
 		irq_info[irq].evtchn = 0; /* zap event-channel binding */
 
 	for (evtchn = 0; evtchn < NR_EVENT_CHANNELS; evtchn++)

@@ -824,7 +825,7 @@ void __init xen_init_IRQ(void)
 		mask_evtchn(i);
 
 	/* Dynamic IRQ space is currently unbound. Zero the refcnts. */
-	for_each_irq_nr(i)
+	for (i = 0; i < nr_irqs; i++)
 		irq_bindcount[i] = 0;
 
 	irq_ctx_init(smp_processor_id());

fs/proc/stat.c +11 -6

Index: linux-2.6/fs/proc/stat.c
===================================================================
@@ -27,6 +27,9 @@ static int show_stat(struct seq_file *p,
 	u64 sum = 0;
 	struct timespec boottime;
 	unsigned int per_irq_sum;
+#ifdef CONFIG_GENERIC_HARDIRQS
+	struct irq_desc *desc;
+#endif
 
 	user = nice = system = idle = iowait =
 		irq = softirq = steal = cputime64_zero;

@@ -44,10 +47,9 @@ static int show_stat(struct seq_file *p,
 		softirq = cputime64_add(softirq, kstat_cpu(i).cpustat.softirq);
 		steal = cputime64_add(steal, kstat_cpu(i).cpustat.steal);
 		guest = cputime64_add(guest, kstat_cpu(i).cpustat.guest);
-
-		for_each_irq_nr(j)
+		for_each_irq_desc(j, desc) {
 			sum += kstat_irqs_cpu(j, i);
-
+		} end_for_each_irq_desc();
 		sum += arch_irq_stat_cpu(i);
 	}
 	sum += arch_irq_stat();

@@ -90,14 +92,17 @@ static int show_stat(struct seq_file *p,
 	seq_printf(p, "intr %llu", (unsigned long long)sum);
 
 	/* sum again ? it could be updated? */
-	for_each_irq_nr(j) {
+	for_each_irq_desc(j, desc) {
 		per_irq_sum = 0;
-
 		for_each_possible_cpu(i)
 			per_irq_sum += kstat_irqs_cpu(j, i);
 
+#ifdef CONFIG_SPARSE_IRQ
+		seq_printf(p, " %#x:%u", j, per_irq_sum);
+#else
 		seq_printf(p, " %u", per_irq_sum);
-	}
+#endif
+	} end_for_each_irq_desc();
 
 	seq_printf(p,
 		"\nctxt %llu\n"

fs/proc/interrupts.c +18 -0

Index: linux-2.6/fs/proc/interrupts.c
===================================================================
@@ -8,6 +8,23 @@
 /*
  * /proc/interrupts
  */
+#ifdef CONFIG_SPARSE_IRQ
+static void *int_seq_start(struct seq_file *f, loff_t *pos)
+{
+	rcu_read_lock();
+	return seq_list_start(&sparse_irqs_head, *pos);
+}
+
+static void *int_seq_next(struct seq_file *f, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &sparse_irqs_head, pos);
+}
+
+static void int_seq_stop(struct seq_file *f, void *v)
+{
+	rcu_read_unlock();
+}
+#else
 static void *int_seq_start(struct seq_file *f, loff_t *pos)
 {
 	return (*pos <= nr_irqs) ? pos : NULL;

@@ -25,6 +42,7 @@ static void int_seq_stop(struct seq_file
 {
 	/* Nothing to do */
 }
+#endif
 
 static const struct seq_operations int_seq_ops = {
 	.start = int_seq_start,

include/linux/interrupt.h +2 -0

Index: linux-2.6/include/linux/interrupt.h
===================================================================
@@ -18,6 +18,8 @@
 #include <asm/ptrace.h>
 #include <asm/system.h>
 
+extern int nr_irqs;
+
 /*
  * These correspond to the IORESOURCE_IRQ_* defines in
  * linux/ioport.h to select the interrupt line behaviour.  When

include/linux/irq.h +90 -5

143 lines not on this page · raw message

include/linux/kernel_stat.h +13 -1

Index: linux-2.6/include/linux/kernel_stat.h
===================================================================
@@ -28,7 +28,9 @@ struct cpu_usage_stat {
 
 struct kernel_stat {
 	struct cpu_usage_stat	cpustat;
-	unsigned int irqs[NR_IRQS];
+#ifndef CONFIG_SPARSE_IRQ
+       unsigned int irqs[NR_IRQS];
+#endif
 };
 
 DECLARE_PER_CPU(struct kernel_stat, kstat);

@@ -39,6 +41,10 @@ DECLARE_PER_CPU(struct kernel_stat, ksta
 
 extern unsigned long long nr_context_switches(void);
 
+#ifndef CONFIG_SPARSE_IRQ
+#define kstat_irqs_this_cpu(irq) \
+	(kstat_this_cpu.irqs[irq])
+
 struct irq_desc;
 
 static inline void kstat_incr_irqs_this_cpu(unsigned int irq,

@@ -46,11 +52,17 @@ static inline void kstat_incr_irqs_this_
 {
 	kstat_this_cpu.irqs[irq]++;
 }
+#endif
+
 
+#ifndef CONFIG_SPARSE_IRQ
 static inline unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
 {
        return kstat_cpu(cpu).irqs[irq];
 }
+#else
+extern unsigned int kstat_irqs_cpu(unsigned int irq, int cpu);
+#endif
 
 /*
  * Number of interrupts per specific IRQ source, since bootup

kernel/irq/autoprobe.c +5 -5

Index: linux-2.6/kernel/irq/autoprobe.c
===================================================================
@@ -57,7 +57,7 @@ unsigned long probe_irq_on(void)
 			desc->chip->startup(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	/* Wait for longstanding interrupts to trigger. */
 	msleep(20);

@@ -75,7 +75,7 @@ unsigned long probe_irq_on(void)
 				desc->status |= IRQ_PENDING;
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	/*
 	 * Wait for spurious interrupts to trigger

@@ -99,7 +99,7 @@ unsigned long probe_irq_on(void)
 					mask |= 1 << i;
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 
 	return mask;
 }

@@ -135,7 +135,7 @@ unsigned int probe_irq_mask(unsigned lon
 			desc->chip->shutdown(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 	mutex_unlock(&probing_active);
 
 	return mask & val;

@@ -179,7 +179,7 @@ int probe_irq_off(unsigned long val)
 			desc->chip->shutdown(i);
 		}
 		spin_unlock_irq(&desc->lock);
-	}
+	} end_for_each_irq_desc();
 	mutex_unlock(&probing_active);
 
 	if (nr_of_irqs > 1)

kernel/irq/chip.c +22 ~~-18~~

Index: linux-2.6/kernel/irq/chip.c
===================================================================
@@ -24,9 +24,11 @@
  */
 void dynamic_irq_init(unsigned int irq)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_desc *desc;
 	unsigned long flags;
 
+	/* first time to use this irq_desc */
+	desc = irq_to_desc_alloc(irq);
 	if (!desc) {
 		WARN(1, KERN_ERR "Trying to initialize invalid IRQ%d\n", irq);
 		return;

@@ -223,7 +225,7 @@ static void default_enable(unsigned int
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
-	desc->chip->unmask(irq);
+	desc_chip_unmask(irq, &desc);
 	desc->status &= ~IRQ_MASKED;
 }

@@ -252,7 +254,7 @@ static void default_shutdown(unsigned in
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
-	desc->chip->mask(irq);
+	desc_chip_mask(irq, &desc);
 	desc->status |= IRQ_MASKED;
 }

@@ -282,13 +284,15 @@ void irq_chip_set_defaults(struct irq_ch
 		chip->end = dummy_irq_chip.end;
 }
 
-static inline void mask_ack_irq(struct irq_desc *desc, int irq)
+static inline void mask_ack_irq(struct irq_desc **descp, int irq)
 {
+	struct irq_desc *desc = *descp;
+
 	if (desc->chip->mask_ack)
-		desc->chip->mask_ack(irq);
+		desc_chip_mask_ack(irq, descp);
 	else {
-		desc->chip->mask(irq);
-		desc->chip->ack(irq);
+		desc_chip_mask(irq, descp);
+		desc_chip_ack(irq, descp);
 	}
 }

@@ -351,7 +355,7 @@ handle_level_irq(unsigned int irq, struc
 	irqreturn_t action_ret;
 
 	spin_lock(&desc->lock);
-	mask_ack_irq(desc, irq);
+	mask_ack_irq(&desc, irq);
 
 	if (unlikely(desc->status & IRQ_INPROGRESS))
 		goto out_unlock;

@@ -376,7 +380,7 @@ handle_level_irq(unsigned int irq, struc
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
 	if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
-		desc->chip->unmask(irq);
+		desc_chip_unmask(irq, &desc);
 out_unlock:
 	spin_unlock(&desc->lock);
 }

@@ -413,7 +417,7 @@ handle_fasteoi_irq(unsigned int irq, str
 	if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
 		desc->status |= IRQ_PENDING;
 		if (desc->chip->mask)
-			desc->chip->mask(irq);
+			desc_chip_mask(irq, &desc);
 		goto out;
 	}

@@ -428,7 +432,7 @@ handle_fasteoi_irq(unsigned int irq, str
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
 out:
-	desc->chip->eoi(irq);
+	desc_chip_eoi(irq, &desc);
 
 	spin_unlock(&desc->lock);
 }

@@ -464,13 +468,13 @@ handle_edge_irq(unsigned int irq, struct
 	if (unlikely((desc->status & (IRQ_INPROGRESS | IRQ_DISABLED)) ||
 		    !desc->action)) {
 		desc->status |= (IRQ_PENDING | IRQ_MASKED);
-		mask_ack_irq(desc, irq);
+		mask_ack_irq(&desc, irq);
 		goto out_unlock;
 	}
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	/* Start handling the irq */
-	desc->chip->ack(irq);
+	desc_chip_ack(irq, &desc);
 
 	/* Mark the IRQ currently in progress.*/
 	desc->status |= IRQ_INPROGRESS;

@@ -480,7 +484,7 @@ handle_edge_irq(unsigned int irq, struct
 		irqreturn_t action_ret;
 
 		if (unlikely(!action)) {
-			desc->chip->mask(irq);
+			desc_chip_mask(irq, &desc);
 			goto out_unlock;
 		}

@@ -492,7 +496,7 @@ handle_edge_irq(unsigned int irq, struct
 		if (unlikely((desc->status &
 			       (IRQ_PENDING | IRQ_MASKED | IRQ_DISABLED)) ==
 			      (IRQ_PENDING | IRQ_MASKED))) {
-			desc->chip->unmask(irq);
+			desc_chip_unmask(irq, &desc);
 			desc->status &= ~IRQ_MASKED;
 		}

@@ -525,14 +529,14 @@ handle_percpu_irq(unsigned int irq, stru
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	if (desc->chip->ack)
-		desc->chip->ack(irq);
+		desc_chip_ack(irq, &desc);
 
 	action_ret = handle_IRQ_event(irq, desc->action);
 	if (!noirqdebug)
 		note_interrupt(irq, desc, action_ret);
 
 	if (desc->chip->eoi)
-		desc->chip->eoi(irq);
+		desc_chip_eoi(irq, &desc);
 }
 
 void

@@ -568,7 +572,7 @@ __set_irq_handler(unsigned int irq, irq_
 	/* Uninstall? */
 	if (handle == handle_bad_irq) {
 		if (desc->chip != &no_irq_chip)
-			mask_ack_irq(desc, irq);
+			mask_ack_irq(&desc, irq);
 		desc->status |= IRQ_DISABLED;
 		desc->depth = 1;
 	}

kernel/irq/handle.c +373 ~~-13~~

451 lines not on this page · raw message

arch/x86/kernel/irq.c +21 -3

Index: linux-2.6/arch/x86/kernel/irq.c
===================================================================
@@ -99,25 +99,37 @@ static int show_other_interrupts(struct
 int show_interrupts(struct seq_file *p, void *v)
 {
 	unsigned long flags, any_count = 0;
-	int i = *(loff_t *) v, j;
+	int i, j;
 	struct irqaction *action;
 	struct irq_desc *desc;
+	int head = 0;
 
+#ifdef CONFIG_SPARSE_IRQ
+	desc = list_entry(v, struct irq_desc, list);
+	i = desc->irq;
+	if (&desc->list == sparse_irqs_head.next)
+		head = 1;
+#else
+	i = *(loff_t *) v;
 	if (i > nr_irqs)
 		return 0;
 
 	if (i == nr_irqs)
 		return show_other_interrupts(p);
+	if (i == 0)
+		head = 1;
+
+	desc = irq_to_desc(i);
+#endif
 
 	/* print header */
-	if (i == 0) {
+	if (head) {
 		seq_printf(p, "           ");
 		for_each_online_cpu(j)
 			seq_printf(p, "CPU%-8d", j);
 		seq_putc(p, '\n');
 	}
 
-	desc = irq_to_desc(i);
 	spin_lock_irqsave(&desc->lock, flags);
 #ifndef CONFIG_SMP
 	any_count = kstat_irqs(i);

@@ -148,6 +160,12 @@ int show_interrupts(struct seq_file *p,
 	seq_putc(p, '\n');
 out:
 	spin_unlock_irqrestore(&desc->lock, flags);
+
+#ifdef CONFIG_SPARSE_IRQ
+	if (&desc->list == sparse_irqs_head.prev)
+		show_other_interrupts(p);
+#endif
+
 	return 0;
 }

include/linux/irqnr.h +4 ~~-11~~

Index: linux-2.6/include/linux/irqnr.h
===================================================================
@@ -7,18 +7,11 @@
 
 # define for_each_irq_desc(irq, desc)		\
 	for (irq = 0; irq < nr_irqs; irq++)
-#else
-extern int nr_irqs;
+# define end_for_each_irq_desc()
 
-# define for_each_irq_desc(irq, desc)		\
-	for (irq = 0, desc = irq_desc; irq < nr_irqs; irq++, desc++)
-
-# define for_each_irq_desc_reverse(irq, desc)				\
-	for (irq = nr_irqs - 1, desc = irq_desc + (nr_irqs - 1);	\
-	     irq >= 0; irq--, desc--)
+static inline early_sparse_irq_init(void)
+{
[… more lines in the raw message]

Lines of this file are not shown. View the raw message.

arch/x86/kernel/irq_32.c +2 -2

Index: linux-2.6/arch/x86/kernel/irq_32.c
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/irq_64.c +4 -4

Index: linux-2.6/arch/x86/kernel/irq_64.c
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/proc.c +2 -1

Index: linux-2.6/kernel/irq/proc.c
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/spurious.c +2 -2

Index: linux-2.6/kernel/irq/spurious.c
===================================================================

Lines of this file are not shown. View the raw message.

init/main.c +11 -0

Index: linux-2.6/init/main.c
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/include/asm/irq_vectors.h +2 -0

Index: linux-2.6/arch/x86/include/asm/irq_vectors.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/i8259.c +24 -5

Index: linux-2.6/arch/x86/kernel/i8259.c
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/uv_irq.c +23 -4

Index: linux-2.6/arch/x86/kernel/uv_irq.c
===================================================================

Lines of this file are not shown. View the raw message.

drivers/pci/msi.c +54 ~~-17~~

71 lines not on this page · raw message

include/linux/msi.h +10 -0

Index: linux-2.6/include/linux/msi.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/include/asm/hpet.h +7 -0

Index: linux-2.6/arch/x86/include/asm/hpet.h
===================================================================

Lines of this file are not shown. View the raw message.

arch/x86/kernel/hpet.c +19 -4

Index: linux-2.6/arch/x86/kernel/hpet.c
===================================================================

Lines of this file are not shown. View the raw message.

include/linux/htirq.h +8 -0

Index: linux-2.6/include/linux/htirq.h
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/migration.c +26 -8

Index: linux-2.6/kernel/irq/migration.c
===================================================================

Lines of this file are not shown. View the raw message.

drivers/pci/intel-iommu.c +19 -4

Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================

Lines of this file are not shown. View the raw message.

include/linux/dmar.h +7 -0

Index: linux-2.6/include/linux/dmar.h
===================================================================

Lines of this file are not shown. View the raw message.

kernel/irq/manage.c +3 -3

Index: linux-2.6/kernel/irq/manage.c
===================================================================

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

Paul Mackerras

2008-11-13 22:19 UTC #46 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 2 lines · Andrew Morton

Other architectures want (or have) sparse interrupts. Are those guys paying attention here?

On powerpc we have a mapping from virtual irq numbers (in the range 0 to NR_IRQS-1) to physical irq numbers (which can be anything) and back again. I think our approach is simpler than what's being proposed here, though we don't try to keep the irqdescs node-local as this patch seems to (fortunately our big systems aren't so NUMA-ish as to make that necessary).

Paul.

David Miller

2008-11-13 22:23 UTC #47 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

From: Paul Mackerras <[email protected]>
Date: Fri, 14 Nov 2008 09:19:13 +1100

Quoted 11 lines

Quoted 2 lines · Andrew Morton
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
On powerpc we have a mapping from virtual irq numbers (in the range 0 to NR_IRQS-1) to physical irq numbers (which can be anything) and back again. I think our approach is simpler than what's being proposed here, though we don't try to keep the irqdescs node-local as this patch seems to (fortunately our big systems aren't so NUMA-ish as to make that necessary).

This is exactly what sparc64 does as well, same as powerpc, and as Paul said it's so much incredibly simpler than the dyn_irq stuff.

Mike Travis

2008-11-13 23:11 UTC #48 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 14 lines · David Miller

From: Paul Mackerras <[email protected]>
Date: Fri, 14 Nov 2008 09:19:13 +1100
Quoted 10 lines
Quoted 2 lines · Andrew Morton
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
On powerpc we have a mapping from virtual irq numbers (in the range 0 to NR_IRQS-1) to physical irq numbers (which can be anything) and back again. I think our approach is simpler than what's being proposed here, though we don't try to keep the irqdescs node-local as this patch seems to (fortunately our big systems aren't so NUMA-ish as to make that necessary).

Quoted 2 lines

This is exactly what sparc64 does as well, same as powerpc, and as Paul said it's so much incredibly simpler than the dyn_irq stuff.

One problem is that pre-defining a static NR_IRQ count is almost always wrong when the NR_CPUS count is large, and should be adjusted as resources require.

Large UV systems will take a performance hit from off-node accesses when the CPU count (or more likely the NODE count) reaches some threshold. So keeping as much interrupt context close to the interrupting source is a good thing.

David Miller

2008-11-13 23:14 UTC #49 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 15:11:29 -0800

Quoted 15 lines

Quoted 14 lines · David Miller
From: Paul Mackerras <[email protected]>
Date: Fri, 14 Nov 2008 09:19:13 +1100
Quoted 10 lines
Quoted 2 lines · Andrew Morton
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
On powerpc we have a mapping from virtual irq numbers (in the range 0 to NR_IRQS-1) to physical irq numbers (which can be anything) and back again. I think our approach is simpler than what's being proposed here, though we don't try to keep the irqdescs node-local as this patch seems to (fortunately our big systems aren't so NUMA-ish as to make that necessary).

Quoted 6 lines

Quoted 2 lines
This is exactly what sparc64 does as well, same as powerpc, and as Paul said it's so much incredibly simpler than the dyn_irq stuff.
One problem is that pre-defining a static NR_IRQ count is almost always wrong when the NR_CPUS count is large, and should be adjusted as resources require.

We use a value of 256 and I've been booting linux on 128 cpu sparc64 systems with lots of PCI-E host controllers (and others have booted it on even larger ones). All of which have several NUMA domains.

It's not an issue.

Quoted 4 lines

Large UV systems will take a performance hit from off-node accesses when the CPU count (or more likely the NODE count) reaches some threshold. So keeping as much interrupt context close to the interrupting source is a good thing.

Just because the same piece of information is repeated over and over again doesn't mean it really matters.

Mike Travis

2008-11-14 00:15 UTC #50 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 25 lines · David Miller

From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 15:11:29 -0800
Quoted 14 lines
Quoted 13 lines · David Miller
From: Paul Mackerras <[email protected]>
Date: Fri, 14 Nov 2008 09:19:13 +1100
Quoted 10 lines
Quoted 2 lines · Andrew Morton
Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
On powerpc we have a mapping from virtual irq numbers (in the range 0 to NR_IRQS-1) to physical irq numbers (which can be anything) and back again. I think our approach is simpler than what's being proposed here, though we don't try to keep the irqdescs node-local as this patch seems to (fortunately our big systems aren't so NUMA-ish as to make that necessary).
Quoted 7 lines
Quoted 2 lines
This is exactly what sparc64 does as well, same as powerpc, and as Paul said it's so much incredibly simpler than the dyn_irq stuff.
One problem is that pre-defining a static NR_IRQ count is almost always wrong when the NR_CPUS count is large, and should be adjusted as resources require.

Quoted 5 lines

We use a value of 256 and I've been booting linux on 128 cpu sparc64 systems with lots of PCI-E host controllers (and others have booted it on even larger ones). All of which have several NUMA domains.
It's not an issue.

Are you saying that having a fixed count of IRQ's is not an issue? With NR_CPUS=4096 what would you fix it to? (Currently it's NR_CPUS * 32 but that might not be sufficient.) Would NR_CPUS=16384 make it an issue?

Quoted 8 lines

Quoted 5 lines
Large UV systems will take a performance hit from off-node accesses when the CPU count (or more likely the NODE count) reaches some threshold. So keeping as much interrupt context close to the interrupting source is a good thing.
Just because the same piece of information is repeated over and over again doesn't mean it really matters.

Which information is repeated over and over? I was under the impression that each and every interrupt writes to the irq_desc entry for that irq? If this is in a big list on node 0, that is data passing over the system bus.

Or am I missing what you're getting at?

Thanks,
Mike

David Miller

2008-11-14 00:22 UTC #51 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 16:15:12 -0800

Quoted 8 lines

Quoted 7 lines · David Miller
From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 15:11:29 -0800
We use a value of 256 and I've been booting linux on 128 cpu sparc64 systems with lots of PCI-E host controllers (and others have booted it on even larger ones). All of which have several NUMA domains.

Quoted 5 lines

Quoted 1 line
It's not an issue.
Are you saying that having a fixed count of IRQ's is not an issue? With NR_CPUS=4096 what would you fix it to? (Currently it's NR_CPUS * 32 but that might not be sufficient.) Would NR_CPUS=16384 make it an issue?

Nope, and nope. I frequently run kernels with NR_CPUS set to huge values.

It seems that the issue of x86 is that it has it's IRQ count tied to the number of cpus, that's not very intelligent. Perhaps that part should be rearranged somehow?

Mike Travis

2008-11-14 00:40 UTC #52 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 16 lines · David Miller

From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 16:15:12 -0800
Quoted 4 lines
Quoted 3 lines · David Miller
From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 15:11:29 -0800
Quoted 8 lines
Quoted 5 lines
We use a value of 256 and I've been booting linux on 128 cpu sparc64 systems with lots of PCI-E host controllers (and others have booted it on even larger ones). All of which have several NUMA domains.
It's not an issue.
Are you saying that having a fixed count of IRQ's is not an issue? With NR_CPUS=4096 what would you fix it to? (Currently it's NR_CPUS * 32 but that might not be sufficient.) Would NR_CPUS=16384 make it an issue?

Quoted 6 lines

Nope, and nope. I frequently run kernels with NR_CPUS set to huge values.
It seems that the issue of x86 is that it has it's IRQ count tied to the number of cpus, that's not very intelligent. Perhaps that part should be rearranged somehow?

Yes, you're probably right but it is what it is. Most of the irq vectors have more to do with cpus than with i/o devices (the system vectors, ipi, kdb and gru [a uv thing] interrupt vectors come first to mind.) These do by necessity need to grow with NR_CPUS, if you're fixing the total IRQ count.

There's been a couple of different proposals to attempt to disassociate i/o and system vectors though even attempting to guess at the number of i/o devices is tricky. Every one of the 512 nodes on a UV system *may* have a number of i/o devices attached to them though practically this will be rare.

David Miller

2008-11-14 02:38 UTC #53 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

From: Mike Travis <[email protected]>
Date: Thu, 13 Nov 2008 16:39:51 -0800

Quoted 5 lines

There's been a couple of different proposals to attempt to disassociate i/o and system vectors though even attempting to guess at the number of i/o devices is tricky. Every one of the 512 nodes on a UV system *may* have a number of i/o devices attached to them though practically this will be rare.

Practicality is what really matters.

We can even make the constant number a config option, so that if someone has a system that actually triggers past this limit even dist vendors can simply bump the config option value.

Mike Travis

2008-11-14 03:06 UTC #54 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 5 lines · David Miller

Practicality is what really matters.
We can even make the constant number a config option, so that if someone has a system that actually triggers past this limit even dist vendors can simply bump the config option value.

Unfortunately the reality is that distros spend months certifying the base kernel for both application and security conformance. And many (most?) of the large system customers will only run this kernel. Even changing root startup options is considered invalidating the conformance testing. That means the kernel has to adapt to the resource needs of the system being serviced.

Ideally, linux shouldn't have any fixed resources. A system built to run on a massively parallel system should also boot and run on a smart phone (not well perhaps... ;-)

Benjamin Herrenschmidt

2008-11-16 21:00 UTC #55 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines

Other architectures want (or have) sparse interrupts. Are those guys paying attention here?
I don't have a clue what all this does. I hope those who will work on this code are sufficiently familiar with it all to be able to maintain it when there are close to zero comments in some of our most tricky and problem-prone code.

Indeed, I'm a bit scared by the patch as it is ...

On powerpc, we have a nice an simple virtual irq layer that also probably better fits the need of architecture that can have multiple PICs with different number space cascaded in various ways in a single machine too.

We provide a basic construct that defines an HW IRQ domain (we call it irq_host, it can have a 1:1 relationship to a HW irq_chip but not always), and when HW IRQs are discovered and made available to linux, they are "mapped" to virtual irq numbers by mapping them to a tuple (irq_hose, hw_number).

The HW numbering can be arbitrarily anything for that controller unrelated to the "linux" virtual number, so all the problems of making IRQ 0 invalid and reserving 1...15 for legacy 8259 etc... are taken care of.

For fast mapping of HW->Linux numbers, we provide various schemes that a given IRQ host implementation can choose (from no translation, some HW PICs or hypervisor can be directly given the linux number and pass it back to us) to linear mapping to a lockless radix tree for large sparse irq domains.

At this stage, I don't feel like changing to this sparse_irq which I haven't fully understood, is complicated, and doesn't seem to provide the features I need anyway.

The only thing that I see possibly of interest is getting rid of the irq_desc array itself, toward something more dynamically resizeable, in which case our powerpc remapping scheme could probably also benefit by also getting rid in a similar way of our equivalent array used for mapping the virq's to host,hw_number tuples.

The irq_desc per-cpu also seem to be an interesting idea.

In any case, the patch is too big and unless I missed something, totally under-documented in what it does, why it does it, how it does it, etc... with a bit more of that, I might take more time to look closely and find out that indeed it's something I can make use of, maybe with a few tweaks.

Cheers,
Ben.

Yinghai Lu

author

2008-11-16 23:45 UTC #56 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 6 lines · Benjamin Herrenschmidt

In any case, the patch is too big and unless I missed something, totally under-documented in what it does, why it does it, how it does it, etc... with a bit more of that, I might take more time to look closely and find out that indeed it's something I can make use of, maybe with a few tweaks.

will cut it to smaller ones. try to have one big one to make review more easy.

starting case
1. IBM x3950 have io apic pins > 224?
2. SGI big box need 8192 irqs

want to make same kernel with NR_CPUS = 4096 can be used with regular sized system.

points are
1. removing irq_desc array and NR_IRQS. for small system will have less wasting ram in that array, and for big system will get more irqs like system have 8 pcie cards and every card need 40 MSI vector or a lot of IO apic controllers etc.
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
3. irq_desc is going with /proc/irq/xxx/smp_affinity.

H. Peter Anvin

reviewer

2008-11-16 23:52 UTC #57 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 1 line · Yinghai Lu

2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.

2 is *STILL WRONG*, dammit!

You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!

-hpa

Yinghai Lu

author

2008-11-16 23:54 UTC #58 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · H. Peter Anvin

Quoted 1 line · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!

you want to u64 instead of unsigned int for irq?

H. Peter Anvin

reviewer

2008-11-17 00:03 UTC #59 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 11 lines · Yinghai Lu

Quoted 7 lines · H. Peter Anvin
Quoted 1 line · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!
you want to u64 instead of unsigned int for irq?

No, I think the whole notion of a static *numeric* identifier for an IRQ when it's something like MSI-X is simply pointless. I think we should assign IRQ numbers beyond the legacy range dynamically.

I really don't think anyone gives a hoot about the IRQ number for any IRQ above the 0-15 legacy range, even including the "APIC" numbers 16+.

-hpa

Yinghai Lu

author

2008-11-17 00:22 UTC #60 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 12 lines · H. Peter Anvin

Quoted 10 lines · Yinghai Lu
Quoted 7 lines · H. Peter Anvin
Quoted 1 line · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!
you want to u64 instead of unsigned int for irq?

Quoted 6 lines

No, I think the whole notion of a static *numeric* identifier for an IRQ when it's something like MSI-X is simply pointless. I think we should assign IRQ numbers beyond the legacy range dynamically.
I really don't think anyone gives a hoot about the IRQ number for any IRQ above the 0-15 legacy range, even including the "APIC" numbers 16+.

you want to change ioapic/pin to irq mapping too?

so INTx and MSI will call create_irq_nr to get one irq for 16, and following first come and first serve rule.

H. Peter Anvin

reviewer

2008-11-17 00:30 UTC #61 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 7 lines · Yinghai Lu

Quoted 3 lines
I really don't think anyone gives a hoot about the IRQ number for any IRQ above the 0-15 legacy range, even including the "APIC" numbers 16+.
you want to change ioapic/pin to irq mapping too?
so INTx and MSI will call create_irq_nr to get one irq for 16, and following first come and first serve rule.

I personally don't think there is any issue with changing ioapic/pin to IRQ mapping. Other people may disagree. My opinion is that IRQ numbers 16-23 are somewhat useful when you're dealing with a single IOAPIC, but after that it's all a blur.

It would, however, be a good idea if IOAPICs had their numbers assigned at detection time, as opposed to when the interrupt is registered, thus making it a stable number for a single boot, at least. The same is probably true for MSI(-X); we could assign it a range of numbers when the device is enumerated (as opposed to when a driver is activated), but I don't know to what extent that is likely to cause more troubles than it solves.

-hpa

Yinghai Lu

author

2008-11-17 00:37 UTC #62 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · H. Peter Anvin

It would, however, be a good idea if IOAPICs had their numbers assigned at detection time, as opposed to when the interrupt is registered, thus making it a stable number for a single boot, at least. The same is probably true for MSI(-X); we could assign it a range of numbers when the device is enumerated (as opposed to when a driver is activated), but I don't know to what extent that is likely to cause more troubles than it solves.

how to find the range for MSIX, one device/func may need a lot. for example, niu driver could use 20 MSI-X for one port. (one dev/func) some could use 256. we only know that when driver is loaded.
So as Eric said, just try to use 12bits (4k range) for them.

H. Peter Anvin

reviewer

2008-11-17 00:52 UTC #63 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 9 lines · Yinghai Lu

Quoted 7 lines · H. Peter Anvin
It would, however, be a good idea if IOAPICs had their numbers assigned at detection time, as opposed to when the interrupt is registered, thus making it a stable number for a single boot, at least. The same is probably true for MSI(-X); we could assign it a range of numbers when the device is enumerated (as opposed to when a driver is activated), but I don't know to what extent that is likely to cause more troubles than it solves.

Quoted 4 lines

how to find the range for MSIX, one device/func may need a lot. for example, niu driver could use 20 MSI-X for one port. (one dev/func) some could use 256. we only know that when driver is loaded.
So as Eric said, just try to use 12bits (4k range) for them.

You can know how many vectors are exported in generic code. However, using 4k per should be fine.

-hpa

Yinghai Lu

author

2008-11-17 00:58 UTC #64 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 9 lines · H. Peter Anvin

Quoted 8 lines · Yinghai Lu
Quoted 7 lines · H. Peter Anvin
It would, however, be a good idea if IOAPICs had their numbers assigned at detection time, as opposed to when the interrupt is registered, thus making it a stable number for a single boot, at least. The same is probably true for MSI(-X); we could assign it a range of numbers when the device is enumerated (as opposed to when a driver is activated), but I don't know to what extent that is likely to cause more troubles than it solves.

Quoted 7 lines

Quoted 4 lines
how to find the range for MSIX, one device/func may need a lot. for example, niu driver could use 20 MSI-X for one port. (one dev/func) some could use 256. we only know that when driver is loaded.
So as Eric said, just try to use 12bits (4k range) for them.
You can know how many vectors are exported in generic code. However, using 4k per should be fine.

so we have one list to map domain/bus/dev/func to bits [31,12] in irq ?

H. Peter Anvin

reviewer

2008-11-17 01:03 UTC #65 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 3 lines · Yinghai Lu

so we have one list to map domain/bus/dev/func to bits [31,12] in irq ?

That works, although having a more generic allocation mechanism which isn't so tied to MSI-X would make more sense.

-hpa

Mike Travis

2008-11-17 02:03 UTC #66 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · H. Peter Anvin

Quoted 2 lines · Yinghai Lu
so we have one list to map domain/bus/dev/func to bits [31,12] in irq ?
That works, although having a more generic allocation mechanism which isn't so tied to MSI-X would make more sense.
-hpa

Hmm, I see I picked up this conversation quite late. I would agree that trying to map any set of fixed bit fields to domain/bus/device/function would be problematic (esp. since I don't understand how MSI's figure into the deal.)

Is there some reason why we need that?

Thanks,
Mike

Benjamin Herrenschmidt

2008-11-17 04:28 UTC #67 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 7 lines · H. Peter Anvin

Quoted 3 lines · Yinghai Lu
so we have one list to map domain/bus/dev/func to bits [31,12] in irq ?
That works, although having a more generic allocation mechanism which isn't so tied to MSI-X would make more sense.

None of that should be related to the linux interrupt number.

Ben.

Benjamin Herrenschmidt

2008-11-17 04:27 UTC #68 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 7 lines · H. Peter Anvin

It would, however, be a good idea if IOAPICs had their numbers assigned at detection time, as opposed to when the interrupt is registered, thus making it a stable number for a single boot, at least. The same is probably true for MSI(-X); we could assign it a range of numbers when the device is enumerated (as opposed to when a driver is activated), but I don't know to what extent that is likely to cause more troubles than it solves.

On powerpc, we establish the mapping at PCI probe time so it's reasonably stable. In fact, we even try to use virq == hwirq number if the hwirq number fits and is available though I'm tempted to remove that "feature" as it can make things more confusing in the end with some interrupts having matching HW numbers and some not.

Cheers,
Ben.

Jeremy Fitzhardinge

2008-11-17 20:25 UTC #69 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 23 lines · Yinghai Lu

Quoted 21 lines · H. Peter Anvin
Quoted 6 lines · Yinghai Lu
Quoted 4 lines · H. Peter Anvin
Quoted 2 lines · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
Quoted 9 lines
Quoted 6 lines
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!
you want to u64 instead of unsigned int for irq?
No, I think the whole notion of a static *numeric* identifier for an IRQ when it's something like MSI-X is simply pointless. I think we should assign IRQ numbers beyond the legacy range dynamically.

Quoted 6 lines

Quoted 3 lines
I really don't think anyone gives a hoot about the IRQ number for any IRQ above the 0-15 legacy range, even including the "APIC" numbers 16+.
you want to change ioapic/pin to irq mapping too?

I would like to see that. I'm already doing this in the Xen dom0 code interrupt code that I posted the other day.

Benjamin Herrenschmidt

2008-11-17 04:26 UTC #70 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 4 lines · H. Peter Anvin

No, I think the whole notion of a static *numeric* identifier for an IRQ when it's something like MSI-X is simply pointless. I think we should assign IRQ numbers beyond the legacy range dynamically.

Yup, exactly. Which is what we do on other platforms :-)

I think there is some value in getting rid of the irq_desc static array, and to a certain extend having the ability to have irq_desc's be per-cpu allocated but I think that patch tries to mix up way too many different things, including a dubious attempt at tying the interrupt subsystem into a specific implementation choice of x86 platforms for numbering.

Linux interrupts should just be a dynamically allocated number space, with an exception for the 16 first ones (0 = illegal, 1...15 = legacy) and that should be -separate- from the actual HW number of one on a given PIC. In fact, powerpc handles multiple HW interrupt domain numbers just fine that way which is very useful for embedded platforms with funky cascaded PIC setups..

Ben.

Benjamin Herrenschmidt

2008-11-17 04:23 UTC #71 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 6 lines

Quoted 4 lines
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!
you want to u64 instead of unsigned int for irq?

No, separate HW number from linux logic number. The later can remain u32 or even u16 if you want :-)

Ben.

Mike Travis

2008-11-17 01:51 UTC #72 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 10 lines · H. Peter Anvin

Quoted 1 line · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!
-hpa

Hmm, I was going to bring this up as well... ;-)

X was changed quite a while ago to domain/bus/dev/func, which was a lot easier to deal with than trying to "offset" the bus with domain * some large number + bus. Currently max nodes is 512 so it deserves it's own field. ;-)

Btw, are you suggesting combining device and function? That might be inadvertently squeezing something that shouldn't be.

Thanks,
Mike

H. Peter Anvin

reviewer

2008-11-17 04:43 UTC #73 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · Mike Travis

X was changed quite a while ago to domain/bus/dev/func, which was a lot easier to deal with than trying to "offset" the bus with domain * some large number + bus. Currently max nodes is 512 so it deserves it's own field. ;-)
Btw, are you suggesting combining device and function? That might be inadvertently squeezing something that shouldn't be.

devfn = (device << 3) + function

It's a standard construct inside Linux.

-hpa

Benjamin Herrenschmidt

2008-11-17 04:23 UTC #74 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · H. Peter Anvin

Quoted 1 line · Yinghai Lu
2. make irq number is bus/devfn/idx, and every dev func will use 12bit range, irq number is relatively fixed not like current MSI irq creating is some kind of floating from NR_IRQS too.
2 is *STILL WRONG*, dammit!
You keep bringing this one up, but our PCI addressing is *DOMAIN*/bus/devfn -- it falls flat on its face when you have more than 16 PCI domains. CAN WE PLEASE STOP WITH THIS FOOLISHNESS NOW!

Besides, the relationship between PCI location and IRQ numbers seems to be an x86 thingy ... don't bring that into the generic code please !

IRQ numbers are arbitrary, some platforms make up numbers out of the blue, or they can be hypervisor internal tokens etc...

The only sane way to handle this generically IMHO is to do what we do on powerpc (and I think sparc64) which is to totally disconnect the HW number from the "linux" number.

Ben.

H. Peter Anvin

reviewer

2008-11-17 04:46 UTC #75 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 8 lines · Benjamin Herrenschmidt

IRQ numbers are arbitrary, some platforms make up numbers out of the blue, or they can be hypervisor internal tokens etc...
The only sane way to handle this generically IMHO is to do what we do on powerpc (and I think sparc64) which is to totally disconnect the HW number from the "linux" number.

Yes, that's what I want to see, too. On x86, it's important to preserve the first 16 (BIOS-compatible, XT-PIC) numbers, as they are widely used as a user interface, but for the rest, there is no point.

It is probably desirable to do that by overlaying the first (primary, south bridge) IO-APIC, which also takes care of the "semi-legacy" IRQ 16-23 numbers.

-hpa

Benjamin Herrenschmidt

2008-11-17 06:53 UTC #76 lore raw

Re: [PATCH] sparse_irq aka dyn_irq v13

Quoted 5 lines · H. Peter Anvin

Quoted 4 lines · Benjamin Herrenschmidt
IRQ numbers are arbitrary, some platforms make up numbers out of the blue, or they can be hypervisor internal tokens etc...

Quoted 8 lines

Quoted 4 lines
The only sane way to handle this generically IMHO is to do what we do on powerpc (and I think sparc64) which is to totally disconnect the HW number from the "linux" number.
Yes, that's what I want to see, too. On x86, it's important to preserve the first 16 (BIOS-compatible, XT-PIC) numbers, as they are widely used as a user interface, but for the rest, there is no point.

Right, and I do that on powerpc by reserving those numbers so they get automatically assigned to and only to the PIC that comes up as claiming the legacy number space (if any, if none, then they remain unassigned to avoid problems with old crappy modules hard coding IRQ numbers and trying to request them).

Quoted 3 lines

It is probably desirable to do that by overlaying the first (primary, south bridge) IO-APIC, which also takes care of the "semi-legacy" IRQ 16-23 numbers.

Would make sense indeed to treat those numbers specifically.

Ben.

H. Peter Anvin

reviewer

2008-11-09 08:36 UTC #77 lore raw

In reply to Ingo Molnar

Quoted 12 lines · Ingo Molnar

Quoted 12 lines

+++ linux-2.6/arch/x86/mm/init_32.c

arch/x86/mm/init_32.c +1 -0

@@ -66,6 +66,7 @@ static unsigned long __meminitdata table
 static unsigned long __meminitdata table_top;
 
 static int __initdata after_init_bootmem;
+int after_bootmem;
 
 static __init void *alloc_low_page(unsigned long *phys)
 {

@@ -987,6 +988,8 @@ void __init mem_init(void)

Quoted 7 lines

arch/x86/mm/init_32.c void __init mem_init(void) · line 991
Quoted 3 lines
arch/x86/mm/init_32.c +1 -0
 	set_highmem_pages_init();
 
+	after_bootmem = 1;
this hack can go away once we have a proper percpu_alloc() that can be used early enough.

Also, flags should be "bool". We're not aggressively going after old code to convert it, but new code should use "bool".

-hpa

Cyrill Gorcunov

2008-11-09 07:50 UTC #78 lore raw