From: Michal Hocko <mhocko@kernel.org>
To: Roman Gushchin <guro@fb.com>
Cc: Aslan Bakirov <aslan@fb.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, kernel-team@fb.com, riel@surriel.com,
hannes@cmpxchg.org
Subject: Re: [PATCH 2/2] mm: hugetlb: Use node interface of cma
Date: Fri, 27 Mar 2020 16:13:39 +0100 [thread overview]
Message-ID: <20200327151339.GF10449@dhcp22.suse.cz> (raw)
In-Reply-To: <20200327144155.GA194089@carbon.DHCP.thefacebook.com>
On Fri 27-03-20 07:41:55, Roman Gushchin wrote:
> On Fri, Mar 27, 2020 at 09:06:10AM +0100, Michal Hocko wrote:
> > 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.
>
> I'm glad you liked it! I agree, it's much nicer now, thanks to Aslan!
>
> I think it's simpler to keep it as a separate patch, because there was
> already a fix by Randy Dunlap on top of my original version.
Why would be squashing all those into a single one be a problem. I will
not insist of course but I would much rather see a single patch which is
easy to review and doesn't contain any intermediate hacks to prevent
from compile time issues than 3 patches with the last one removing hacks
and a large part of the implementation. You can easily record the
multiple authors can be easily recorded in the s-o-b chain.
> > > Signed-off-by: Aslan Bakirov <aslan@fb.com>
>
> Acked-by: Roman Gushchin <guro@fb.com>
>
> Thanks!
>
> >
> > 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
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2020-03-27 15:13 UTC|newest]
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
2020-03-27 14:41 ` Roman Gushchin
2020-03-27 15:13 ` Michal Hocko [this message]
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=20200327151339.GF10449@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).