On Mon, Apr 08, 2024 at 08:44:31AM +0300, Mike Rapoport wrote:
>On Fri, Apr 05, 2024 at 01:58:21AM +0000, Wei Yang wrote:
>> The dummy entry is introduced in the initial implementation of lmb in
>> commit 7c8c6b9776fb ("powerpc: Merge lmb.c and make MM initialization
>> use it.").
>>
>> As the comment says the empty dummy entry is to simplify the code.
>>
>> /* Create a dummy zero size LMB which will get coalesced away later.
>> * This simplifies the lmb_add() code below...
>> */
>>
>> While current code is reimplemented by Tejun in commit 784656f9c680
>> ("memblock: Reimplement memblock_add_region()"). This empty dummy entry
>> seems not benefit the code any more.
>>
>> Let's remove it.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>> CC: Paul Mackerras <paulus@xxxxxxxxxx>
>> CC: Tejun Heo <tj@xxxxxxxxxx>
>> CC: Mike Rapoport <rppt@xxxxxxxxxx>
>>
>> ---
>> v2: remove cnt initialization to 0 and keep special case for empty array
>> v3: reset cnt to 0 in memblock test
>> ---
>> mm/memblock.c | 7 ++-----
>> tools/testing/memblock/tests/basic_api.c | 8 ++++----
>> tools/testing/memblock/tests/common.c | 4 ++--
>> 3 files changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index d09136e040d3..98d25689cf10 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -114,12 +114,10 @@ static struct memblock_region memblock_physmem_init_regions[INIT_PHYSMEM_REGIONS
>>
>> struct memblock memblock __initdata_memblock = {
>> .memory.regions = memblock_memory_init_regions,
>> - .memory.cnt = 1, /* empty dummy entry */
>> .memory.max = INIT_MEMBLOCK_MEMORY_REGIONS,
>> .memory.name = "memory",
>>
>> .reserved.regions = memblock_reserved_init_regions,
>> - .reserved.cnt = 1, /* empty dummy entry */
>> .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS,
>> .reserved.name = "reserved",
>>
>> @@ -130,7 +128,6 @@ struct memblock memblock __initdata_memblock = {
>> #ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP
>> struct memblock_type physmem = {
>> .regions = memblock_physmem_init_regions,
>> - .cnt = 1, /* empty dummy entry */
>> .max = INIT_PHYSMEM_REGIONS,
>> .name = "physmem",
>> };
>> @@ -356,7 +353,6 @@ static void __init_memblock memblock_remove_region(struct memblock_type *type, u
>> /* Special case for empty arrays */
>> if (type->cnt == 0) {
>> WARN_ON(type->total_size != 0);
>> - type->cnt = 1;
>
>Sorry if I wasn't clear, I meant to keep special case only in
>memblock_add_range. Here I think
>
> WARN_ON(type->cnt == 0 && type->total_size != 0);
>
>should be enough.
>
You mean change like this?
diff --git a/mm/memblock.c b/mm/memblock.c
index d09136e040d3..b75b835f99d1 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -352,16 +352,7 @@ static void __init_memblock memblock_remove_region(struct memblock_type *type, u
memmove(&type->regions[r], &type->regions[r + 1],
(type->cnt - (r + 1)) * sizeof(type->regions[r]));
type->cnt--;
-
- /* Special case for empty arrays */
- if (type->cnt == 0) {
- WARN_ON(type->total_size != 0);
- type->cnt = 1;
- type->regions[0].base = 0;
- type->regions[0].size = 0;
- type->regions[0].flags = 0;
- memblock_set_region_node(&type->regions[0], MAX_NUMNODES);
- }
+ WARN_ON(type->cnt == 0 && type->total_size != 0);
}
#ifndef CONFIG_ARCH_KEEP_MEMBLOCK
This doesn't work, since on removing the last region, memmove would take no
effect and left regions[0].base .size .flags not changed.
So we need to keep the special handling here.
