linux-kernel - Re: [PATCH v3 05/28] mm/hugetlb: fix round-robin bootmem allocation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPTztWakxktuqyW5bY2BYfC-jpVpEpDbLpxc_QgSH2Bb4DYh5A@mail.gmail.com>
Date: Mon, 10 Feb 2025 10:30:03 -0800
From: Frank van der Linden <fvdl@...gle.com>
To: Oscar Salvador <osalvador@...e.de>
Cc: akpm@...ux-foundation.org, muchun.song@...ux.dev, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, yuzhao@...gle.com, usamaarif642@...il.com, 
	joao.m.martins@...cle.com, roman.gushchin@...ux.dev, 
	Zhenguo Yao <yaozhenguo1@...il.com>
Subject: Re: [PATCH v3 05/28] mm/hugetlb: fix round-robin bootmem allocation

On Mon, Feb 10, 2025 at 4:57 AM Oscar Salvador <osalvador@...e.de> wrote:
>
> On Thu, Feb 06, 2025 at 06:50:45PM +0000, Frank van der Linden wrote:
> > Commit b5389086ad7b ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> > changed the NUMA_NO_NODE round-robin allocation behavior in case of a
> > failure to allocate from one NUMA node. The code originally moved on to
> > the next node to try again, but now it immediately breaks out of the loop.
> >
> > Restore the original behavior.
>
> Did you stumble upon this?
>
> AFAICS, memblock_alloc_range_nid() will call memblock_find_in_range_node() with
> NUMA_NO_NODE for exact_nide = false, which would be our case.
> Meaning that if memblock_alloc_try_nid_raw() returns false here, it is because
> we could not allocate the page in any node, right?

Hmm.. this patch is from earlier in the development when I stumbled on
a situation that looked like I describe it. However, you're right,
there is really no point to this patch. I mean, it's harmless in the
sense that it doesn't cause bugs, but it just wastes time. The problem
I saw might just have been caused by my own changes at the time, which
have been re-arranged since. I was probably also confused by the
for_each, which isn't really a for loop the way that it's used in this
function.

Looking at it further, it's actually the node-specific case that is
wrong. That case also uses memblock_alloc_try_nid_raw(), but it should
be using memblock_alloc_exact_nid_raw(). Right now, if you do e.g.
hugepages=0:X,1:Y, and there aren't X pages available on node 0, it
will still fall back to node 1, which is unexpected/unwanted behavior.

So, I'll change this patch to fix the node-specific case instead.
Looking at it, I also need to avoid the same fallback for the CMA
case, which will affect those patches a bit.

So, there'll be a v4 coming up. Fortunately, it won't affect the
patches with your Reviewed-by, so thanks again for those.

- Frank