lkml.org 
[lkml]   [2009]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] irq: optimize init_kstat_irqs/init_copy_kstat_irqs
From
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


\
 
 \ /
  Last update: 2009-02-07 10:03    [W:1.637 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site