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@xxxxxxxxx>
>> >> CC: Mike Rapoport <rppt@xxxxxxxxxx>
>> >> CC: Yajun Deng <yajun.deng@xxxxxxxxx>
>> >> CC: <stable@xxxxxxxxxxxxxxx>
>> >> ---
>> >> 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
