| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <942308c1-2dfd-4c61-8ec5-d70b26d7642c@linux.ibm.com>
Date: Thu, 22 May 2025 17:59:50 +0530
From: Donet Tom <donettom@...ux.ibm.com>
To: David Hildenbrand <david@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Mike Rapoport <rppt@...nel.org>, Oscar Salvador <osalvador@...e.de>,
Zi Yan <ziy@...dia.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Ritesh Harjani <ritesh.list@...il.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, "Rafael J . Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Alison Schofield <alison.schofield@...el.com>,
Yury Norov <yury.norov@...il.com>, Dave Jiang <dave.jiang@...el.com>
Subject: Re: [PATCH v5 1/4] drivers/base/node: Optimize memory block
registration to reduce boot time
On 5/22/25 5:39 PM, David Hildenbrand wrote:
> On 22.05.25 11:17, Donet Tom wrote:
>> During node device initialization, `memory blocks` are registered under
>> each NUMA node. The `memory blocks` to be registered are identified
>> using
>> the node’s start and end PFNs, which are obtained from the node's
>> pg_data
>>
>> However, not all PFNs within this range necessarily belong to the same
>> node—some may belong to other nodes. Additionally, due to the
>> discontiguous nature of physical memory, certain sections within a
>> `memory block` may be absent.
>>
>> As a result, `memory blocks` that fall between a node’s start and end
>> PFNs may span across multiple nodes, and some sections within those
>> blocks
>> may be missing. `Memory blocks` have a fixed size, which is architecture
>> dependent.
>>
>> Due to these considerations, the memory block registration is currently
>> performed as follows:
>>
>> for_each_online_node(nid):
>> start_pfn = pgdat->node_start_pfn;
>> end_pfn = pgdat->node_start_pfn + node_spanned_pages;
>> for_each_memory_block_between(PFN_PHYS(start_pfn),
>> PFN_PHYS(end_pfn))
>> mem_blk = memory_block_id(pfn_to_section_nr(pfn));
>> pfn_mb_start=section_nr_to_pfn(mem_blk->start_section_nr)
>> pfn_mb_end = pfn_start + memory_block_pfns - 1
>> for (pfn = pfn_mb_start; pfn < pfn_mb_end; pfn++):
>> if (get_nid_for_pfn(pfn) != nid):
>> continue;
>> else
>> do_register_memory_block_under_node(nid, mem_blk,
>> MEMINIT_EARLY);
>>
>> Here, we derive the start and end PFNs from the node's pg_data, then
>> determine the memory blocks that may belong to the node. For each
>> `memory block` in this range, we inspect all PFNs it contains and check
>> their associated NUMA node ID. If a PFN within the block matches the
>> current node, the memory block is registered under that node.
>>
>> If CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, get_nid_for_pfn()
>> performs
>> a binary search in the `memblock regions` to determine the NUMA node ID
>> for a given PFN. If it is not enabled, the node ID is retrieved directly
>> from the struct page.
>>
>> On large systems, this process can become time-consuming, especially
>> since
>> we iterate over each `memory block` and all PFNs within it until a
>> match is
>> found. When CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, the additional
>> overhead of the binary search increases the execution time
>> significantly,
>> potentially leading to soft lockups during boot.
>>
>> In this patch, we iterate over `memblock region` to identify the
>> `memory blocks` that belong to the current NUMA node. `memblock regions`
>> are contiguous memory ranges, each associated with a single NUMA
>> node, and
>> they do not span across multiple nodes.
>>
>> for_each_memory_region(r): // r => region
>> if (!node_online(r->nid)):
>> continue;
>> else
>> for_each_memory_block_between(r->base, r->base + r->size - 1):
>> do_register_memory_block_under_node(r->nid, mem_blk,
>> MEMINIT_EARLY);
>>
>> We iterate over all memblock regions, and if the node associated with
>> the
>> region is online, we calculate the start and end memory blocks based
>> on the
>> region's start and end PFNs. We then register all the memory blocks
>> within
>> that range under the region node.
>>
>> Test Results on My system with 32TB RAM
>> =======================================
>> 1. Boot time with CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled.
>>
>> Without this patch
>> ------------------
>> Startup finished in 1min 16.528s (kernel)
>>
>> With this patch
>> ---------------
>> Startup finished in 17.236s (kernel) - 78% Improvement
>>
>> 2. Boot time with CONFIG_DEFERRED_STRUCT_PAGE_INIT disabled.
>>
>> Without this patch
>> ------------------
>> Startup finished in 28.320s (kernel)
>>
>> With this patch
>> ---------------
>> Startup finished in 15.621s (kernel) - 46% Improvement
>>
>> Acked-by: Zi Yan <ziy@...dia.com>
>> Signed-off-by: Donet Tom <donettom@...ux.ibm.com>
>>
>> ---
>> v4 -> v5
>>
>> 1. Moved all helpers(memory_block_id(), pfn_to_block_id(), and
>> phys_to_block_id())
>> into memory.h and exported sections_per_block.
>> 2. register_memory_blocks_early() moved out of for_each_online_node().
>> Now we iterate over all memory regions at once and register the
>> memory blocks.
>>
>> Tested corner cases where memory blocks span across multiple
>> memblock regions; it
>> is working fine.
>>
>> #cd /sys/devices/system/node/
>> # find node1/ |grep memory0
>> node1/memory0
>> # find node0/ |grep memory0
>> node0/memory0
>> # find node0/ |grep memory0
>> node2/memory0
>> # cat node0/memory0/valid_zones
>> none
>>
>> V4 -
>> https://lore.kernel.org/all/f94685be9cdc931a026999d236d7e92de29725c7.1747376551.git.donettom@linux.ibm.com/
>> V3 -
>> https://lore.kernel.org/all/b49ed289096643ff5b5fbedcf1d1c1be42845a74.1746250339.git.donettom@linux.ibm.com/
>> v2 -
>> https://lore.kernel.org/all/fbe1e0c7d91bf3fa9a64ff5d84b53ded1d0d5ac7.1745852397.git.donettom@linux.ibm.com/
>> v1 -
>> https://lore.kernel.org/all/50142a29010463f436dc5c4feb540e5de3bb09df.1744175097.git.donettom@linux.ibm.com/
>> ---
>> drivers/base/memory.c | 21 ++++----------------
>> drivers/base/node.c | 45 ++++++++++++++++++++++++++++++++++++++++--
>> include/linux/memory.h | 19 +++++++++++++++++-
>> include/linux/node.h | 3 +++
>> 4 files changed, 68 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index 19469e7f88c2..39fcc075a36f 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -22,6 +22,7 @@
>> #include <linux/stat.h>
>> #include <linux/slab.h>
>> #include <linux/xarray.h>
>> +#include <linux/export.h>
>> #include <linux/atomic.h>
>> #include <linux/uaccess.h>
>> @@ -48,22 +49,8 @@ int mhp_online_type_from_str(const char *str)
>> #define to_memory_block(dev) container_of(dev, struct
>> memory_block, dev)
>> -static int sections_per_block;
>> -
>> -static inline unsigned long memory_block_id(unsigned long section_nr)
>> -{
>> - return section_nr / sections_per_block;
>> -}
>> -
>> -static inline unsigned long pfn_to_block_id(unsigned long pfn)
>> -{
>> - return memory_block_id(pfn_to_section_nr(pfn));
>> -}
>> -
>> -static inline unsigned long phys_to_block_id(unsigned long phys)
>> -{
>> - return pfn_to_block_id(PFN_DOWN(phys));
>> -}
>> +int sections_per_block;
>> +EXPORT_SYMBOL(sections_per_block);
>> static int memory_subsys_online(struct device *dev);
>> static int memory_subsys_offline(struct device *dev);
>> @@ -632,7 +619,7 @@ int __weak arch_get_memory_phys_device(unsigned
>> long start_pfn)
>> *
>> * Called under device_hotplug_lock.
>> */
>> -static struct memory_block *find_memory_block_by_id(unsigned long
>> block_id)
>> +struct memory_block *find_memory_block_by_id(unsigned long block_id)
>> {
>> struct memory_block *mem;
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index cd13ef287011..e8b6f6b9ce51 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -20,6 +20,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/swap.h>
>> #include <linux/slab.h>
>> +#include <linux/memblock.h>
>> static const struct bus_type node_subsys = {
>> .name = "node",
>> @@ -850,6 +851,41 @@ void unregister_memory_block_under_nodes(struct
>> memory_block *mem_blk)
>> kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
>> }
>> +/*
>> + * register_memory_blocks_under_node_early : Register the memory blocks
>> + * under the nodes.
>
> "register_memory_blocks_under_nodes"
>
>> + *
>> + * This function iterates over all memblock regions, and if the node
>> associated with
>
> "the node" does not apply.
>
>> + * the region is online, calculates the start and end memory blocks
>> based on the
>> + * region's start and end PFNs. Then, registers all the memory
>> blocks within that
>> + * range under the region node.
>
> More like "registers all memory blocks under the corresponding nodes
> ..." then clarify that a block might get registered under multiple
> nodes etc.
>
>> + */
>> +static void register_memory_blocks_under_node_early(void)
>> +{
>> + struct memblock_region *r;
>> +
>> + for_each_mem_region(r) {
>> + const unsigned long start_block_id = phys_to_block_id(r->base);
>> + const unsigned long end_block_id = phys_to_block_id(r->base
>> + r->size - 1);
>> + unsigned long block_id;
>> +
>> + if (!node_online(r->nid))
>> + continue;
>> +
>> + for (block_id = start_block_id; block_id <= end_block_id;
>> block_id++) {
>> + struct memory_block *mem;
>> +
>> + mem = find_memory_block_by_id(block_id);
>> + if (!mem)
>> + continue;
>> +
>> + do_register_memory_block_under_node(r->nid, mem,
>> MEMINIT_EARLY);
>> + put_device(&mem->dev);
>> + }
>> +
>> + }
>> +}
>> +
>> void register_memory_blocks_under_node(int nid, unsigned long
>> start_pfn,
>> unsigned long end_pfn,
>> enum meminit_context context)
>> @@ -971,11 +1007,16 @@ void __init node_dev_init(void)
>> /*
>> * Create all node devices, which will properly link the node
>> - * to applicable memory block devices and already created cpu
>> devices.
>> + * to already created cpu devices.
>> */
>> for_each_online_node(i) {
>> - ret = register_one_node(i);
>> + ret = __register_one_node(i);
>> if (ret)
>> panic("%s() failed to add node: %d\n", __func__, ret);
>> }
>> +
>> + /*
>> + * Link the node to memory block devices
>> + */
>> + register_memory_blocks_under_node_early();
> > }> diff --git a/include/linux/memory.h b/include/linux/memory.h
>> index 12daa6ec7d09..2a61088e17ad 100644
>> --- a/include/linux/memory.h
>> +++ b/include/linux/memory.h
>> @@ -171,12 +171,30 @@ struct memory_group
>> *memory_group_find_by_id(int mgid);
>> typedef int (*walk_memory_groups_func_t)(struct memory_group *,
>> void *);
>> int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t
>> func,
>> struct memory_group *excluded, void *arg);
>> +struct memory_block *find_memory_block_by_id(unsigned long block_id);
>> #define hotplug_memory_notifier(fn, pri) ({ \
>> static __meminitdata struct notifier_block fn##_mem_nb =\
>> { .notifier_call = fn, .priority = pri };\
>> register_memory_notifier(&fn##_mem_nb); \
>> })
>> +extern int sections_per_block;
>> +
>> +static inline unsigned long memory_block_id(unsigned long section_nr)
>> +{
>> + return section_nr / sections_per_block;
>> +}
>> +
>> +static inline unsigned long pfn_to_block_id(unsigned long pfn)
>> +{
>> + return memory_block_id(pfn_to_section_nr(pfn));
>> +}
>> +
>> +static inline unsigned long phys_to_block_id(unsigned long phys)
>> +{
>> + return pfn_to_block_id(PFN_DOWN(phys));
>> +}
>> +
>> #ifdef CONFIG_NUMA
>> void memory_block_add_nid(struct memory_block *mem, int nid,
>> enum meminit_context context);
>> @@ -188,5 +206,4 @@ void memory_block_add_nid(struct memory_block
>> *mem, int nid,
>> * can sleep.
>> */
>> extern struct mutex text_mutex;
>> -
>
> Unrelated change.
>
>
Thank you, David.
I will make the changes and send the next version.
>
>
> Apart from that LGTM
>
Powered by blists - more mailing lists