[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.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.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.