[PATCH 1/3] mm/mm_init.c: remove comments in zone_spanned_pages_in_node() and zone_absent_pages_in_node()

[PATCH 1/3] mm/mm_init.c: remove comments in zone_spanned_pages_in_node() and zone_absent_pages_in_node()

[Haifeng Xu]

Since commit 03e85f9d5f1f ("mm/page_alloc: Introduce free_area_init_core_hotplug"),
free_area_init_node() doesn't get called in hotplug path anymore, so
remove the comments related to it.

Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
 mm/mm_init.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 7f7f9c677854..cf55bba04b23 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1174,7 +1174,6 @@ static unsigned long __init zone_absent_pages_in_node(int nid,
 	unsigned long zone_start_pfn, zone_end_pfn;
 	unsigned long nr_absent;
 
-	/* When hotadd a new node from cpu_up(), the node should be empty */
 	if (!node_start_pfn && !node_end_pfn)
 		return 0;
 
@@ -1227,7 +1226,7 @@ static unsigned long __init zone_spanned_pages_in_node(int nid,
 {
 	unsigned long zone_low = arch_zone_lowest_possible_pfn[zone_type];
 	unsigned long zone_high = arch_zone_highest_possible_pfn[zone_type];
-	/* When hotadd a new node from cpu_up(), the node should be empty */
+
 	if (!node_start_pfn && !node_end_pfn)
 		return 0;
 
-- 
2.25.1

Re: [PATCH 1/3] mm/mm_init.c: remove comments in zone_spanned_pages_in_node() and zone_absent_pages_in_node()

[Mike Rapoport]

On Thu, May 25, 2023 at 04:01:48AM +0000, Haifeng Xu wrote:
> Since commit 03e85f9d5f1f ("mm/page_alloc: Introduce free_area_init_core_hotplug"),
> free_area_init_node() doesn't get called in hotplug path anymore, so
> remove the comments related to it.
> 
> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
> ---
>  mm/mm_init.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 7f7f9c677854..cf55bba04b23 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1174,7 +1174,6 @@ static unsigned long __init zone_absent_pages_in_node(int nid,
>  	unsigned long zone_start_pfn, zone_end_pfn;
>  	unsigned long nr_absent;
>  
> -	/* When hotadd a new node from cpu_up(), the node should be empty */
>  	if (!node_start_pfn && !node_end_pfn)
>  		return 0;

I think it's better to call calculate_node_totalpages only if node actually has 
memory and for node with no memory we just need to make sure all the node
and zone page counts are indeed set to 0.

Then the condition here and in zone_spanned_pages_in_node() can be removed
along with the comment and patch 2 won't be required at all.
  
> @@ -1227,7 +1226,7 @@ static unsigned long __init zone_spanned_pages_in_node(int nid,
>  {
>  	unsigned long zone_low = arch_zone_lowest_possible_pfn[zone_type];
>  	unsigned long zone_high = arch_zone_highest_possible_pfn[zone_type];
> -	/* When hotadd a new node from cpu_up(), the node should be empty */
> +
>  	if (!node_start_pfn && !node_end_pfn)
>  		return 0;
>  
> -- 
> 2.25.1
> 
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/3] mm/mm_init.c: remove comments in zone_spanned_pages_in_node() and zone_absent_pages_in_node()

[Haifeng Xu]

On 2023/5/25 16:24, Mike Rapoport wrote:
> On Thu, May 25, 2023 at 04:01:48AM +0000, Haifeng Xu wrote:
>> Since commit 03e85f9d5f1f ("mm/page_alloc: Introduce free_area_init_core_hotplug"),
>> free_area_init_node() doesn't get called in hotplug path anymore, so
>> remove the comments related to it.
>>
>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>> ---
>>  mm/mm_init.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>> index 7f7f9c677854..cf55bba04b23 100644
>> --- a/mm/mm_init.c
>> +++ b/mm/mm_init.c
>> @@ -1174,7 +1174,6 @@ static unsigned long __init zone_absent_pages_in_node(int nid,
>>  	unsigned long zone_start_pfn, zone_end_pfn;
>>  	unsigned long nr_absent;
>>  
>> -	/* When hotadd a new node from cpu_up(), the node should be empty */
>>  	if (!node_start_pfn && !node_end_pfn)
>>  		return 0;
> 
> I think it's better to call calculate_node_totalpages only if node actually has 
> memory and for node with no memory we just need to make sure all the node
> and zone page counts are indeed set to 0.

Yes, it's good advice.

> 
> Then the condition here and in zone_spanned_pages_in_node() can be removed
> along with the comment and patch 2 won't be required at all.

Regarding to patch 2, if the node has mmeory but contains zone with no memory,
it can avoid the calculation of absent pages.

I thik we can add a check that whether zone_end_pfn is equal to zone_start_pfn
in patch 3. If it's true, return 0 directly. So patch 2 can be dropped.

Thanks!

>   
>> @@ -1227,7 +1226,7 @@ static unsigned long __init zone_spanned_pages_in_node(int nid,
>>  {
>>  	unsigned long zone_low = arch_zone_lowest_possible_pfn[zone_type];
>>  	unsigned long zone_high = arch_zone_highest_possible_pfn[zone_type];
>> -	/* When hotadd a new node from cpu_up(), the node should be empty */
>> +
>>  	if (!node_start_pfn && !node_end_pfn)
>>  		return 0;
>>  
>> -- 
>> 2.25.1
>>
>>
>