|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |