Messages in this thread | | | Date | Tue, 21 Aug 2012 21:26:17 +0100 | From | Attilio Rao <> | Subject | Re: [PATCH 3/5] X86/XEN: Introduce the x86_init.paging.pagetable_init PVOPS |
| |
On 21/08/12 16:44, Thomas Gleixner wrote: > On Tue, 21 Aug 2012, Attilio Rao wrote: > > >> This new PVOPS is responsible to setup the kernel pagetables and >> replace entirely x86_init.paging.pagetable_setup_start and >> x86_init.paging.pagetable_setup_done PVOPS work. >> > > >> For performance the x86_64 stub is implemented as a macro to paging_init() >> rather than an actual function stub. >> > Huch, using a macro for an once per boot time call is really a massive > performance improvement. > > It's confusing and wrong. You just use a macro because x86_64 does not > need any extra setups aside of paging_init(). > > >> diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c >> index 849be14..c1e910a 100644 >> --- a/arch/x86/kernel/x86_init.c >> +++ b/arch/x86/kernel/x86_init.c >> @@ -68,6 +68,7 @@ struct x86_init_ops x86_init __initdata = { >> }, >> >> .paging = { >> + .pagetable_init = native_pagetable_init, >> > I'd prefer to see these patches implemented differently. > > #1 Remove the base argument from pagetable_setup_start (leave > pagetable_setup_done() alone). > > #2 Rename pagetable_setup_start to pagetable_init, > native_pagetable_setup_start to native_pagetable_init and > xen_pagetable_setup_start to xen_pagetable_init > > #3 Instead of copying the whole native_pagetable_setup_start() > function and deleting it later, move the paging_init() call from > setup.c to native_pagetable_init() and xen_pagetable_init() > and define native_pagetable_init as paging_init() for x86_64 > > #4 Move the code from xen_pagetable_setup_done() into > xen_pagetable_init() and remove the now unused > pagetable_setup_done(). > > That's less code shuffling and pointless copying which makes the > review way easier. >
I've followed these steps in a new patch series (integrating suggestions from Konrad and Stefano too).
Attilio
| |