(Resending with expanded cc list.)
Several drivers (e.g., mlx4_core) do something similar to:
err = pci_enable_msix(pdev, entries, num_possible_cpus());
which takes us down this code path:
pci_enable_msix
native_setup_msi_irqs
create_irq_nr
__assign_irq_vector
__assign_irq_vector() preferentially uses vectors from low-numbered
CPUs. On a system with a large number (>256) CPUs this can result in
a CPU running out of vectors, and subsequent attempts to assign an
interrupt to that CPU will fail.
The following patch prefers vectors from the node associated with the
device (if the device is associated with a node). This should make it
far less likely that a single CPU's vectors will be exhausted.
Signed-off-by: Arthur Kepner <[email protected]>
---
io_apic.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 7cc0a72..af5f9d8 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1117,6 +1117,49 @@ next:
return err;
}
+static int
+__assign_irq_vector_node(int irq, struct irq_cfg *cfg,
+ const struct cpumask *mask, int node)
+{
+ int err = -EAGAIN;
+ int cpu, best_cpu = -1, min_vector_count = NR_VECTORS;
+
+ for_each_cpu_and(cpu, cpumask_of_node(node), mask) {
+ /* find the 'best' CPU to take this vector -
+ * the one with the fewest assigned vectors is
+ * considered 'best' */
+ int i, vector_count = 0;
+
+ if (!cpu_online(cpu))
+ continue;
+
+ for (i = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
+ i < NR_VECTORS ; i++)
+ if (per_cpu(vector_irq, cpu)[i] != -1)
+ vector_count++;
+
+ if (vector_count < min_vector_count) {
+ min_vector_count = vector_count;
+ best_cpu = cpu;
+ }
+ }
+
+ if (best_cpu >= 0) {
+ cpumask_var_t tmp_mask;
+
+ if (!alloc_cpumask_var(&tmp_mask, GFP_ATOMIC))
+ return -ENOMEM;
+
+ cpumask_clear(tmp_mask);
+ cpumask_set_cpu(best_cpu, tmp_mask);
+ err = __assign_irq_vector(irq, cfg, tmp_mask);
+
+ free_cpumask_var(tmp_mask);
+ }
+
+ return err;
+}
+
int assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
{
int err;
@@ -1128,6 +1171,39 @@ int assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
return err;
}
+static int
+assign_irq_vector_node(int irq, struct irq_cfg *cfg,
+ const struct cpumask *mask, int node)
+{
+ int err;
+ unsigned long flags;
+
+ if (node == NUMA_NO_NODE)
+ return assign_irq_vector(irq, cfg, mask);
+
+ raw_spin_lock_irqsave(&vector_lock, flags);
+ err = __assign_irq_vector_node(irq, cfg, mask, node);
+ raw_spin_unlock_irqrestore(&vector_lock, flags);
+
+ if (err != 0)
+ /* uh oh - try again w/o specifying a node */
+ return assign_irq_vector(irq, cfg, mask);
+ else {
+ /* and set the affinity mask so that only
+ * CPUs on 'node' will be used */
+ struct irq_desc *desc = irq_to_desc(irq);
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&desc->lock, flags);
+ cpumask_and(desc->irq_data.affinity, cpu_online_mask,
+ cpumask_of_node(node));
+ desc->status |= IRQ_AFFINITY_SET;
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+ }
+
+ return err;
+}
+
static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
{
int cpu, vector;
@@ -3057,7 +3133,6 @@ device_initcall(ioapic_init_sysfs);
unsigned int create_irq_nr(unsigned int from, int node)
{
struct irq_cfg *cfg;
- unsigned long flags;
unsigned int ret = 0;
int irq;
@@ -3073,10 +3148,8 @@ unsigned int create_irq_nr(unsigned int from, int node)
return 0;
}
- raw_spin_lock_irqsave(&vector_lock, flags);
- if (!__assign_irq_vector(irq, cfg, apic->target_cpus()))
+ if (!assign_irq_vector_node(irq, cfg, apic->target_cpus(), node))
ret = irq;
- raw_spin_unlock_irqrestore(&vector_lock, flags);
if (ret) {
set_irq_chip_data(irq, cfg);
On Thu, 9 Dec 2010, Arthur Kepner wrote:
>
> (Resending with expanded cc list.)
>
> Several drivers (e.g., mlx4_core) do something similar to:
>
> err = pci_enable_msix(pdev, entries, num_possible_cpus());
>
> which takes us down this code path:
>
> pci_enable_msix
> native_setup_msi_irqs
> create_irq_nr
> __assign_irq_vector
>
> __assign_irq_vector() preferentially uses vectors from low-numbered
> CPUs. On a system with a large number (>256) CPUs this can result in
> a CPU running out of vectors, and subsequent attempts to assign an
> interrupt to that CPU will fail.
>
> The following patch prefers vectors from the node associated with the
> device (if the device is associated with a node). This should make it
> far less likely that a single CPU's vectors will be exhausted.
>
I'm not going to pretend that I know this code *at all*, but what you
wrote made me think, and I want to share my thoughts. Perhaps they are
useful, perhaps not.
Assigning to the CPU associated with a device sounds sane and sounds like
it will distribute things more. So far so good. But I can't help wondering
if it wouldn't be sane (besides doing this) to simply fall back to the
next higher CPU if the chosen one is exhausted (or wrap around to the
first one if we are already at the highest one)... So that we'll only fail
completely if *all* CPU's are exhausted..?
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
On Thu, 9 Dec 2010, Arthur Kepner wrote:
It's in my list of patches to go through already.
> (Resending with expanded cc list.)
FYI, you expanded the cc list just in your mail client. In reality
exactly by zero. [email protected] is a mail exploder to hpa, mingo and
tglx :)
Thanks,
tglx