Re: [PATCH 1/2] irq: sparseirq enabling v2

[Yinghai Lu <yinghai@kernel.org>]

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.

also if we need move_irq_desc between node?

>

 ...

> 
>> +#ifdef CONFIG_SPARSE_IRQ
>> +	for (new = irq_want; new < NR_IRQS; new++)
>> +#else
>> +	for (new = irq_want; new > 0; new--)
>> +#endif
> 
> this assymetry seems unnecessary too now i think.
> 
>> +#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.
...
> 
> 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

YH