| 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: <a7f5a4f7-1ec6-42dc-a93d-af043a01044f@redhat.com>
Date: Tue, 15 Apr 2025 21:07:53 -0400
From: Luiz Capitulino <luizcap@...hat.com>
To: Frank van der Linden <fvdl@...gle.com>, akpm@...ux-foundation.org,
muchun.song@...ux.dev, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Cc: david@...hat.com, osalvador@...e.de
Subject: Re: [PATCH] mm/hugetlb: use separate nodemask for bootmem allocations
On 2025-04-02 16:56, Frank van der Linden wrote:
> Hugetlb boot allocation has used online nodes for allocation since
> commit de55996d7188 ("mm/hugetlb: use online nodes for bootmem
> allocation"). This was needed to be able to do the allocations
> earlier in boot, before N_MEMORY was set.
Honest question: I imagine there's a reason why we can't move
x86's hugetlb_cma_reserve() and hugetlb_bootmem_alloc() calls
in setup_arch() to after x86_init.paging.pagetable_init() (which
seems to be where we call zone_sizes_init())? This way we could
go back to using N_MEMORY and avoid this dance.
I'm not familiar with vmemmap if that's the reason...
- Luiz
>
> This might lead to a different distribution of gigantic hugepages
> across NUMA nodes if there are memoryless nodes in the system.
>
> What happens is that the memoryless nodes are tried, but then
> the memblock allocation fails and falls back, which usually means
> that the node that has the highest physical address available
> will be used (top-down allocation). While this will end up
> getting the same number of hugetlb pages, they might not be
> be distributed the same way. The fallback for each memoryless
> node might not end up coming from the same node as the
> successful round-robin allocation from N_MEMORY nodes.
>
> While administrators that rely on having a specific number of
> hugepages per node should use the hugepages=N:X syntax, it's
> better not to change the old behavior for the plain hugepages=N
> case.
>
> To do this, construct a nodemask for hugetlb bootmem purposes
> only, containing nodes that have memory. Then use that
> for round-robin bootmem allocations.
>
> This saves some cycles, and the added advantage here is that
> hugetlb_cma can use it too, avoiding the older issue of
> pointless attempts to create a CMA area for memoryless nodes
> (which will also cause the per-node CMA area size to be too
> small).
>
> Fixes: de55996d7188 ("mm/hugetlb: use online nodes for bootmem allocation")
> Signed-off-by: Frank van der Linden <fvdl@...gle.com>
> ---
> include/linux/hugetlb.h | 3 +++
> mm/hugetlb.c | 30 ++++++++++++++++++++++++++++--
> mm/hugetlb_cma.c | 11 +++++++----
> 3 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 8f3ac832ee7f..fc9166f7f679 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -14,6 +14,7 @@
> #include <linux/pgtable.h>
> #include <linux/gfp.h>
> #include <linux/userfaultfd_k.h>
> +#include <linux/nodemask.h>
>
> struct ctl_table;
> struct user_struct;
> @@ -176,6 +177,8 @@ extern struct list_head huge_boot_pages[MAX_NUMNODES];
>
> void hugetlb_bootmem_alloc(void);
> bool hugetlb_bootmem_allocated(void);
> +extern nodemask_t hugetlb_bootmem_nodes;
> +void hugetlb_bootmem_set_nodes(void);
>
> /* arch callbacks */
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 6fccfe6d046c..e69f6f31e082 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -58,6 +58,7 @@ int hugetlb_max_hstate __read_mostly;
> unsigned int default_hstate_idx;
> struct hstate hstates[HUGE_MAX_HSTATE];
>
> +__initdata nodemask_t hugetlb_bootmem_nodes;
> __initdata struct list_head huge_boot_pages[MAX_NUMNODES];
> static unsigned long hstate_boot_nrinvalid[HUGE_MAX_HSTATE] __initdata;
>
> @@ -3237,7 +3238,8 @@ int __alloc_bootmem_huge_page(struct hstate *h, int nid)
> }
>
> /* allocate from next node when distributing huge pages */
> - for_each_node_mask_to_alloc(&h->next_nid_to_alloc, nr_nodes, node, &node_states[N_ONLINE]) {
> + for_each_node_mask_to_alloc(&h->next_nid_to_alloc, nr_nodes, node,
> + &hugetlb_bootmem_nodes) {
> m = alloc_bootmem(h, node, false);
> if (!m)
> return 0;
> @@ -3701,6 +3703,15 @@ static void __init hugetlb_init_hstates(void)
> struct hstate *h, *h2;
>
> for_each_hstate(h) {
> + /*
> + * Always reset to first_memory_node here, even if
> + * next_nid_to_alloc was set before - we can't
> + * reference hugetlb_bootmem_nodes after init, and
> + * first_memory_node is right for all further allocations.
> + */
> + h->next_nid_to_alloc = first_memory_node;
> + h->next_nid_to_free = first_memory_node;
> +
> /* oversize hugepages were init'ed in early boot */
> if (!hstate_is_gigantic(h))
> hugetlb_hstate_alloc_pages(h);
> @@ -4990,6 +5001,20 @@ static int __init default_hugepagesz_setup(char *s)
> }
> hugetlb_early_param("default_hugepagesz", default_hugepagesz_setup);
>
> +void __init hugetlb_bootmem_set_nodes(void)
> +{
> + int i, nid;
> + unsigned long start_pfn, end_pfn;
> +
> + if (!nodes_empty(hugetlb_bootmem_nodes))
> + return;
> +
> + for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
> + if (end_pfn > start_pfn)
> + node_set(nid, hugetlb_bootmem_nodes);
> + }
> +}
> +
> static bool __hugetlb_bootmem_allocated __initdata;
>
> bool __init hugetlb_bootmem_allocated(void)
> @@ -5005,6 +5030,8 @@ void __init hugetlb_bootmem_alloc(void)
> if (__hugetlb_bootmem_allocated)
> return;
>
> + hugetlb_bootmem_set_nodes();
> +
> for (i = 0; i < MAX_NUMNODES; i++)
> INIT_LIST_HEAD(&huge_boot_pages[i]);
>
> @@ -5012,7 +5039,6 @@ void __init hugetlb_bootmem_alloc(void)
>
> for_each_hstate(h) {
> h->next_nid_to_alloc = first_online_node;
> - h->next_nid_to_free = first_online_node;
>
> if (hstate_is_gigantic(h))
> hugetlb_hstate_alloc_pages(h);
> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> index e0f2d5c3a84c..f58ef4969e7a 100644
> --- a/mm/hugetlb_cma.c
> +++ b/mm/hugetlb_cma.c
> @@ -66,7 +66,7 @@ hugetlb_cma_alloc_bootmem(struct hstate *h, int *nid, bool node_exact)
> if (node_exact)
> return NULL;
>
> - for_each_online_node(node) {
> + for_each_node_mask(node, hugetlb_bootmem_nodes) {
> cma = hugetlb_cma[node];
> if (!cma || node == *nid)
> continue;
> @@ -153,11 +153,13 @@ void __init hugetlb_cma_reserve(int order)
> if (!hugetlb_cma_size)
> return;
>
> + hugetlb_bootmem_set_nodes();
> +
> for (nid = 0; nid < MAX_NUMNODES; nid++) {
> if (hugetlb_cma_size_in_node[nid] == 0)
> continue;
>
> - if (!node_online(nid)) {
> + if (!node_isset(nid, hugetlb_bootmem_nodes)) {
> pr_warn("hugetlb_cma: invalid node %d specified\n", nid);
> hugetlb_cma_size -= hugetlb_cma_size_in_node[nid];
> hugetlb_cma_size_in_node[nid] = 0;
> @@ -190,13 +192,14 @@ void __init hugetlb_cma_reserve(int order)
> * If 3 GB area is requested on a machine with 4 numa nodes,
> * let's allocate 1 GB on first three nodes and ignore the last one.
> */
> - per_node = DIV_ROUND_UP(hugetlb_cma_size, nr_online_nodes);
> + per_node = DIV_ROUND_UP(hugetlb_cma_size,
> + nodes_weight(hugetlb_bootmem_nodes));
> pr_info("hugetlb_cma: reserve %lu MiB, up to %lu MiB per node\n",
> hugetlb_cma_size / SZ_1M, per_node / SZ_1M);
> }
>
> reserved = 0;
> - for_each_online_node(nid) {
> + for_each_node_mask(nid, hugetlb_bootmem_nodes) {
> int res;
> char name[CMA_MAX_NAME];
>
Powered by blists - more mailing lists