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.
> type->regions[0].base = 0;
> type->regions[0].size = 0;
> type->regions[0].flags = 0;
> @@ -600,12 +596,13 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>
> /* special case for empty array */
> if (type->regions[0].size == 0) {
> - WARN_ON(type->cnt != 1 || type->total_size);
> + WARN_ON(type->cnt != 0 || type->total_size);
> type->regions[0].base = base;
> type->regions[0].size = size;
> type->regions[0].flags = flags;
> memblock_set_region_node(&type->regions[0], nid);
> type->total_size = size;
> + type->cnt = 1;
> return 0;
> }
>
> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
> index 57bf2688edfd..f317fe691fc4 100644
> --- a/tools/testing/memblock/tests/basic_api.c
> +++ b/tools/testing/memblock/tests/basic_api.c
> @@ -15,12 +15,12 @@ static int memblock_initialization_check(void)
> PREFIX_PUSH();
>
> ASSERT_NE(memblock.memory.regions, NULL);
> - ASSERT_EQ(memblock.memory.cnt, 1);
> + ASSERT_EQ(memblock.memory.cnt, 0);
> ASSERT_EQ(memblock.memory.max, EXPECTED_MEMBLOCK_REGIONS);
> ASSERT_EQ(strcmp(memblock.memory.name, "memory"), 0);
>
> ASSERT_NE(memblock.reserved.regions, NULL);
> - ASSERT_EQ(memblock.reserved.cnt, 1);
> + ASSERT_EQ(memblock.reserved.cnt, 0);
> ASSERT_EQ(memblock.memory.max, EXPECTED_MEMBLOCK_REGIONS);
> ASSERT_EQ(strcmp(memblock.reserved.name, "reserved"), 0);
>
> @@ -1295,7 +1295,7 @@ static int memblock_remove_only_region_check(void)
> ASSERT_EQ(rgn->base, 0);
> ASSERT_EQ(rgn->size, 0);
>
> - ASSERT_EQ(memblock.memory.cnt, 1);
> + ASSERT_EQ(memblock.memory.cnt, 0);
> ASSERT_EQ(memblock.memory.total_size, 0);
>
> test_pass_pop();
> @@ -1723,7 +1723,7 @@ static int memblock_free_only_region_check(void)
> ASSERT_EQ(rgn->base, 0);
> ASSERT_EQ(rgn->size, 0);
>
> - ASSERT_EQ(memblock.reserved.cnt, 1);
> + ASSERT_EQ(memblock.reserved.cnt, 0);
> ASSERT_EQ(memblock.reserved.total_size, 0);
>
> test_pass_pop();
> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
> index f43b6f414983..c2c569f12178 100644
> --- a/tools/testing/memblock/tests/common.c
> +++ b/tools/testing/memblock/tests/common.c
> @@ -40,13 +40,13 @@ void reset_memblock_regions(void)
> {
> memset(memblock.memory.regions, 0,
> memblock.memory.cnt * sizeof(struct memblock_region));
> - memblock.memory.cnt = 1;
> + memblock.memory.cnt = 0;
> memblock.memory.max = INIT_MEMBLOCK_REGIONS;
> memblock.memory.total_size = 0;
>
> memset(memblock.reserved.regions, 0,
> memblock.reserved.cnt * sizeof(struct memblock_region));
> - memblock.reserved.cnt = 1;
> + memblock.reserved.cnt = 0;
> memblock.reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS;
> memblock.reserved.total_size = 0;
> }
> --
> 2.34.1
>
--
Sincerely yours,
Mike.
