Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Aslan Bakirov <aslan@fb.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, kernel-team@fb.com, riel@surriel.com,
	guro@fb.com, hannes@cmpxchg.org
Subject: Re: [PATCH 2/2] mm: hugetlb: Use node interface of cma
Date: Fri, 27 Mar 2020 09:06:10 +0100
Message-ID: <20200327080610.GV27965@dhcp22.suse.cz> (raw)
In-Reply-To: <20200326212718.3798742-2-aslan@fb.com>

On Thu 26-03-20 14:27:18, Aslan Bakirov wrote:
> With introduction of numa node interface for CMA, this patch is for using that
> interface for allocating memory on numa nodes if NUMA is configured.
> This will be more efficient  and cleaner because first, instead of iterating
> mem range of each numa node, cma_declare_contigueous_nid() will do
> its own address finding if we pass 0 for  both min_pfn and max_pfn,
> second, it can also handle caseswhere NUMA is not configured
> by passing NUMA_NO_NODE as an argument.
> 
> In addition, checking if desired size of memory is available or not,
> is happening in cma_declare_contiguous_nid()  because base and
> limit will be determined there, since 0(any) for  base and
> 0(any) for limit is passed as argument to the function.

This looks much better than the original patch. Can we simply squash
your and Roman's patch in the mmotm tree and post it for the review in
one piece? It would be slightly easier to review that way.

> Signed-off-by: Aslan Bakirov <aslan@fb.com>

Thanks!

> ---
>  mm/hugetlb.c | 40 +++++++++++-----------------------------
>  1 file changed, 11 insertions(+), 29 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index b9f0c903c4cf..62989220c4ff 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5573,42 +5573,24 @@ void __init hugetlb_cma_reserve(int order)
>  
>  	reserved = 0;
>  	for_each_node_state(nid, N_ONLINE) {
> -		unsigned long min_pfn = 0, max_pfn = 0;
>  		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 = min(per_node, hugetlb_cma_size - reserved);
>  		size = round_up(size, PAGE_SIZE << order);
> -
> -		if (size > ((max_pfn - min_pfn) << PAGE_SHIFT) / 2) {
> -			pr_warn("hugetlb_cma: cma_area is too big, please try less than %lu MiB\n",
> -				round_down(((max_pfn - min_pfn) << PAGE_SHIFT) *
> -					   nr_online_nodes / 2 / SZ_1M,
> -					   PAGE_SIZE << order));
> -			break;
> -		}
> -
> -		res = cma_declare_contiguous(PFN_PHYS(min_pfn), size,
> -					     PFN_PHYS(max_pfn),
> +		
> +		
> +#ifndef CONFIG_NUMA
> +		nid = NUMA_NO_NODE
> +#endif		
> +		res = cma_declare_contiguous_nid(0, size,
> +					     0, 
>  					     PAGE_SIZE << order,
>  					     0, false,
> -					     "hugetlb", &hugetlb_cma[nid]);
> +					     "hugetlb", &hugetlb_cma[nid], nid);		
> +
>  		if (res) {
> -			phys_addr_t begpa = PFN_PHYS(min_pfn);
> -			phys_addr_t endpa = PFN_PHYS(max_pfn);
> -			pr_warn("%s: reservation failed: err %d, node %d, [%pap, %pap)\n",
> -				__func__, res, nid, &begpa, &endpa);
> +			pr_warn("%s: reservation failed: err %d, node %d\n",
> +				__func__, res, nid);
>  			break;
>  		}
>  
> -- 
> 2.17.1

-- 
Michal Hocko
SUSE Labs


  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26 21:27 [PATCH 1/2] mm: cma: NUMA node interface Aslan Bakirov
2020-03-26 21:27 ` [PATCH 2/2] mm: hugetlb: Use node interface of cma Aslan Bakirov
2020-03-27  8:06   ` Michal Hocko [this message]
2020-03-27 14:41     ` Roman Gushchin
2020-03-27 15:13       ` Michal Hocko
2020-04-02 15:20         ` Vlastimil Babka
2020-04-02 17:24           ` Michal Hocko
2020-03-27  8:02 ` [PATCH 1/2] mm: cma: NUMA node interface Michal Hocko
2020-04-02 15:48 ` Vlastimil Babka
2020-04-02 22:03   ` Aslan Bakirov
2020-04-03 15:09   ` Roman Gushchin
2020-04-03 10:18 Aslan Bakirov
2020-04-03 10:18 ` [PATCH 2/2] mm: hugetlb: Use node interface of cma Aslan Bakirov
2020-04-03 10:36   ` Michal Hocko
2020-04-03 11:21     ` Aslan Bakirov
2020-04-03 11:18 [PATCH 1/2] mm: cma: NUMA node interface Aslan Bakirov
2020-04-03 11:18 ` [PATCH 2/2] mm: hugetlb: Use node interface of cma Aslan Bakirov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200327080610.GV27965@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=aslan@fb.com \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@surriel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git