Messages in this thread | | | Date | Fri, 25 Jan 2008 14:30:15 +0100 | From | Ingo Molnar <> | Subject | Re: [x86.git] new CPA implementation |
| |
* Andi Kleen <ak@novell.com> wrote:
> > - the new implementation is much more scalable, because it is > > lockless > > in the fastpath > > What fast path? This should not really be called that often, > especially not when DEBUG_PAGEALLOC has its own simple implementation.
that's a point you are still missing badly in all these discussions about unification and sound design practices: code reuse and a clean, layered design. PAGEALLOC now uses change_page_attr() again and that approach is working really well.
You made PAGEALLOC use a special-purpose remapping function but that would make c_p_a() unrobust indirectly - simply because it would be used much less.
So i've removed that change and have fixed c_p_a() instead to both be fast and scalable enough for PAGEALLOC and to be robust enough for PAGEALLOC. Both sides of the equation (PAGEALLOC and c_p_a()) benefit from that.
> Anyways the most important general optimization imho (which you > unfortunately dropped) was to get rid of the WBINVDs which unlike > everything else cpa does are _really_ costly.
it wasnt "dropped" - your wbinvd->clflush feature was never submitted in a clean fashion. The "great CPA" patchset you submitted to lkml 3 weeks ago was a badly interwined tangle of features and fixes, which just added ontop of the existing set of bad practices and design. We asked you to restructure, we even added your patchset to x86.git to encourage you to do it (and you wrote the whole code to begin with), but you did not clean it up and you generally were not showing interest in such efforts. We finally got sick of your stance and i rewrote the code with Thomas.
The current CPA patchset in x86.git does it the right way around: _first_ it got rid of the sources of unrobustness, consolidated and cleaned up the structure and design of the codebase, and now we can layer features ontop of it. (once we can trust the new codebase)
> > - the CPA-testsuite now passes without failures on both 32-bit and > > 64-bit. (it never fully worked with your CPA series.) > > Without reassembly implemented CPA_TEST will always imply running a > lot of the direct memory as 4K pages so it can't be safely enabled on > production kernels anymore. You should probably at least add a warning > or limit the test to only work on a small portion of the direct > mapping now.
good point - i have made it dependent on DEBUG_KERNEL now and extended the help text.
Ingo
| |