[Xen-devel] [PATCH] x86/smpboot: Remove redundant order calculations

Andrew Cooper posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190711154945.7322-1-andrew.cooper3@citrix.com
xen/arch/x86/smpboot.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)

[Xen-devel] [PATCH] x86/smpboot: Remove redundant order calculations

Posted by Andrew Cooper 1 week ago
The GDT and IDT allocations are all order 0, and not going to change.

Use an explicit 0, instead of calling get_order_from_pages().  This
allows for the removal of the 'order' local parameter in both
cpu_smpboot_{alloc,free}().

While making this adjustment, rearrange cpu_smpboot_free() to fold the
two "if ( remove )" clauses.  There is no explicit requirements for the
order of free()s.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/smpboot.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 004285d14c..65e9ceeece 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -902,7 +902,7 @@ static void cleanup_cpu_root_pgt(unsigned int cpu)
  */
 static void cpu_smpboot_free(unsigned int cpu, bool remove)
 {
-    unsigned int order, socket = cpu_to_socket(cpu);
+    unsigned int socket = cpu_to_socket(cpu);
     struct cpuinfo_x86 *c = cpu_data;
 
     if ( cpumask_empty(socket_cpumask[socket]) )
@@ -944,16 +944,12 @@ static void cpu_smpboot_free(unsigned int cpu, bool remove)
             free_domheap_page(mfn_to_page(mfn));
     }
 
-    order = get_order_from_pages(NR_RESERVED_GDT_PAGES);
-    if ( remove )
-        FREE_XENHEAP_PAGES(per_cpu(gdt_table, cpu), order);
-
-    free_xenheap_pages(per_cpu(compat_gdt_table, cpu), order);
+    FREE_XENHEAP_PAGE(per_cpu(compat_gdt_table, cpu));
 
     if ( remove )
     {
-        order = get_order_from_bytes(IDT_ENTRIES * sizeof(idt_entry_t));
-        FREE_XENHEAP_PAGES(idt_tables[cpu], order);
+        FREE_XENHEAP_PAGE(per_cpu(gdt_table, cpu));
+        FREE_XENHEAP_PAGE(idt_tables[cpu]);
 
         if ( stack_base[cpu] )
         {
@@ -965,7 +961,7 @@ static void cpu_smpboot_free(unsigned int cpu, bool remove)
 
 static int cpu_smpboot_alloc(unsigned int cpu)
 {
-    unsigned int i, order, memflags = 0;
+    unsigned int i, memflags = 0;
     nodeid_t node = cpu_to_node(cpu);
     seg_desc_t *gdt;
     unsigned long stub_page;
@@ -980,8 +976,7 @@ static int cpu_smpboot_alloc(unsigned int cpu)
         goto out;
     memguard_guard_stack(stack_base[cpu]);
 
-    order = get_order_from_pages(NR_RESERVED_GDT_PAGES);
-    gdt = per_cpu(gdt_table, cpu) ?: alloc_xenheap_pages(order, memflags);
+    gdt = per_cpu(gdt_table, cpu) ?: alloc_xenheap_pages(0, memflags);
     if ( gdt == NULL )
         goto out;
     per_cpu(gdt_table, cpu) = gdt;
@@ -991,7 +986,7 @@ static int cpu_smpboot_alloc(unsigned int cpu)
     BUILD_BUG_ON(NR_CPUS > 0x10000);
     gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu;
 
-    per_cpu(compat_gdt_table, cpu) = gdt = alloc_xenheap_pages(order, memflags);
+    per_cpu(compat_gdt_table, cpu) = gdt = alloc_xenheap_pages(0, memflags);
     if ( gdt == NULL )
         goto out;
     per_cpu(compat_gdt_table_l1e, cpu) =
@@ -999,9 +994,8 @@ static int cpu_smpboot_alloc(unsigned int cpu)
     memcpy(gdt, boot_cpu_compat_gdt_table, NR_RESERVED_GDT_PAGES * PAGE_SIZE);
     gdt[PER_CPU_GDT_ENTRY - FIRST_RESERVED_GDT_ENTRY].a = cpu;
 
-    order = get_order_from_bytes(IDT_ENTRIES * sizeof(idt_entry_t));
     if ( idt_tables[cpu] == NULL )
-        idt_tables[cpu] = alloc_xenheap_pages(order, memflags);
+        idt_tables[cpu] = alloc_xenheap_pages(0, memflags);
     if ( idt_tables[cpu] == NULL )
         goto out;
     memcpy(idt_tables[cpu], idt_table, IDT_ENTRIES * sizeof(idt_entry_t));
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/smpboot: Remove redundant order calculations

Posted by Jan Beulich 1 week ago
On 11.07.2019 17:49, Andrew Cooper wrote:
> The GDT and IDT allocations are all order 0, and not going to change.
> 
> Use an explicit 0, instead of calling get_order_from_pages().  This
> allows for the removal of the 'order' local parameter in both
> cpu_smpboot_{alloc,free}().
> 
> While making this adjustment, rearrange cpu_smpboot_free() to fold the
> two "if ( remove )" clauses.  There is no explicit requirements for the
> order of free()s.
> 
> No practical change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

While I think that it was appropriate for the code to be independent
of actual (albeit never changing) sizes, I have to agree that with
the context switch side change in it's better to be consistent here.
Hence despite my slight dislike
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel