Messages in this thread | | | Date | Wed, 26 Nov 2008 09:17:24 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] irq: sparseirq enabling v2 |
| |
* Yinghai Lu <yinghai@kernel.org> wrote:
> Ingo Molnar wrote: > > * Yinghai Lu <yinghai@kernel.org> wrote: > > > >> impact: new feature sparseirq > > > >> v2: use pointer array instead of hash > > > > ok, this looks pretty good! > > > > A few details: > > > >> +#ifdef CONFIG_SPARSE_IRQ > >> +#define set_ioapic_affinity_irq set_ioapic_affinity_irq_desc > >> +#else > >> +static void set_ioapic_affinity_irq(unsigned int irq, cpumask_t mask) > >> +{ > >> + struct irq_desc *desc; > >> + > >> + desc = irq_to_desc(irq); > >> + > >> + set_ioapic_affinity_irq_desc(desc, mask); > >> +} > >> +#endif > > > > i think this distinction can now go away?. > > i may miss sth in your previous mail. you said we may put full hash > back later, so need keep those change to avoid lookup costs later.
full hash as in no NR_IRQS limit?
I dont think that's a good idea even in the far future: we can always make NR_IRQS large enough, and turn the current array-of-pointers into array-of-arrays-of-pointers scheme or an outright hash - but without losing the fundamental abstraction that it's a 0..NR_IRQS array (just accessed abstractly).
> also if we need move_irq_desc between node?
i'm not sure i understand. Do you mean we should deallocate and reallocate the irq_desc to another node?
> >> +#ifdef CONFIG_SPARSE_IRQ > >> + irq_want = nr_irqs; > >> +#else > >> + irq_want = NR_IRQS - 1; > >> +#endif > > > > ditto. I think we dont want 'nr_irqs' anymore - just remain with > > NR_IRQS, right? > > > > nr_irqs is the total GSI number when sparseirq is used. so MSI irq > will start from that. ...
ah, okay. I think we should renumber MSIs to the scheme Thomas suggested, independently of CONFIG_SPARSE_IRQ.
CONFIG_SPARSE_IRQ should _only_ impact the irq_desc[] data structure: it turns from a flat C array of irq_desc into an array of irq_desc pointers.
No other semantic changes. Possibly all hidden away completely in the irq-desc-array accessors.
> > Plus in a few more places. > > > > Please look at _every_ #ifdef or #if in your patch in a .c file > > and ask the question: can we somehow in some way eliminate it and > > convert it to some nice inline somewhere or eliminate it via some > > other trick? > > will
thanks!
Ingo
| |