From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: ** X-Spam-Status: No, score=2.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1E1BC2BA12 for ; Thu, 2 Apr 2020 22:27:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 58848206F8 for ; Thu, 2 Apr 2020 22:27:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TGL9a2Wp" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 58848206F8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D3B538E0008; Thu, 2 Apr 2020 18:27:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CEB728E0007; Thu, 2 Apr 2020 18:27:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C01D68E0008; Thu, 2 Apr 2020 18:27:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0211.hostedemail.com [216.40.44.211]) by kanga.kvack.org (Postfix) with ESMTP id A967E8E0007 for ; Thu, 2 Apr 2020 18:27:03 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 56A525DED for ; Thu, 2 Apr 2020 22:27:03 +0000 (UTC) X-FDA: 76664351526.25.net28_3d8f7f0757e56 X-HE-Tag: net28_3d8f7f0757e56 X-Filterd-Recvd-Size: 11491 Received: from mail-qt1-f194.google.com (mail-qt1-f194.google.com [209.85.160.194]) by imf30.hostedemail.com (Postfix) with ESMTP for ; Thu, 2 Apr 2020 22:27:02 +0000 (UTC) Received: by mail-qt1-f194.google.com with SMTP id a5so4886832qtw.10 for ; Thu, 02 Apr 2020 15:27:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SkKZYZwHyDIc8tVdiChMb7XTJ+74HhRVxIjNE7pkEIs=; b=TGL9a2WpdqCq8wtEH8QzS4fxwYf662vvUY+UrR4f1A6t9wteD4BzFp+09yNOX3na3j h3vMtrTAlMckog+YO4W1IL8hbNfa/UNxCFjWJ/K13uK3kbQrYhRaHm/woj0dwBXJmm1T fwAlhGi7zA8s60kpJsBIsDdnDlI+V8QSJZfUzGp9+gXINunBjCTvkCS9vO40M2sGwuQY VI06oeSzo3pgTGCx2bbLjrKxVbiMEPgrXvr27woSvmqX4bMg3u78uvFlFbq61aBhG5G8 Gg5BPZRmQgYhj7t3TDyDsntpDrtc/wC7QErOnMLx61HzKU/XVFccs9G7ddpXrew/Kg8k V09A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SkKZYZwHyDIc8tVdiChMb7XTJ+74HhRVxIjNE7pkEIs=; b=ac/hl3I7DFSSknUMgIci99MYmTDWjbykDwEk+Likox7cwDTc2Ro/qTMhcpwocrDfzm Mz96qwHyN+/2p7CffegL8lDrHhMR8DoR3BfPAbY+v3JMu+YYB2QpvRJnUHdPWE+G9voL KpTP5vsGEg3w4HHBDmbWHEfPHRmxfnPWt9pEjrfHMpiW3uVIry+sbM6/kQJr/lZIXjb/ jNSC6VzYiGd9FS9r7az6ATzcpc9DJq5AegM8upCU3R2/6uM45uOR0AYTGdQ9FOsNwMQA zRs4/ZrQgzf9Mj3QaCpzNsTAzxWHp+zyAu4flb+26QLv2rLb5Ku5nOm4q4agYB30ckAJ qaPA== X-Gm-Message-State: AGi0PuYFFEQop6FVpzvTon9l1XV5xuyPJtuuyKmHIGvR0aa4aVu+fbc5 rPhWWuTcC2Xfe9IOpfhZkyrXsGxMbUkLtukiGz0= X-Google-Smtp-Source: APiQypIlzEz9ONMKLvoEfsLd4LqIAz7tHQMYZw+8NIFgK9qS1myNJr8ZlT4p2gT1N0oZmKMRqUkMUCi62bzag/l/hGs= X-Received: by 2002:ac8:2d88:: with SMTP id p8mr5424009qta.346.1585866422441; Thu, 02 Apr 2020 15:27:02 -0700 (PDT) MIME-Version: 1.0 References: <20200402215810.1898270-1-aslan@fb.com> <20200402220713.GO21484@bombadil.infradead.org> In-Reply-To: <20200402220713.GO21484@bombadil.infradead.org> From: Aslan Bakirov Date: Thu, 2 Apr 2020 23:26:51 +0100 Message-ID: Subject: Re: [PATCH v2] mm: cma: NUMA node interface To: Matthew Wilcox Cc: Aslan Bakirov , akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel-team@fb.com, riel@surriel.com, Roman Gushchin , mhocko@kernel.org, hannes@cmpxchg.org Content-Type: multipart/alternative; boundary="000000000000c5595505a2564c88" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --000000000000c5595505a2564c88 Content-Type: text/plain; charset="UTF-8" On Thu, Apr 2, 2020 at 11:07 PM Matthew Wilcox wrote: > On Thu, Apr 02, 2020 at 02:58:10PM -0700, Aslan Bakirov wrote: > > I've noticed that there is no interfaces exposed by CMA which would let > me > > to declare contigous memory on particular NUMA node. > > Do you have a user for this functionality? > Yes, we are using this functionality in second part of this patchset, We use this interface in hugetlb_cma_reserve() to reserve CMA on specific node. > > > +++ b/include/linux/cma.h > > @@ -24,10 +24,20 @@ extern phys_addr_t cma_get_base(const struct cma > *cma); > > extern unsigned long cma_get_size(const struct cma *cma); > > extern const char *cma_get_name(const struct cma *cma); > > > > -extern int __init cma_declare_contiguous(phys_addr_t base, > > +extern int __init cma_declare_contiguous_nid(phys_addr_t base, > > phys_addr_t size, phys_addr_t limit, > > phys_addr_t alignment, unsigned int order_per_bit, > > - bool fixed, const char *name, struct cma > **res_cma); > > + bool fixed, const char *name, struct cma **res_cma, > > + int nid); > > +static inline int __init cma_declare_contiguous(phys_addr_t base, > > + phys_addr_t size, phys_addr_t limit, > > + phys_addr_t alignment, unsigned int order_per_bit, > > + bool fixed, const char *name, struct cma **res_cma) > > + { > > + return cma_declare_contiguous_nid(base, > size, > > + limit, alignment, > order_per_bit, > > + fixed, name, res_cma, > NUMA_NO_NODE); > > + } > > ... what even is this indentation? > > Sorry, about indentation problems, fix is coming in the next version. > +phys_addr_t memblock_alloc_range_nid(phys_addr_t size, > > + phys_addr_t align, phys_addr_t > start, > > + phys_addr_t end, int nid, bool > exact_nid); > > >80 columns. checkpatch should warn you of nits like this. > > > if (base < highmem_start && limit > highmem_start) { > > - addr = memblock_phys_alloc_range(size, alignment, > > - highmem_start, > limit); > > + addr = memblock_alloc_range_nid(size, alignment, > > + highmem_start, > limit, nid, false); > > The deep indentation makes it hard to add new parameters. I'd do it as: > addr = memblock_alloc_range_nid(size, alignment, > highmem_start, limit, nid, false); > > > --000000000000c5595505a2564c88 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Apr 2, 2020 at 11:07 PM Matth= ew Wilcox <willy@infradead.org> wrote:
On = Thu, Apr 02, 2020 at 02:58:10PM -0700, Aslan Bakirov wrote:
> I've noticed that there is no interfaces exposed by CMA which woul= d let me
> to declare contigous memory on particular NUMA node.

Do you have a user for this functionality?

> +++ b/include/linux/cma.h
> @@ -24,10 +24,20 @@ extern phys_addr_t cma_get_base(const struct cma *= cma);
>=C2=A0 extern unsigned long cma_get_size(const struct cma *cma);
>=C2=A0 extern const char *cma_get_name(const struct cma *cma);
>=C2=A0
> -extern int __init cma_declare_contiguous(phys_addr_t base,
> +extern int __init cma_declare_contiguous_nid(phys_addr_t base,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0phys_addr_t size, phys_addr_t limit,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0phys_addr_t alignment, unsigned int order_per_bit,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0bool fixed, const char *name, struct cma **res_cma);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0bool fixed, const char *name, struct cma **res_cma,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0int nid);
> +static inline int __init cma_declare_contiguous(phys_addr_t base,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0phys_addr_t size, phys_addr_t limit,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0phys_addr_t alignment, unsigned int order_per_bit,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0bool fixed, const char *name, struct cma **res_cma)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0{
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return cma_declare_contiguous_nid(base, = size,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0limit, alignment, order_per_bit,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0fixed, name, res_cma, NUMA_NO_NODE);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0}

... what even is this indentation?

> +phys_addr_t memblock_alloc_range_nid(phys_addr_t size,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0phys_= addr_t align, phys_addr_t start,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0phys_= addr_t end, int nid, bool exact_nid);

>80 columns.=C2=A0 checkpatch should warn you of nits like this.

>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (base < hi= ghmem_start && limit > highmem_start) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0addr =3D memblock_phys_alloc_range(size, alignment,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 highmem_start, limit);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0addr =3D memblock_alloc_range_nid(size, alignment,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 highmem_start, limit, nid, fa= lse);

The deep indentation makes it hard to add new parameters.=C2=A0 I'd do = it as:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 addr =3D memblock_alloc_range_nid(size, alignment,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 highmem_= start, limit, nid, false);


--000000000000c5595505a2564c88--