Messages in this thread | | | Date | Tue, 24 Jan 2006 09:33:00 +0100 | From | "Jan Beulich" <> | Subject | Re: [PATCH] tvec_bases too large for per-cpu data |
| |
>> >Did you consider using alloc_percpu()? >> >> I did, but I saw drawbacks with that (most notably the fact that all instances are allocated at >> once, possibly wasting a lot of memory). > >It's 4k for each cpu which is in the possible_map but which will never be >brought online. I don't think that'll be a lot of memory - are there >machines which have a lot of possible-but-not-really-there CPUs?
I would suppose so. Why wouldn't a machine supporting CPU hotplug not reasonably be able to double, triple, etc the number of CPUs originally present?
>There _must_ be ordering issues. Otherwise we'd just dynamically allocate >all the structs up-front and be done with it. > >Presumably the ordering issue is that init_timers() is called before >kmem_cache_init(). That's non-obvious and should be commented.
That I can easily do, sure.
>- The `#ifdef CONFIG_NUMA' in init_timers_cpu() seems to be unnecessary - > kmalloc_node() will use kmalloc() if !NUMA.
That is correct, but I wanted the fallback if kmalloc_node() fails (from briefly looking at that code it didn't seem like it would do such fallback itself). And calling kmalloc() twice if !NUMA seemed pointless.
>- The likely()s in init_timers_cpu() seems fairly pointless - it's not a > fastpath.
OK, will change that.
>- We prefer to do this: > > if (expr) { > ... > } else { > ... > } > > and not > > if (expr) { > ... > } > else { > ... > }
I can change that, too, but I don't see why this gets pointed out again and again when there really is no consistency across the entire kernel...
Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |