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

Andrew Cooper posted 1 patch 18 weeks 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 18 weeks 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 18 weeks 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