Messages in this thread | | | Subject | Re: [PATCH 0/8] tip related: radix tree for spareseirq and logical flat clean up | From | (Eric W. Biederman) | Date | Sat, 13 Feb 2010 04:26:01 -0800 |
| |
Yinghai Lu <yinghai@kernel.org> writes:
> On 02/12/2010 07:44 PM, Eric W. Biederman wrote: >> Yinghai Lu <yinghai@kernel.org> writes: >> >>> ---------spareirq radix tree related ---------------- >>> 94007e8: irq: remove not need bootmem code >>> 4b0d3fa: radix: move radix init early >>> 56af1a9: sparseirq: change irq_desc_ptrs to static >>> b236235: sparseirq: use radix_tree instead of ptrs array >>> 5918787: x86: remove arch_probe_nr_irqs >>> >>> so could reduce nr_irqs limitation for bunch ixgbe... >>> >>> ---------------x86 logical flat related ----------- >>> f5954c4: use nr_cpus= to set nr_cpu_ids early >>> 7b8d6a9: x86: use num_processors for possible cpus >>> d79d1de: x86: make 32bit apic flat to physflat switch like 64bit >> >> Thanks for keeping this work alive. >> >> I just skimmed through do_IRQ and I happened to notice that >> we have an unnecessary inefficiency that using a radix tree for >> irq_to_desc will magnify. >> >> handle_irq should take an struct irq_desc * instead of a unsigned int irq. >> >> and the per cpu vector_irq array should become a per cpu vector_desc array. >> >> As soon as irq_to_desc is more than &irq_desc[irq] this saves us work >> and cache line misses at the cost of a simple code cleanup. >> > > please check > > Subject: [PATCH] x86: use vector_desc instead of vector_irq > > Eric pointed out that radix tree version of irq_to_desc will magnify delay on the path > of handle_irq. > use vector_desc to reduce the calling of irq_to_desc. > > next step: need to change all ack, mask, umask, eoi for all irq_chip to take irq_desc
Overall this looks good, and your next step sounds good.
Thanks, Eric
> Index: linux-2.6/arch/x86/kernel/irq_32.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/irq_32.c > +++ linux-2.6/arch/x86/kernel/irq_32.c > @@ -192,17 +192,17 @@ static inline int > execute_on_irq_stack(int overflow, struct irq_desc *desc, int irq) { return 0; } > #endif > > -bool handle_irq(unsigned irq, struct pt_regs *regs) > +bool handle_irq(struct irq_desc *desc, struct pt_regs *regs) > { > - struct irq_desc *desc; > int overflow; > + int irq;
This should be: unsigned irq;
irq numbers are unsigned, and with sparse allocation we might even see irq numbers large enough for that to matter.
> overflow = check_stack_overflow(); > > - desc = irq_to_desc(irq); > if (unlikely(!desc)) > return false; > > + irq = desc->irq; > if (!execute_on_irq_stack(overflow, desc, irq)) { > if (unlikely(overflow)) > print_stack_overflow();
> Index: linux-2.6/arch/x86/kernel/irqinit.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/irqinit.c > +++ linux-2.6/arch/x86/kernel/irqinit.c > @@ -83,16 +83,14 @@ static struct irqaction irq2 = { > .name = "cascade", > }; > > -DEFINE_PER_CPU(vector_irq_t, vector_irq) = { > - [0 ... NR_VECTORS - 1] = -1, > -}; > +DEFINE_PER_CPU(vector_desc_t, vector_desc); > > int vector_used_by_percpu_irq(unsigned int vector) > { > int cpu; > > for_each_online_cpu(cpu) { > - if (per_cpu(vector_irq, cpu)[vector] != -1) > + if (per_cpu(vector_desc, cpu)[vector] != NULL) > return 1; > } > > @@ -139,7 +137,7 @@ void __init init_IRQ(void) > * irq's migrate etc. > */ > for (i = 0; i < nr_legacy_irqs; i++) > - per_cpu(vector_irq, 0)[IRQ0_VECTOR + i] = i; > + per_cpu(vector_desc, 0)[IRQ0_VECTOR + i] = irq_to_desc(i);
I am not familiar with this hunk (it must be in the x86 tree). Are you certain we have allocated the legacy irq desc here?
> x86_init.irqs.intr_init(); > }
| |