Messages in this thread | | | Date | Sat, 7 Feb 2009 01:01:03 -0800 | Subject | Re: [PATCH] irq: optimize init_kstat_irqs/init_copy_kstat_irqs | From | Yinghai Lu <> |
| |
On Sat, Feb 7, 2009 at 12:26 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 06 Feb 2009 23:49:41 -0800 Yinghai Lu <yinghai@kernel.org> wrote: > >> >> Impact: even later type of kstat_irqs is changed >> >> simplify and make init_kstat_irqs etc more type prof according to Andrew >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> >> --- >> kernel/irq/handle.c | 20 +++++++++++--------- >> kernel/irq/numa_migrate.c | 11 +++-------- >> 2 files changed, 14 insertions(+), 17 deletions(-) >> >> Index: linux-2.6/kernel/irq/handle.c >> =================================================================== >> --- linux-2.6.orig/kernel/irq/handle.c >> +++ linux-2.6/kernel/irq/handle.c >> @@ -82,19 +82,21 @@ static struct irq_desc irq_desc_init = { >> >> void init_kstat_irqs(struct irq_desc *desc, int cpu, int nr) >> { >> - unsigned long bytes; >> - char *ptr; >> int node; >> - >> - /* Compute how many bytes we need per irq and allocate them */ >> - bytes = nr * sizeof(unsigned int); >> + void *ptr; >> >> node = cpu_to_node(cpu); >> - ptr = kzalloc_node(bytes, GFP_ATOMIC, node); >> - printk(KERN_DEBUG " alloc kstat_irqs on cpu %d node %d\n", cpu, node); >> + ptr = kzalloc_node(nr * sizeof(*desc->kstat_irqs), GFP_ATOMIC, node); >> >> - if (ptr) >> - desc->kstat_irqs = (unsigned int *)ptr; >> + /* >> + * don't overwite if can not get new one >> + * init_copy_kstat_irqs() could still use old one >> + */ >> + if (ptr) { >> + printk(KERN_DEBUG " alloc kstat_irqs on cpu %d node %d\n", >> + cpu, node); >> + desc->kstat_irqs = ptr; >> + } > > else > init_one_irq_desc() goes BUG. > >> } > > Sorry, but it's just not acceptable for irq_to_desc_alloc_cpu() and > init_one_irq_desc() to go BUG if a GFP_ATOMIC allocation attempt > failed. > > If this code is only called on kernel boot then OK, that's acceptable. > But if that is the case then all this code should be marked > __init/__initdata. And ack_apic_edge() sure doesn't look like a > boot-time-only function.
add kzalloc_node_safe()?
> > This is basic stuff which even a cursory review should have picked up. > > Also, please go through all this code and convert WARN_ON(1) into > WARN(), and convert BUG_ON(1) into BUG().
sure, will do it.
YH
| |