[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86-64: don't use xmalloc_array() for allocation of the (per-CPU) IDTs
On 13/10/2011 13:14, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: >>>> On 13.10.11 at 11:34, Keir Fraser <keir@xxxxxxx> wrote: >> Given our antipathy to the x86-32 hypervisor, and the fact that any >> remaining users of it are unlikely to be running MP systems at all let alone >> large MP systems, how about this cleanup patch?... (It looks quite confusing >> as a patch, actually, but does the obvious thing). > > Looks good to me - I was actually considering to convert the x86-64 > code back to alloc_xenheap_pages() too (for we'll need to do that > eventually anyway when we want to support more than 5Tb of memory) > when I put together that earlier patch, but then refrained from doing so > to keep the patch size down. You mean there's a 5TB limit for alloc_domheap_pages() allocations?? I only switched to alloc_xenheap_pages() because it's safe for x86-32 too... -- Keir > Jan > >> x86: Simplify smpboot_alloc by merging x86-{32,64} code as far as possible. >> >> We still need one ifdef, as x86-32 does not have a compat_gdt_table. >> >> On x86-32 there is 1/2-page wastage due to allocating a whole page for >> the per-CPU IDT, however we expect very few users of the x86-32 >> hypervisor. Those that cannot move to the 64-bit hypervisor are likely >> using old single-processor systems or new single-procesor netbooks. On >> UP and small MP systems, the wastage is insignificant. >> >> Signed-off-by: Keir Fraser <keir@xxxxxxx> >> diff -r 1515484353c6 xen/arch/x86/smpboot.c >> --- a/xen/arch/x86/smpboot.c Thu Oct 13 10:09:28 2011 +0200 >> +++ b/xen/arch/x86/smpboot.c Thu Oct 13 10:25:01 2011 +0100 >> @@ -640,21 +640,16 @@ static void cpu_smpboot_free(unsigned in >> unsigned int order; >> >> order = get_order_from_pages(NR_RESERVED_GDT_PAGES); >> + free_xenheap_pages(per_cpu(gdt_table, cpu), order); >> + per_cpu(gdt_table, cpu) = NULL; >> + >> #ifdef __x86_64__ >> - if ( per_cpu(compat_gdt_table, cpu) ) >> - free_domheap_pages(virt_to_page(per_cpu(gdt_table, cpu)), order); >> - if ( per_cpu(gdt_table, cpu) ) >> - free_domheap_pages(virt_to_page(per_cpu(compat_gdt_table, cpu)), >> - order); >> + free_xenheap_pages(per_cpu(compat_gdt_table, cpu), order); >> per_cpu(compat_gdt_table, cpu) = NULL; >> - order = get_order_from_bytes(IDT_ENTRIES * sizeof(**idt_tables)); >> - if ( idt_tables[cpu] ) >> - free_domheap_pages(virt_to_page(idt_tables[cpu]), order); >> -#else >> - free_xenheap_pages(per_cpu(gdt_table, cpu), order); >> - xfree(idt_tables[cpu]); >> #endif >> - per_cpu(gdt_table, cpu) = NULL; >> + >> + order = get_order_from_bytes(IDT_ENTRIES * sizeof(idt_entry_t)); >> + free_xenheap_pages(idt_tables[cpu], order); >> idt_tables[cpu] = NULL; >> >> if ( stack_base[cpu] != NULL ) >> @@ -669,9 +664,6 @@ static int cpu_smpboot_alloc(unsigned in >> { >> unsigned int order; >> struct desc_struct *gdt; >> -#ifdef __x86_64__ >> - struct page_info *page; >> -#endif >> >> stack_base[cpu] = alloc_xenheap_pages(STACK_ORDER, 0); >> if ( stack_base[cpu] == NULL ) >> @@ -679,41 +671,28 @@ static int cpu_smpboot_alloc(unsigned in >> memguard_guard_stack(stack_base[cpu]); >> >> order = get_order_from_pages(NR_RESERVED_GDT_PAGES); >> -#ifdef __x86_64__ >> - page = alloc_domheap_pages(NULL, order, >> - MEMF_node(cpu_to_node(cpu))); >> - if ( !page ) >> + per_cpu(gdt_table, cpu) = gdt = >> + alloc_xenheap_pages(order, MEMF_node(cpu_to_node(cpu))); >> + if ( gdt == NULL ) >> goto oom; >> - per_cpu(compat_gdt_table, cpu) = gdt = page_to_virt(page); >> - memcpy(gdt, boot_cpu_compat_gdt_table, >> - NR_RESERVED_GDT_PAGES * PAGE_SIZE); >> - gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu; >> - page = alloc_domheap_pages(NULL, order, >> - MEMF_node(cpu_to_node(cpu))); >> - if ( !page ) >> - goto oom; >> - per_cpu(gdt_table, cpu) = gdt = page_to_virt(page); >> - order = get_order_from_bytes(IDT_ENTRIES * sizeof(**idt_tables)); >> - page = alloc_domheap_pages(NULL, order, >> - MEMF_node(cpu_to_node(cpu))); >> - if ( !page ) >> - goto oom; >> - idt_tables[cpu] = page_to_virt(page); >> -#else >> - per_cpu(gdt_table, cpu) = gdt = alloc_xenheap_pages(order, 0); >> - if ( !gdt ) >> - goto oom; >> - idt_tables[cpu] = xmalloc_array(idt_entry_t, IDT_ENTRIES); >> - if ( idt_tables[cpu] == NULL ) >> - goto oom; >> -#endif >> - memcpy(gdt, boot_cpu_gdt_table, >> - NR_RESERVED_GDT_PAGES * PAGE_SIZE); >> + memcpy(gdt, boot_cpu_gdt_table, NR_RESERVED_GDT_PAGES * PAGE_SIZE); >> BUILD_BUG_ON(NR_CPUS > 0x10000); >> gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu; >> >> - memcpy(idt_tables[cpu], idt_table, >> - IDT_ENTRIES*sizeof(idt_entry_t)); >> +#ifdef __x86_64__ >> + per_cpu(compat_gdt_table, cpu) = gdt = >> + alloc_xenheap_pages(order, MEMF_node(cpu_to_node(cpu))); >> + if ( gdt == NULL ) >> + goto oom; >> + memcpy(gdt, boot_cpu_compat_gdt_table, NR_RESERVED_GDT_PAGES * >> PAGE_SIZE); >> + gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu; >> +#endif >> + >> + order = get_order_from_bytes(IDT_ENTRIES * sizeof(idt_entry_t)); >> + idt_tables[cpu] = alloc_xenheap_pages(order, >> MEMF_node(cpu_to_node(cpu))); >> + if ( idt_tables[cpu] == NULL ) >> + goto oom; >> + memcpy(idt_tables[cpu], idt_table, IDT_ENTRIES * sizeof(idt_entry_t)); >> >> return 0; >> >> >>> Anyway, despite all this... >>> >>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> >>> >>> Acked-by: Keir Fraser <keir@xxxxxxx> >>> >>>> --- a/xen/arch/x86/smpboot.c >>>> +++ b/xen/arch/x86/smpboot.c >>>> @@ -639,9 +639,6 @@ static void cpu_smpboot_free(unsigned in >>>> { >>>> unsigned int order; >>>> >>>> - xfree(idt_tables[cpu]); >>>> - idt_tables[cpu] = NULL; >>>> - >>>> order = get_order_from_pages(NR_RESERVED_GDT_PAGES); >>>> #ifdef __x86_64__ >>>> if ( per_cpu(compat_gdt_table, cpu) ) >>>> @@ -650,10 +647,15 @@ static void cpu_smpboot_free(unsigned in >>>> free_domheap_pages(virt_to_page(per_cpu(compat_gdt_table, cpu)), >>>> order); >>>> per_cpu(compat_gdt_table, cpu) = NULL; >>>> + order = get_order_from_bytes(IDT_ENTRIES * sizeof(**idt_tables)); >>>> + if ( idt_tables[cpu] ) >>>> + free_domheap_pages(virt_to_page(idt_tables[cpu]), order); >>>> #else >>>> free_xenheap_pages(per_cpu(gdt_table, cpu), order); >>>> + xfree(idt_tables[cpu]); >>>> #endif >>>> per_cpu(gdt_table, cpu) = NULL; >>>> + idt_tables[cpu] = NULL; >>>> >>>> if ( stack_base[cpu] != NULL ) >>>> { >>>> @@ -691,19 +693,25 @@ static int cpu_smpboot_alloc(unsigned in >>>> if ( !page ) >>>> goto oom; >>>> per_cpu(gdt_table, cpu) = gdt = page_to_virt(page); >>>> + order = get_order_from_bytes(IDT_ENTRIES * sizeof(**idt_tables)); >>>> + page = alloc_domheap_pages(NULL, order, >>>> + MEMF_node(cpu_to_node(cpu))); >>>> + if ( !page ) >>>> + goto oom; >>>> + idt_tables[cpu] = page_to_virt(page); >>>> #else >>>> per_cpu(gdt_table, cpu) = gdt = alloc_xenheap_pages(order, 0); >>>> if ( !gdt ) >>>> goto oom; >>>> + idt_tables[cpu] = xmalloc_array(idt_entry_t, IDT_ENTRIES); >>>> + if ( idt_tables[cpu] == NULL ) >>>> + goto oom; >>>> #endif >>>> memcpy(gdt, boot_cpu_gdt_table, >>>> NR_RESERVED_GDT_PAGES * PAGE_SIZE); >>>> BUILD_BUG_ON(NR_CPUS > 0x10000); >>>> gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu; >>>> >>>> - idt_tables[cpu] = xmalloc_array(idt_entry_t, IDT_ENTRIES); >>>> - if ( idt_tables[cpu] == NULL ) >>>> - goto oom; >>>> memcpy(idt_tables[cpu], idt_table, >>>> IDT_ENTRIES*sizeof(idt_entry_t)); >>>> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Xen-devel mailing list >>>> Xen-devel@xxxxxxxxxxxxxxxxxxx >>>> http://lists.xensource.com/xen-devel >>> >>> > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |