On Sun, Mar 16, 2025 at 05:32:35PM +0200, Mike Rapoport wrote:
On Fri, Mar 14, 2025 at 02:03:51AM +0000, Wei Yang wrote:
On Thu, Mar 13, 2025 at 05:07:59PM +0200, Mike Rapoport wrote:
Commit 61167ad5fecd ("mm: pass nid to reserve_bootmem_region()") introduce a way to set nid to all reserved region.
But there is a corner case it will leave some region with invalid nid. When memblock_set_node() doubles the array of memblock.reserved, it may lead to a new reserved region before current position. The new region will be left with an invalid node id.
Repeat the process when detecting it.
Fixes: 61167ad5fecd ("mm: pass nid to reserve_bootmem_region()") Signed-off-by: Wei Yang richard.weiyang@gmail.com CC: Mike Rapoport rppt@kernel.org CC: Yajun Deng yajun.deng@linux.dev CC: stable@vger.kernel.org
mm/memblock.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/mm/memblock.c b/mm/memblock.c index 85442f1b7f14..302dd7bc622d 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -2184,7 +2184,10 @@ static void __init memmap_init_reserved_pages(void) * set nid on all reserved pages and also treat struct * pages for the NOMAP regions as PageReserved */ +repeat: for_each_mem_region(region) {
unsigned long max = memblock.reserved.max;- nid = memblock_get_region_node(region); start = region->base; end = start + region->size;
@@ -2193,6 +2196,15 @@ static void __init memmap_init_reserved_pages(void) reserve_bootmem_region(start, end, nid); memblock_set_node(start, region->size, &memblock.reserved, nid);
/** 'max' is changed means memblock.reserved has been doubled* its array, which may result a new reserved region before* current 'start'. Now we should repeat the procedure to set* its node id.*/if (max != memblock.reserved.max)goto repeat;This check can be moved outside the loop, can't it?
We can. You mean something like this?
Yes, something like this.
diff --git a/mm/memblock.c b/mm/memblock.c index 85442f1b7f14..67fd1695cce4 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -2179,11 +2179,14 @@ static void __init memmap_init_reserved_pages(void) struct memblock_region *region; phys_addr_t start, end; int nid;
- unsigned long max;
maybe max_reserved?
Sure, will update it.
/* * set nid on all reserved pages and also treat struct * pages for the NOMAP regions as PageReserved */ +repeat:
- max = memblock.reserved.max; for_each_mem_region(region) { nid = memblock_get_region_node(region); start = region->base;
@@ -2194,6 +2197,13 @@ static void __init memmap_init_reserved_pages(void) memblock_set_node(start, region->size, &memblock.reserved, nid); }
- /*
* 'max' is changed means memblock.reserved has been doubled its* array, which may result a new reserved region before current* 'start'. Now we should repeat the procedure to set its node id.*/- if (max != memblock.reserved.max)
goto repeat;/* * initialize struct pages for reserved regions that don't have -- Wei Yang Help you, Help me
-- Sincerely yours, Mike.