Re: [PATCH] x86_64: (NEW) Dynamically allocate arch specific system vectors

[Eric W. Biederman]

Alan Mayer <ajm@xxxxxxx> writes:

> Okay, here it is as an attachment.  I think my email client is munging it.
> I haven't been able to fix it, apparently.
>
> I, too, would like to know what Eric thinks.

I think arch/x86 is about to fall over from accidental complexity of
the irq handling.  Looking at your problem and the problem of killing
NR_IRQS I spent way to much time playing with it this weekend then
I should have, but I think I have found a path that works and is
fairly easily verifiable.

The short version is we make vector_irq the one repository of knowledge
about what we are doing with vectors.

We create a common factor of assign_irq_vector that looks something like:

bool __grab_irq_vector(struct irq_desc *desc, unsigned vector, cpumask_t new_domain)
{
        /* Must be called with vector lock */
        struct irq_cfg *cfg;
        bool grabbed = false;
        unsigned int old_vector;
        cpumask_t mask;
        int cpu;

        cfg = get_irqp_cfg(irq);
        old_vector = cfg->vector;
        cpus_and(mask, new_domain, cpu_online_map);

        for_each_cpu_mask_nr(cpu, mask) {
		if (per_cpu(vector_irq, cpu)[vector])
                	goto out;
        }
        /* Available reserve it */
        for_each_cpu_mask_nr(cpu, mask)
  	      per_cpu(vector_irq, cpu)[vector] = desc;
        if (cfg->vector) {
        	cfg->move_in_progress;
                cfg->old_domain = cfg->domain;
        }
        cfg->vector = vector;
        cfg->domain = mask;
        grabbed = true;
        
out:
        return grabbed;
}

Then in your allocator for per cpu irqs you can do:
spin_lock(&vector_lock);
for (vector = FIRST_VECTOR; vector != LAST_VECTOR, vector--) {
	if (__grab_irq_vector(desc, CPU_MASK_ALL))
        	goto found;
}
spin_unlock(&vector_lock);

Although I am not at all convinced that dynamic allocation of
the vector number (instead of statically reserving it makes sense).
The only way I can see to guarantee all of the special is to
statically allocate them with a lot of good comments.  I think
the introduction of system_vectors quite likely defeated the
errata work around we have the lapic timer in a separate priority.

Still if we go in for dynamic allocation of the system vectors 
the above looks much simpler and easier to work with than
a lot of other possibilities.

I think used_vectors and system_vectors are data structures that
we need to remove, as their interactions with assign_irq_vector
are not at all well defined or nice.

I think vector_irq should return an irq_desc and have an entry for
all of the static vectors as well (if we are going to do weird
things with dynamic high priority vector allocation, and dynamic
detection of which vectors assign_irq_vector may use).

I have a patch series that gets me 90% of the way there, and the
rest appears easy but I don't have any time to mess with it right
now.  I will try and post it something in the next couple of days.

Eric


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/