linux-kernel - Re: [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to get end

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Jan 2021 10:40:23 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/memory_hotplug: use helper function zone_end_pfn() to
 get end_pfn

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@...hat.com>

> Signed-off-by: Miaohe Lin <linmiaohe@...wei.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