On Wed, 4 Sep 2024 19:55:41 +0800 Qiang Liu <liuq131@xxxxxxxxxxxxxxx> wrote:
> Each call to the init_reserved_page function will look up the
> corresponding zid for the given pfn parameter. Even if subsequent
> calls have the same zid for the pfn as the current one, the lookup
> will be repeated.
>
> During system initialization, the memmap_init_reserved_pages function
> calls init_reserved_page for each contiguous memory region in mem_region.
> Implementing a cache for zid can significantly improve performance.
> Tests have shown that adding a zid cache reduces the execution time of
> the memmap_init_reserved_pages function by over 7%.
>
OK, but how much speedup do we see overall? In other words, is
memmap_init_reserved_pages() a significant consumer of execution time?
I'd be surprised if it makes much difference at all - MAX_NR_ZONES is a
small number. Maybe we call init_reserved_page() a lot.
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -710,19 +710,25 @@ static void __meminit init_reserved_page(unsigned long pfn, int nid)
> {
> pg_data_t *pgdat;
> int zid;
> + struct zone *zone;
> + static int zidcache;
What locking protects zidcache? lock_device_hotplug() and/or
__init-time serialization? This might be worth mentioning?
>
> if (early_page_initialised(pfn, nid))
> return;
>
> pgdat = NODE_DATA(nid);
>
> - for (zid = 0; zid < MAX_NR_ZONES; zid++) {
> - struct zone *zone = &pgdat->node_zones[zid];
> + zone = &pgdat->node_zones[zidcache];
OK, but if init_reserved_page() was previously called against a
different node, `zidcache' will refer to a zone in a different node.
The code will work OK, but it's worth mentioning somewhere I guess.
> + if (unlikely(zone_spans_pfn(zone, pfn)))
Isn't this wrong? We need to redo the search if !zone_spans_pfn(...)?
> + for (zid = 0; zid < MAX_NR_ZONES; zid++) {
> + zone = &pgdat->node_zones[zid];
>
> - if (zone_spans_pfn(zone, pfn))
> - break;
> - }
> - __init_single_page(pfn_to_page(pfn), pfn, zid, nid);
> + if (zone_spans_pfn(zone, pfn)) {
> + zidcache = zid;
> + break;
> + }
> + }
> + __init_single_page(pfn_to_page(pfn), pfn, zidcache, nid);
> }
> #else
> static inline void pgdat_set_deferred_range(pg_data_t *pgdat) {}
> --
> 2.27.0
