| Message ID | 20250318071948.23854-3-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | memblock: some fix for memmap_init_reserved_pages() | expand |
On 3/18/25 12:49, Wei Yang 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. But is it really possible for the memblock array to double during memmap_init_reserved_pages() ? Just wondering - could you please give some example scenarios. > > 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> > > --- > v2: move check out side of the loop > --- > mm/memblock.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 85442f1b7f14..0bae7547d2db 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_reserved; > > /* > * set nid on all reserved pages and also treat struct > * pages for the NOMAP regions as PageReserved > */ > +repeat: > + max_reserved = 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_reserved != memblock.reserved.max) > + goto repeat; > > /* > * initialize struct pages for reserved regions that don't have
On Tue, Mar 18, 2025 at 04:25:23PM +0530, Anshuman Khandual wrote: >On 3/18/25 12:49, Wei Yang 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. > >But is it really possible for the memblock array to double during >memmap_init_reserved_pages() ? Just wondering - could you please >give some example scenarios. > The possibility is low, but I think it is possible. I have created a test case to reproduce it. Not sure it could explain ?
diff --git a/mm/memblock.c b/mm/memblock.c index 85442f1b7f14..0bae7547d2db 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_reserved; /* * set nid on all reserved pages and also treat struct * pages for the NOMAP regions as PageReserved */ +repeat: + max_reserved = 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_reserved != memblock.reserved.max) + goto repeat; /* * initialize struct pages for reserved regions that don't have
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> --- v2: move check out side of the loop --- mm/memblock.c | 10 ++++++++++ 1 file changed, 10 insertions(+)