All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end_pfn
@ 2021-01-27  9:32 Miaohe Lin
  2021-01-27  9:40 ` David Hildenbrand
  0 siblings, 1 reply; 3+ messages in thread
From: Miaohe Lin @ 2021-01-27  9:32 UTC (permalink / raw
  To: akpm; +Cc: linux-mm, linux-kernel, linmiaohe

Commit 108bcc96ef70 ("mm: add & use zone_end_pfn() and zone_spans_pfn()")
introduced the helper zone_end_pfn() to calculate the zone end pfn.  But
update_pgdat_span() forgot to use it. Use this helper and re-name local
variable zone_end_pfn to end_pfn to fix below compilation error:

mm/memory_hotplug.c: In function ‘update_pgdat_span’:
mm/memory_hotplug.c:448:32: error: called object ‘zone_end_pfn’ is not a
function or function pointer
   unsigned long zone_end_pfn = zone_end_pfn(zone);
                                ^~~~~~~~~~~~
mm/memory_hotplug.c:448:17: note: declared here
   unsigned long zone_end_pfn = zone_end_pfn(zone);
                 ^~~~~~~~~~~~

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory_hotplug.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 710e469fb3a1..0483db52b85f 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -493,20 +493,19 @@ static void update_pgdat_span(struct pglist_data *pgdat)
 
 	for (zone = pgdat->node_zones;
 	     zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
-		unsigned long zone_end_pfn = zone->zone_start_pfn +
-					     zone->spanned_pages;
+		unsigned long end_pfn = zone_end_pfn(zone);
 
 		/* No need to lock the zones, they can't change. */
 		if (!zone->spanned_pages)
 			continue;
 		if (!node_end_pfn) {
 			node_start_pfn = zone->zone_start_pfn;
-			node_end_pfn = zone_end_pfn;
+			node_end_pfn = end_pfn;
 			continue;
 		}
 
-		if (zone_end_pfn > node_end_pfn)
-			node_end_pfn = zone_end_pfn;
+		if (end_pfn > node_end_pfn)
+			node_end_pfn = end_pfn;
 		if (zone->zone_start_pfn < node_start_pfn)
 			node_start_pfn = zone->zone_start_pfn;
 	}
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end_pfn
  2021-01-27  9:32 [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end_pfn Miaohe Lin
@ 2021-01-27  9:40 ` David Hildenbrand
  2021-01-28  1:58   ` Miaohe Lin
  0 siblings, 1 reply; 3+ messages in thread
From: David Hildenbrand @ 2021-01-27  9:40 UTC (permalink / raw
  To: Miaohe Lin, akpm; +Cc: linux-mm, linux-kernel

On 27.01.21 10:32, Miaohe Lin wrote:
> Commit 108bcc96ef70 ("mm: add & use zone_end_pfn() and zone_spans_pfn()")
> introduced the helper zone_end_pfn() to calculate the zone end pfn.  But
> update_pgdat_span() forgot to use it. Use this helper and re-name local
> variable zone_end_pfn to end_pfn to fix below compilation error:
> 
> mm/memory_hotplug.c: In function ‘update_pgdat_span’:
> mm/memory_hotplug.c:448:32: error: called object ‘zone_end_pfn’ is not a
> function or function pointer
>     unsigned long zone_end_pfn = zone_end_pfn(zone);
>                                  ^~~~~~~~~~~~
> mm/memory_hotplug.c:448:17: note: declared here
>     unsigned long zone_end_pfn = zone_end_pfn(zone);
>                   ^~~~~~~~~~~~
> 

Please don't talk about compilation issues your changes introduce in 
that detail, that's just confusing and looks like something would 
already be broken. Please simplify to something like

"Let's use zone_end_pfn(zone). We have to rename the local variable to 
avoid an identifier clash (variable vs. function)."

With that

Reviewed-by: David Hildenbrand <david@redhat.com>

> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/memory_hotplug.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 710e469fb3a1..0483db52b85f 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -493,20 +493,19 @@ static void update_pgdat_span(struct pglist_data *pgdat)
>   
>   	for (zone = pgdat->node_zones;
>   	     zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
> -		unsigned long zone_end_pfn = zone->zone_start_pfn +
> -					     zone->spanned_pages;
> +		unsigned long end_pfn = zone_end_pfn(zone);
>   
>   		/* No need to lock the zones, they can't change. */
>   		if (!zone->spanned_pages)
>   			continue;
>   		if (!node_end_pfn) {
>   			node_start_pfn = zone->zone_start_pfn;
> -			node_end_pfn = zone_end_pfn;
> +			node_end_pfn = end_pfn;
>   			continue;
>   		}
>   
> -		if (zone_end_pfn > node_end_pfn)
> -			node_end_pfn = zone_end_pfn;
> +		if (end_pfn > node_end_pfn)
> +			node_end_pfn = end_pfn;
>   		if (zone->zone_start_pfn < node_start_pfn)
>   			node_start_pfn = zone->zone_start_pfn;
>   	}
> 


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end_pfn
  2021-01-27  9:40 ` David Hildenbrand
@ 2021-01-28  1:58   ` Miaohe Lin
  0 siblings, 0 replies; 3+ messages in thread
From: Miaohe Lin @ 2021-01-28  1:58 UTC (permalink / raw
  To: David Hildenbrand; +Cc: linux-mm, linux-kernel, Andrew Morton

Hi:
On 2021/1/27 17:40, David Hildenbrand wrote:
> On 27.01.21 10:32, Miaohe Lin wrote:
>> Commit 108bcc96ef70 ("mm: add & use zone_end_pfn() and zone_spans_pfn()")
>> introduced the helper zone_end_pfn() to calculate the zone end pfn.  But
>> update_pgdat_span() forgot to use it. Use this helper and re-name local
>> variable zone_end_pfn to end_pfn to fix below compilation error:
>>
>> mm/memory_hotplug.c: In function ‘update_pgdat_span’:
>> mm/memory_hotplug.c:448:32: error: called object ‘zone_end_pfn’ is not a
>> function or function pointer
>>     unsigned long zone_end_pfn = zone_end_pfn(zone);
>>                                  ^~~~~~~~~~~~
>> mm/memory_hotplug.c:448:17: note: declared here
>>     unsigned long zone_end_pfn = zone_end_pfn(zone);
>>                   ^~~~~~~~~~~~
>>
> 
> Please don't talk about compilation issues your changes introduce in that detail, that's just confusing and looks like something would already be broken. Please simplify to something like

I will keep this in mind. Thanks.

> 
> "Let's use zone_end_pfn(zone). We have to rename the local variable to avoid an identifier clash (variable vs. function)."
> 
> With that
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Many thanks for review and reply.

>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>   mm/memory_hotplug.c | 9 ++++-----
>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 710e469fb3a1..0483db52b85f 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -493,20 +493,19 @@ static void update_pgdat_span(struct pglist_data *pgdat)
>>         for (zone = pgdat->node_zones;
>>            zone < pgdat->node_zones + MAX_NR_ZONES; zone++) {
>> -        unsigned long zone_end_pfn = zone->zone_start_pfn +
>> -                         zone->spanned_pages;
>> +        unsigned long end_pfn = zone_end_pfn(zone);
>>             /* No need to lock the zones, they can't change. */
>>           if (!zone->spanned_pages)
>>               continue;
>>           if (!node_end_pfn) {
>>               node_start_pfn = zone->zone_start_pfn;
>> -            node_end_pfn = zone_end_pfn;
>> +            node_end_pfn = end_pfn;
>>               continue;
>>           }
>>   -        if (zone_end_pfn > node_end_pfn)
>> -            node_end_pfn = zone_end_pfn;
>> +        if (end_pfn > node_end_pfn)
>> +            node_end_pfn = end_pfn;
>>           if (zone->zone_start_pfn < node_start_pfn)
>>               node_start_pfn = zone->zone_start_pfn;
>>       }
>>
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-28  1:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-27  9:32 [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end_pfn Miaohe Lin
2021-01-27  9:40 ` David Hildenbrand
2021-01-28  1:58   ` Miaohe Lin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.