Messages in this thread | | | Date | Tue, 1 Feb 2011 16:14:55 +0100 | From | Hans Rosenfeld <> | Subject | Re: [PATCH 4/4] x86, amd: Support L3 Cache Partitioning on AMD family 0x15 CPUs |
| |
On Thu, Jan 27, 2011 at 07:47:56AM -0500, Ingo Molnar wrote: > > The problem is that cpuinfo_x86.compute_unit_id etc. don't exist unless > > CONFIG_SMP is enabled. I don't think there is any reason why this should > > be that way, but changing this just for this particular L3 feature seems > > too intrusive. Do you really want me to do that? > > All the CONFIG_X86_HT #ifdefs in arch/x86/kernel/cpu/amd.c look pretty ugly too - > and it's not really a properly modularized solution. > > We generally want to unify the SMP and UP kernels as much as possible. 'CONFIG_SMP' > is not really a property of the hardware, it's a property of the software. > > If some topology information should be excluded then it can already be done by > turning off CONFIG_CPU_SUP_AMD under CONFIG_EXPERT.
I see several solutions to resolve this issue:
1. Remove #ifdef CONFIG_SMP around compute_unit_id in struct cpuinfo_x86 and then use my original patch. This would work without introducing new #ifdef ugliness with the L3 cache partitioning, but it would increase #ifdef ugliness in struct cpuinfo_x86. Also, compute_unit_id would just so happen to be initialized to 0, there would be no other code using it for CONFIG_SMP. L3 cache partitioning would be the first SMP-specific feature to be available in non-SMP kernels.
2. Same as #1, but remove CONFIG_SMP completely from struct cpuinfo_x86. This would mean less #ifdef ugliness there, but then we would have a bunch of unused fields in there in non-SMP kernels, which would also just be initialized to 0. I don't think that would be correct for booted_cores, but as it is unused I don't see an immediate problem with that. Of course, this is also neither correct nor less ugly.
3. Same as #2, but also rework all code using those fields to be usable on non-SMP kernels. This would be essentially a rework of all that CONFIG_SMP stuff, and I think thats too much to ask for just for a little extra L3 feature.
Maybe I'm missing something here, but I don't see how this could be done cleanly in any other way at this time.
Of course, you could just take the modified patch I sent you. That would be ugly, but not more so than the existing code. If this is not acceptable, please tell me which of the other two ugly solutions you would prefer.
Hans
-- %SYSTEM-F-ANARCHISM, The operating system has been overthrown
| |