* [PATCH] mm/memblock: remove empty dummy entry
@ 2024-04-03 9:10 Wei Yang
2024-04-04 10:13 ` Mike Rapoport
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yang @ 2024-04-03 9:10 UTC (permalink / raw)
To: rppt, akpm; +Cc: linux-mm, Wei Yang, Paul Mackerras, Tejun Heo
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@gmail.com>
CC: Paul Mackerras <paulus@ozlabs.org>
CC: Tejun Heo <tj@kernel.org>
CC: Mike Rapoport <rppt@kernel.org>
---
mm/memblock.c | 18 +++---------------
tools/testing/memblock/tests/basic_api.c | 8 ++++----
tools/testing/memblock/tests/common.c | 4 ++--
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/mm/memblock.c b/mm/memblock.c
index d09136e040d3..100d3b7ab499 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -114,12 +114,12 @@ 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.cnt = 0,
.memory.max = INIT_MEMBLOCK_MEMORY_REGIONS,
.memory.name = "memory",
.reserved.regions = memblock_reserved_init_regions,
- .reserved.cnt = 1, /* empty dummy entry */
+ .reserved.cnt = 0,
.reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS,
.reserved.name = "reserved",
@@ -130,7 +130,7 @@ 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 */
+ .cnt = 0,
.max = INIT_PHYSMEM_REGIONS,
.name = "physmem",
};
@@ -356,7 +356,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;
type->regions[0].base = 0;
type->regions[0].size = 0;
type->regions[0].flags = 0;
@@ -598,17 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
if (!size)
return 0;
- /* special case for empty array */
- if (type->regions[0].size == 0) {
- WARN_ON(type->cnt != 1 || 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;
- return 0;
- }
-
/*
* The worst case is when new range overlaps all existing regions,
* then we'll need type->cnt + 1 empty regions in @type. So if
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..4fa94b281280 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mm/memblock: remove empty dummy entry
2024-04-03 9:10 [PATCH] mm/memblock: remove empty dummy entry Wei Yang
@ 2024-04-04 10:13 ` Mike Rapoport
2024-04-04 12:44 ` Wei Yang
2024-04-05 1:51 ` Wei Yang
0 siblings, 2 replies; 4+ messages in thread
From: Mike Rapoport @ 2024-04-04 10:13 UTC (permalink / raw)
To: Wei Yang; +Cc: akpm, linux-mm, Paul Mackerras, Tejun Heo
On Wed, Apr 03, 2024 at 09:10:45AM +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@gmail.com>
> CC: Paul Mackerras <paulus@ozlabs.org>
> CC: Tejun Heo <tj@kernel.org>
> CC: Mike Rapoport <rppt@kernel.org>
> ---
> mm/memblock.c | 18 +++---------------
> tools/testing/memblock/tests/basic_api.c | 8 ++++----
> tools/testing/memblock/tests/common.c | 4 ++--
> 3 files changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/mm/memblock.c b/mm/memblock.c
> index d09136e040d3..100d3b7ab499 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -114,12 +114,12 @@ 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.cnt = 0,
No need to set memory.cnt to 0, this line can be just removed
> .memory.max = INIT_MEMBLOCK_MEMORY_REGIONS,
> .memory.name = "memory",
>
> .reserved.regions = memblock_reserved_init_regions,
> - .reserved.cnt = 1, /* empty dummy entry */
> + .reserved.cnt = 0,
Ditto.
> .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS,
> .reserved.name = "reserved",
>
> @@ -130,7 +130,7 @@ 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 */
> + .cnt = 0,
Ditto.
> .max = INIT_PHYSMEM_REGIONS,
> .name = "physmem",
> };
> @@ -356,7 +356,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;
> type->regions[0].base = 0;
> type->regions[0].size = 0;
> type->regions[0].flags = 0;
> @@ -598,17 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
> if (!size)
> return 0;
>
> - /* special case for empty array */
> - if (type->regions[0].size == 0) {
> - WARN_ON(type->cnt != 1 || 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;
> - return 0;
> - }
I'd keep the special case for empty array, just update it for new semantics
of type->cnt.
> -
> /*
> * The worst case is when new range overlaps all existing regions,
> * then we'll need type->cnt + 1 empty regions in @type. So if
> 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..4fa94b281280 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;
Here as well, no need to initialize to 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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mm/memblock: remove empty dummy entry
2024-04-04 10:13 ` Mike Rapoport
@ 2024-04-04 12:44 ` Wei Yang
2024-04-05 1:51 ` Wei Yang
1 sibling, 0 replies; 4+ messages in thread
From: Wei Yang @ 2024-04-04 12:44 UTC (permalink / raw)
To: Mike Rapoport; +Cc: Wei Yang, akpm, linux-mm, Paul Mackerras, Tejun Heo
On Thu, Apr 04, 2024 at 01:13:41PM +0300, Mike Rapoport wrote:
>On Wed, Apr 03, 2024 at 09:10:45AM +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@gmail.com>
>> CC: Paul Mackerras <paulus@ozlabs.org>
>> CC: Tejun Heo <tj@kernel.org>
>> CC: Mike Rapoport <rppt@kernel.org>
>> ---
>> mm/memblock.c | 18 +++---------------
>> tools/testing/memblock/tests/basic_api.c | 8 ++++----
>> tools/testing/memblock/tests/common.c | 4 ++--
>> 3 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index d09136e040d3..100d3b7ab499 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -114,12 +114,12 @@ 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.cnt = 0,
>
>No need to set memory.cnt to 0, this line can be just removed
>
>> .memory.max = INIT_MEMBLOCK_MEMORY_REGIONS,
>> .memory.name = "memory",
>>
>> .reserved.regions = memblock_reserved_init_regions,
>> - .reserved.cnt = 1, /* empty dummy entry */
>> + .reserved.cnt = 0,
>
>Ditto.
>
>> .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS,
>> .reserved.name = "reserved",
>>
>> @@ -130,7 +130,7 @@ 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 */
>> + .cnt = 0,
>
>Ditto.
>
>> .max = INIT_PHYSMEM_REGIONS,
>> .name = "physmem",
>> };
>> @@ -356,7 +356,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;
>> type->regions[0].base = 0;
>> type->regions[0].size = 0;
>> type->regions[0].flags = 0;
>> @@ -598,17 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>> if (!size)
>> return 0;
>>
>> - /* special case for empty array */
>> - if (type->regions[0].size == 0) {
>> - WARN_ON(type->cnt != 1 || 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;
>> - return 0;
>> - }
>
>I'd keep the special case for empty array, just update it for new semantics
>of type->cnt.
>
>> -
>> /*
>> * The worst case is when new range overlaps all existing regions,
>> * then we'll need type->cnt + 1 empty regions in @type. So if
>> 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..4fa94b281280 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;
>
>Here as well, no need to initialize to 0.
>
Sure, I'd prepare v2.
>> 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.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mm/memblock: remove empty dummy entry
2024-04-04 10:13 ` Mike Rapoport
2024-04-04 12:44 ` Wei Yang
@ 2024-04-05 1:51 ` Wei Yang
1 sibling, 0 replies; 4+ messages in thread
From: Wei Yang @ 2024-04-05 1:51 UTC (permalink / raw)
To: Mike Rapoport; +Cc: Wei Yang, akpm, linux-mm, Paul Mackerras, Tejun Heo
On Thu, Apr 04, 2024 at 01:13:41PM +0300, Mike Rapoport wrote:
>On Wed, Apr 03, 2024 at 09:10:45AM +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@gmail.com>
>> CC: Paul Mackerras <paulus@ozlabs.org>
>> CC: Tejun Heo <tj@kernel.org>
>> CC: Mike Rapoport <rppt@kernel.org>
>> ---
>> mm/memblock.c | 18 +++---------------
>> tools/testing/memblock/tests/basic_api.c | 8 ++++----
>> tools/testing/memblock/tests/common.c | 4 ++--
>> 3 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index d09136e040d3..100d3b7ab499 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -114,12 +114,12 @@ 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.cnt = 0,
>
>No need to set memory.cnt to 0, this line can be just removed
>
>> .memory.max = INIT_MEMBLOCK_MEMORY_REGIONS,
>> .memory.name = "memory",
>>
>> .reserved.regions = memblock_reserved_init_regions,
>> - .reserved.cnt = 1, /* empty dummy entry */
>> + .reserved.cnt = 0,
>
>Ditto.
>
>> .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS,
>> .reserved.name = "reserved",
>>
>> @@ -130,7 +130,7 @@ 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 */
>> + .cnt = 0,
>
>Ditto.
>
>> .max = INIT_PHYSMEM_REGIONS,
>> .name = "physmem",
>> };
>> @@ -356,7 +356,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;
>> type->regions[0].base = 0;
>> type->regions[0].size = 0;
>> type->regions[0].flags = 0;
>> @@ -598,17 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>> if (!size)
>> return 0;
>>
>> - /* special case for empty array */
>> - if (type->regions[0].size == 0) {
>> - WARN_ON(type->cnt != 1 || 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;
>> - return 0;
>> - }
>
>I'd keep the special case for empty array, just update it for new semantics
>of type->cnt.
>
>> -
>> /*
>> * The worst case is when new range overlaps all existing regions,
>> * then we'll need type->cnt + 1 empty regions in @type. So if
>> 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..4fa94b281280 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;
>
>Here as well, no need to initialize to 0.
>
Oops, I found maybe we should keep this. Since reset_memblock_regions is
called on each new round test, we should clear it.
>> 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.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-05 1:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-03 9:10 [PATCH] mm/memblock: remove empty dummy entry Wei Yang
2024-04-04 10:13 ` Mike Rapoport
2024-04-04 12:44 ` Wei Yang
2024-04-05 1:51 ` Wei Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox