linux-kernel - Re: [PATCH] mm: hugetlb: fix hugetlb_cma_reserve() if CONFIG

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-next>] [day] [month] [year] [list]

Date:   Wed, 18 Mar 2020 17:16:25 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Roman Gushchin <guro@...com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        kernel-team@...com, linux-kernel@...r.kernel.org,
        Rik van Riel <riel@...riel.com>,
        Andreas Schaufler <andreas.schaufler@....de>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Guido Günther <agx@...xcpu.org>,
        Naresh Kamboju <naresh.kamboju@...aro.org>
Subject: Re: [PATCH] mm: hugetlb: fix hugetlb_cma_reserve() if CONFIG_NUMA
 isn't set

On Wed 18-03-20 08:34:24, Roman Gushchin wrote:
> If CONFIG_NUMA isn't set, there is no need to ensure that
> the hugetlb cma area belongs to a specific numa node.
> 
> min/max_low_pfn can be used for limiting the maximum size
> of the hugetlb_cma area.
> 
> Also for_each_mem_pfn_range() is defined only if
> CONFIG_HAVE_MEMBLOCK_NODE_MAP is set, and on arm (unlike most
> other architectures) it depends on CONFIG_NUMA. This makes the
> build fail if CONFIG_NUMA isn't set.

CONFIG_HAVE_MEMBLOCK_NODE_MAP has popped out as a problem several times
already. Is there any real reason we cannot make it unconditional?
Essentially make the functionality always enabled and drop the config?
The code below is ugly as hell. Just look at it. You have
for_each_node_state without any ifdefery but the having ifdef
CONFIG_NUMA. That just doesn't make any sense.

> Signed-off-by: Roman Gushchin <guro@...com>
> Reported-by: Andreas Schaufler <andreas.schaufler@....de>
> ---
>  mm/hugetlb.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 7a20cae7c77a..a6161239abde 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5439,16 +5439,21 @@ void __init hugetlb_cma_reserve(int order)
>  
>  	reserved = 0;
>  	for_each_node_state(nid, N_ONLINE) {
> -		unsigned long start_pfn, end_pfn;
>  		unsigned long min_pfn = 0, max_pfn = 0;
> -		int res, i;
> +		int res;
> +#ifdef CONFIG_NUMA
> +		unsigned long start_pfn, end_pfn;
> +		int i;
>  
>  		for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
>  			if (!min_pfn)
>  				min_pfn = start_pfn;
>  			max_pfn = end_pfn;
>  		}
> -
> +#else
> +		min_pfn = min_low_pfn;
> +		max_pfn = max_low_pfn;
> +#endif
>  		size = max(per_node, hugetlb_cma_size - reserved);
>  		size = round_up(size, PAGE_SIZE << order);
>  
> -- 
> 2.24.1

-- 
Michal Hocko
SUSE Labs