[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] xen/mm: Remove loops in get_order_from_{pages,bytes}()



These two functions are just fls() expressions in disguise.  Furthermore,
GCC-12 at least won't const-propagate get_order_from_pages() with an input of
3 or above, which covers most of the cases it gets used for.

Rewrite them, and provide some self testing.

Interesting, unless they are always_inline, GCC has a habit of out-of-lining
them even when inlining would have been a win.  I expect this is to do with
the fls() functions being superficially more complex.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
CC: Michal Orzel <michal.orzel@xxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2999078934

I think paddr_t is the wrong type to be using in get_order_from_bytes(), and
so really is fls64().  I could add fls_g() which would be slightly nicer on
builds where paddr_t != uint64_t.

Bloat-o-meter reports:

  x86:
    add/remove: 1/0 grow/shrink: 1/20 up/down: 39/-624 (-585)
    Function                                     old     new   delta
    deallocate_buffer                              -      37     +37
    iommu_flush_iotlb                            518     520      +2
    amd_iommu_init                               648     647      -1
    do_memory_op                               11052   11050      -2
    dom0_construct_pvh                          4025    4021      -4
    allocate_ring_buffer.constprop                79      73      -6
    conring_flush.constprop                      267     259      -8
    allocate_buffer                              131     123      -8
    _free_percpu_area                            104      96      -8
    vcpu_create                                  865     849     -16
    kexec_init                                   295     279     -16
    enable_qinval                                447     427     -20
    cpu_percpu_callback                          238     216     -22
    heap_init_late                              1683    1659     -24
    amd_iommu_alloc_intremap_table               363     331     -32
    dom0_construct                              8314    8274     -40
    alloc_pgtable_maddr                          263     217     -46
    free_ivrs_mapping_callback                   132      82     -50
    amd_iommu_setup_device_table                 746     680     -66
    _xmalloc.part                                585     517     -68
    enable_iommu                                1668    1583     -85
    amd_iommu_init_cleanup                       551     449    -102

  arm64:
    add/remove: 2/0 grow/shrink: 3/9 up/down: 248/-400 (-152)
    Function                                     old     new   delta
    guest_map_pages                                -     176    +176
    get_allocation_size                            -      28     +28
    allocate_domheap_memory                      268     288     +20
    do_memory_op                                8584    8600     +16
    heap_init_late                              1048    1052      +4
    find_unallocated_memory                      452     456      +4
    kernel_decompress                            772     768      -4
    end_boot_allocator                           836     832      -4
    cpu_percpu_callback                          328     324      -4
    arch_livepatch_quiesce                       212     208      -4
    apply_alternatives_all                       360     356      -4
    conring_flush.constprop                      336     328      -8
    _xmalloc.part                                584     572     -12
    construct_hwdom                             1088    1024     -64
    allocate_bank_memory                         444     148    -296

The three introduced functions are all examples of the inlining decisions
going wrong.  Still, at least all of these examples are now basic blocks
rather than loops.
---
 xen/common/bitops.c  | 27 +++++++++++++++++++++++++++
 xen/include/xen/mm.h | 18 ++++--------------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/xen/common/bitops.c b/xen/common/bitops.c
index 859a4ca5c131..1291cbbf0967 100644
--- a/xen/common/bitops.c
+++ b/xen/common/bitops.c
@@ -259,3 +259,30 @@ static void __init __constructor test_muldiv64(void)
     if ( res != 0xffffffffffffffffULL )
         panic("muldiv64(), expecting -1ULL, got %#"PRIx64"\n", res);
 }
+
+#include <xen/mm.h>
+
+/* Not really bitops, but here in lieu of any better location */
+static void __init test_order_from_pages(void)
+{
+    CHECK(get_order_from_pages, 1, 0);
+    CHECK(get_order_from_pages, 2, 1);
+    CHECK(get_order_from_pages, 3, 2);
+    CHECK(get_order_from_pages, 4, 2);
+    CHECK(get_order_from_pages, 5, 3);
+    CHECK(get_order_from_pages, 8, 3);
+    CHECK(get_order_from_pages, 9, 4);
+}
+
+static void __init test_order_from_bytes(void)
+{
+    CHECK(get_order_from_bytes, PAGE_SIZE - 1, 0);
+    CHECK(get_order_from_bytes, PAGE_SIZE,     0);
+    CHECK(get_order_from_bytes, PAGE_SIZE + 1, 1);
+}
+
+static void __init __constructor test_orders(void)
+{
+    test_order_from_pages();
+    test_order_from_bytes();
+}
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 153c57f5ddc2..8f4bba94919b 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -530,26 +530,16 @@ page_list_splice(struct page_list_head *list, struct 
page_list_head *head)
     list_for_each_entry_safe_reverse(pos, tmp, head, list)
 #endif
 
-static inline unsigned int get_order_from_bytes(paddr_t size)
+static always_inline unsigned int get_order_from_bytes(paddr_t size)
 {
-    unsigned int order;
-
     size = (size - 1) >> PAGE_SHIFT;
-    for ( order = 0; size; order++ )
-        size >>= 1;
 
-    return order;
+    return fls64(size);
 }
 
-static inline unsigned int get_order_from_pages(unsigned long nr_pages)
+static always_inline unsigned int get_order_from_pages(unsigned long nr_pages)
 {
-    unsigned int order;
-
-    nr_pages--;
-    for ( order = 0; nr_pages; order++ )
-        nr_pages >>= 1;
-
-    return order;
+    return flsl(nr_pages - 1);
 }
 
 #ifndef arch_free_heap_page
-- 
2.39.5




 


Rackspace

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