At 10:27 11/06/02, Andrew Morton wrote:>Rusty Russell wrote:> >> > ...> > Let's not perpetuate the myth that everything in the kernel needs to> > be tuned to the last cycle at all costs, hm?>>I was more concerned about the RAM use, actually.>>This patch is an additional reason for CONFIG_NR_CPUS, but I've rather>gone cold on that idea because the "proper fix" is to make all those>huge per-cpu arrays dynamically allocated. So you can run a 64p kernel>on 2p without losing hundreds of k of memory and kernel address space.>>But it looks like all those dynamically-allocated structures would>have to be allocated out to NR_CPUS anyway, to support hotplug, yes?>>In which case, CONFIG_NR_CPUS is the only way to get the memory>back...

Why? You can get rid of all uses of NR_CPUS (except for using it as a max capping value so none goes above it) and always use smp_num_cpus instead. And make the cpu hotplug code update smp_num_cpus as appropriate.

All code relying on smp_num_cpus for per-cpu buffers can do a check whether the current cpu is greater than the value of smp_num_cpus at per-cpu buffer allocation time and if so lock the kernel (or only the buffers if possible) and grow the buffer allocation up to the new smp_num_cpus value. And all that can be done nicely out of line in a really, really, snail speed slow path... The fastpath only needs to contain:

cpu = smp_processor_id();#ifdef CONFIG_HOTPLUG_CPU if (unlikely(cpu >= old_smp_num_cpus)) goto snail_path; snail_path_done:#endifSo zero penalty for non-hotplug users and loads of penalty for hotplug users but frankly I couldn't care less for those. The slow path will trigger so seldom it is not worth thinking about the performance hit there.

You could even make the above look nicer by making it a function like:

And let our_hotplug_callback() deal with the case where cpu is >= old_smp_num_cpus, for example for ntfs that would involve extending the number of per-cpu buffers. And in the !CONFIG_HOTPLUG_CPU case the whole check_for_cpu_hotplug_event function becomes a NOP. All in the spirit of not having #ifdefs sprinkled around the code.

There are a lot of ways to deal with this corner case dynamically, so please use one of them. I don't buy the "lets penalise 99% of users for the sake of a feature that almost noone will ever use" argument.