* [PATCH 1/3] mm/memblock: pass size instead of end to memblock_set_node() [not found] <20250312130728.1117-1-richard.weiyang@gmail.com> @ 2025-03-12 13:07 ` Wei Yang 2025-03-12 13:07 ` [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled Wei Yang 1 sibling, 0 replies; 6+ messages in thread From: Wei Yang @ 2025-03-12 13:07 UTC (permalink / raw) To: rppt, akpm; +Cc: linux-mm, Wei Yang, Yajun Deng, stable The second parameter of memblock_set_node() is size instead of end. Since it iterates from lower address to higher address, finally the node id is correct. But during the process, some of them are wrong. Pass size instead of end. 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/memblock.c b/mm/memblock.c index 64ae678cd1d1..85442f1b7f14 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -2192,7 +2192,7 @@ static void __init memmap_init_reserved_pages(void) if (memblock_is_nomap(region)) reserve_bootmem_region(start, end, nid); - memblock_set_node(start, end, &memblock.reserved, nid); + memblock_set_node(start, region->size, &memblock.reserved, nid); } /* -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled [not found] <20250312130728.1117-1-richard.weiyang@gmail.com> 2025-03-12 13:07 ` [PATCH 1/3] mm/memblock: pass size instead of end to memblock_set_node() Wei Yang @ 2025-03-12 13:07 ` Wei Yang 2025-03-13 15:07 ` Mike Rapoport 1 sibling, 1 reply; 6+ messages in thread From: Wei Yang @ 2025-03-12 13:07 UTC (permalink / raw) To: rppt, akpm; +Cc: linux-mm, Wei Yang, Yajun Deng, stable 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; } /* -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled 2025-03-12 13:07 ` [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled Wei Yang @ 2025-03-13 15:07 ` Mike Rapoport 2025-03-14 2:03 ` Wei Yang 0 siblings, 1 reply; 6+ messages in thread From: Mike Rapoport @ 2025-03-13 15:07 UTC (permalink / raw) To: Wei Yang; +Cc: akpm, linux-mm, Yajun Deng, stable Hi Wei, On Wed, Mar 12, 2025 at 01:07:27PM +0000, 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. > > 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? > } > > /* > -- > 2.34.1 > -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled 2025-03-13 15:07 ` Mike Rapoport @ 2025-03-14 2:03 ` Wei Yang 2025-03-16 15:32 ` Mike Rapoport 0 siblings, 1 reply; 6+ messages in thread From: Wei Yang @ 2025-03-14 2:03 UTC (permalink / raw) To: Mike Rapoport; +Cc: Wei Yang, akpm, linux-mm, Yajun Deng, stable On Thu, Mar 13, 2025 at 05:07:59PM +0200, Mike Rapoport wrote: >Hi Wei, > >On Wed, Mar 12, 2025 at 01:07:27PM +0000, 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. >> >> 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? 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; /* * 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled 2025-03-14 2:03 ` Wei Yang @ 2025-03-16 15:32 ` Mike Rapoport 2025-03-18 2:58 ` Wei Yang 0 siblings, 1 reply; 6+ messages in thread From: Mike Rapoport @ 2025-03-16 15:32 UTC (permalink / raw) To: Wei Yang; +Cc: akpm, linux-mm, Yajun Deng, stable 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? > > /* > * 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled 2025-03-16 15:32 ` Mike Rapoport @ 2025-03-18 2:58 ` Wei Yang 0 siblings, 0 replies; 6+ messages in thread From: Wei Yang @ 2025-03-18 2:58 UTC (permalink / raw) To: Mike Rapoport; +Cc: Wei Yang, akpm, linux-mm, Yajun Deng, stable 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. -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-18 2:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250312130728.1117-1-richard.weiyang@gmail.com>
2025-03-12 13:07 ` [PATCH 1/3] mm/memblock: pass size instead of end to memblock_set_node() Wei Yang
2025-03-12 13:07 ` [PATCH 2/3] mm/memblock: repeat setting reserved region nid if array is doubled Wei Yang
2025-03-13 15:07 ` Mike Rapoport
2025-03-14 2:03 ` Wei Yang
2025-03-16 15:32 ` Mike Rapoport
2025-03-18 2:58 ` Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox