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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32AF1C433F5 for ; Mon, 27 Sep 2021 10:40:28 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C12A761056 for ; Mon, 27 Sep 2021 10:40:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C12A761056 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.196687.349624 (Exim 4.92) (envelope-from ) id 1mUo3H-0001g1-ME; Mon, 27 Sep 2021 10:40:03 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 196687.349624; Mon, 27 Sep 2021 10:40:03 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mUo3H-0001fO-Ib; Mon, 27 Sep 2021 10:40:03 +0000 Received: by outflank-mailman (input) for mailman id 196687; Mon, 27 Sep 2021 10:40:02 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mUo3G-0001V3-Es for xen-devel@lists.xenproject.org; Mon, 27 Sep 2021 10:40:02 +0000 Received: from mail-oi1-x231.google.com (unknown [2607:f8b0:4864:20::231]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 008d67c5-ae9e-4f5b-994c-51083a48528e; Mon, 27 Sep 2021 10:39:59 +0000 (UTC) Received: by mail-oi1-x231.google.com with SMTP id e24so15062268oig.11 for ; Mon, 27 Sep 2021 03:39:59 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 008d67c5-ae9e-4f5b-994c-51083a48528e DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OB4tnKWET1q9U/7EbB1ovFbkJ0Rz+j6x5E3x6eSDVbE=; b=T+WuS8wW3hq4DovQKI4udHWt/6L2K9EMrKp4B1uYYcXk+FwZXJXDROmCmemKCrHNiG Php1djzDkfNWyKGrVBaDhhYzalTy8eo29EQiOK8R2eU5FpKhpSwHuu2emxZRluYyM1Bx XY/RdqzilRv2hrUH37fHVZ7WIBxD4i9l+8I+Qpv/s97/4JEC8L3TBF/T/pEvXlX7WB5M CF8EKIm/0ylh+kz0awL5gCQ2ZPKgJxOZKCpuXqlk+jIhECuzK0SgaSqdOnjesjeC26hN luxu4U9TyJJBk2EWvyRrHIA2mfYn4WxqfKafYWoLmsn87KqNyMKdtr3IajD036g+Xl/D Sbbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OB4tnKWET1q9U/7EbB1ovFbkJ0Rz+j6x5E3x6eSDVbE=; b=cnn3mFqtXt/DPu7EMKyxB2fO7jhYGendUT3ScNl0DyRg2FMUO+LU1gUkLO7d1w7SH9 q7W+LvoMdfdMNk7DLOL8e2Op5kk8ARyAOIMyL1RuyB24A0mgvC+wmf37QhkfwCwTDTaI NuwJ1sehRMaWQ/ZjE7QGTTKCrIJqN086JtQgCFIfBSDUnS6ahH7VyEJGrheh6x2DV/X9 wFxwfR4YyX8pDd/5xnmNaMMT23krgLYqA1bAzag4NdaREeabvDau2M/LIaVxwa68se0P 0Vc0LGlrOkaKWcnNP29+A/Q56m1nDN7l68ymppgOLyI2wMxSnbSSAlrjm0XJzEvBkrJl BbAg== X-Gm-Message-State: AOAM533Irjl/41OqaCETWTQUi6+CgjfeNMW756XvevmqEvGGB6yyhg+S IfuwiYOY69gjKLLQIKN5BC7uFk9tjs/wCtr0kqs= X-Google-Smtp-Source: ABdhPJwxmU8SQv0k4R5Stztam8sQ1v17wQ5MXWUykj5HTFo3qZhEm1MNf3uoT8in6MXBWNb/m+UK9d03LYDyMU2JtX4= X-Received: by 2002:aca:c6c7:: with SMTP id w190mr11376304oif.96.1632739199132; Mon, 27 Sep 2021 03:39:59 -0700 (PDT) MIME-Version: 1.0 References: <20210923120236.3692135-1-wei.chen@arm.com> <20210923120236.3692135-23-wei.chen@arm.com> In-Reply-To: From: Julien Grall Date: Mon, 27 Sep 2021 14:39:48 +0400 Message-ID: Subject: Re: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override default NR_NODE_MEMBLKS To: Wei Chen Cc: Julien Grall , Stefano Stabellini , xen-devel , Bertrand Marquis , Jan Beulich , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Andrew Cooper Content-Type: multipart/alternative; boundary="000000000000f9a62205ccf7b7d3" --000000000000f9a62205ccf7b7d3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 27 Sep 2021, 12:22 Wei Chen, wrote: > Hi Julien, > > From: Julien Grall > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 15:36 > To: Wei Chen > Cc: Stefano Stabellini ; xen-devel < > xen-devel@lists.xenproject.org>; Bertrand Marquis < > Bertrand.Marquis@arm.com>; Jan Beulich ; Roger Pau > Monn=C3=A9 ; Andrew Cooper > Subject: Re: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override default > NR_NODE_MEMBLKS > > > On Mon, 27 Sep 2021, 08:53 Wei Chen, wrote: > Hi Julien, > > > -----Original Message----- > > From: Xen-devel On > Behalf Of Wei > > Chen > > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 14:46 > > To: Stefano Stabellini > > Cc: mailto:xen-devel@lists.xenproject.org; mailto:julien@xen.org; > Bertrand Marquis > > ; mailto:jbeulich@suse.com; mailto: > roger.pau@citrix.com; > > mailto:andrew.cooper3@citrix.com > > Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override defaul= t > > NR_NODE_MEMBLKS > > > > Hi Stefano, Julien, > > > > > -----Original Message----- > > > From: Stefano Stabellini > > > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 13:00 > > > To: Wei Chen > > > Cc: Stefano Stabellini ; xen- > > > mailto:devel@lists.xenproject.org; mailto:julien@xen.org; Bertrand > Marquis > > > ; mailto:jbeulich@suse.com; mailto: > roger.pau@citrix.com; > > > mailto:andrew.cooper3@citrix.com > > > Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override > default > > > NR_NODE_MEMBLKS > > > > > > +x86 maintainers > > > > > > On Mon, 27 Sep 2021, Wei Chen wrote: > > > > > -----Original Message----- > > > > > From: Stefano Stabellini > > > > > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 11:26 > > > > > To: Wei Chen > > > > > Cc: Stefano Stabellini ; xen- > > > > > mailto:devel@lists.xenproject.org; mailto:julien@xen.org; > Bertrand Marquis > > > > > > > > > > Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override > > > default > > > > > NR_NODE_MEMBLKS > > > > > > > > > > On Sun, 26 Sep 2021, Wei Chen wrote: > > > > > > > -----Original Message----- > > > > > > > From: Stefano Stabellini > > > > > > > Sent: 2021=E5=B9=B49=E6=9C=8824=E6=97=A5 9:35 > > > > > > > To: Wei Chen > > > > > > > Cc: mailto:xen-devel@lists.xenproject.org; mailto: > sstabellini@kernel.org; > > > > > mailto:julien@xen.org; > > > > > > > Bertrand Marquis > > > > > > > Subject: Re: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to > override > > > > > default > > > > > > > NR_NODE_MEMBLKS > > > > > > > > > > > > > > On Thu, 23 Sep 2021, Wei Chen wrote: > > > > > > > > As a memory range described in device tree cannot be split > > > across > > > > > > > > multiple nodes. So we define NR_NODE_MEMBLKS as NR_MEM_BANK= S > > in > > > > > > > > arch header. > > > > > > > > > > > > > > This statement is true but what is the goal of this patch? Is > it > > > to > > > > > > > reduce code size and memory consumption? > > > > > > > > > > > > > > > > > > > No, when Julien and I discussed this in last version[1], we > hadn't > > > > > thought > > > > > > so deeply. We just thought a memory range described in DT canno= t > > be > > > > > split > > > > > > across multiple nodes. So NR_MEM_BANKS should be equal to > > > NR_MEM_BANKS. > > > > > > > > > > > > https://lists.xenproject.org/archives/html/xen-devel/2021- > > > > > 08/msg00974.html > > > > > > > > > > > > > I am asking because NR_MEM_BANKS is 128 and > > > > > > > NR_NODE_MEMBLKS=3D2*MAX_NUMNODES which is 64 by default so ag= ain > > > > > > > NR_NODE_MEMBLKS is 128 before this patch. > > > > > > > > > > > > > > In other words, this patch alone doesn't make any difference; > at > > > least > > > > > > > doesn't make any difference unless CONFIG_NR_NUMA_NODES is > > > increased. > > > > > > > > > > > > > > So, is the goal to reduce memory usage when > CONFIG_NR_NUMA_NODES > > > is > > > > > > > higher than 64? > > > > > > > > > > > > > > > > > > > I also thought about this problem when I was writing this patch= . > > > > > > CONFIG_NR_NUMA_NODES is increasing, but NR_MEM_BANKS is a fixed > > > > > > value, then NR_MEM_BANKS can be smaller than CONFIG_NR_NUMA_NOD= ES > > > > > > at one point. > > > > > > > > > > > > But I agree with Julien's suggestion, NR_MEM_BANKS and > > > NR_NODE_MEMBLKS > > > > > > must be aware of each other. I had thought to add some ASSERT > > check, > > > > > > but I don't know how to do it better. So I post this patch for > > more > > > > > > suggestion. > > > > > > > > > > OK. In that case I'd say to get rid of the previous definition of > > > > > NR_NODE_MEMBLKS as it is probably not necessary, see below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > And keep default NR_NODE_MEMBLKS in common header > > > > > > > > for those architectures NUMA is disabled. > > > > > > > > > > > > > > This last sentence is not accurate: on x86 NUMA is enabled an= d > > > > > > > NR_NODE_MEMBLKS is still defined in xen/include/xen/numa.h > > (there > > > is > > > > > no > > > > > > > x86 definition of it) > > > > > > > > > > > > > > > > > > > Yes. > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Wei Chen > > > > > > > > --- > > > > > > > > xen/include/asm-arm/numa.h | 8 +++++++- > > > > > > > > xen/include/xen/numa.h | 2 ++ > > > > > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm- > > > arm/numa.h > > > > > > > > index 8f1c67e3eb..21569e634b 100644 > > > > > > > > --- a/xen/include/asm-arm/numa.h > > > > > > > > +++ b/xen/include/asm-arm/numa.h > > > > > > > > @@ -3,9 +3,15 @@ > > > > > > > > > > > > > > > > #include > > > > > > > > > > > > > > > > +#include > > > > > > > > + > > > > > > > > typedef u8 nodeid_t; > > > > > > > > > > > > > > > > -#ifndef CONFIG_NUMA > > > > > > > > +#ifdef CONFIG_NUMA > > > > > > > > + > > > > > > > > +#define NR_NODE_MEMBLKS NR_MEM_BANKS > > > > > > > > + > > > > > > > > +#else > > > > > > > > > > > > > > > > /* Fake one node for now. See also node_online_map. */ > > > > > > > > #define cpu_to_node(cpu) 0 > > > > > > > > diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.= h > > > > > > > > index 1978e2be1b..1731e1cc6b 100644 > > > > > > > > --- a/xen/include/xen/numa.h > > > > > > > > +++ b/xen/include/xen/numa.h > > > > > > > > @@ -12,7 +12,9 @@ > > > > > > > > #define MAX_NUMNODES 1 > > > > > > > > #endif > > > > > > > > > > > > > > > > +#ifndef NR_NODE_MEMBLKS > > > > > > > > #define NR_NODE_MEMBLKS (MAX_NUMNODES*2) > > > > > > > > +#endif > > > > > > > > > > This one we can remove it completely right? > > > > > > > > How about define NR_MEM_BANKS to: > > > > #ifdef CONFIG_NR_NUMA_NODES > > > > #define NR_MEM_BANKS (CONFIG_NR_NUMA_NODES * 2) > > > > #else > > > > #define NR_MEM_BANKS 128 > > > > #endif > > > > for both x86 and Arm. For those architectures do not support or > enable > > > > NUMA, they can still use "NR_MEM_BANKS 128". And replace all > > > NR_NODE_MEMBLKS > > > > in NUMA code to NR_MEM_BANKS to remove NR_NODE_MEMBLKS completely. > > > > In this case, NR_MEM_BANKS can be aware of the changes of > > > CONFIG_NR_NUMA_NODES. > > > > > > x86 doesn't have NR_MEM_BANKS as far as I can tell. I guess you also > > > meant to rename NR_NODE_MEMBLKS to NR_MEM_BANKS? > > > > > > > Yes. > > > > > But NR_MEM_BANKS is not directly related to CONFIG_NR_NUMA_NODES > because > > > there can be many memory banks for each numa node, certainly more tha= n > > > 2. The existing definition on x86: > > > > > > #define NR_NODE_MEMBLKS (MAX_NUMNODES*2) > > > > > > Doesn't make a lot of sense to me. Was it just an arbitrary limit for > > > the lack of a better way to set a maximum? > > > > > > > At that time, this was probably the most cost-effective approach. > > Enough and easy. But, if more nodes need to be supported in the > > future, it may bring more memory blocks. And this maximum value > > might not apply. The maximum may need to support dynamic extension. > > > > > > > > On the other hand, NR_MEM_BANKS and NR_NODE_MEMBLKS seem to be relate= d. > > > In fact, what's the difference? > > > > > > NR_MEM_BANKS is the max number of memory banks (with or without > > > numa-node-id). > > > > > > NR_NODE_MEMBLKS is the max number of memory banks with NUMA support > > > (with numa-node-id)? > > > > > > They are basically the same thing. On ARM I would just do: > > > > > > > Probably not, NR_MEM_BANKS will count those memory ranges without > > numa-node-id in boot memory parsing stage (process_memory_node or > > EFI parser). But NR_NODE_MEMBLKS will only count those memory ranges > > with numa-node-id. > > > > > #define NR_NODE_MEMBLKS MAX(NR_MEM_BANKS, (CONFIG_NR_NUMA_NODES * 2)) > > > > > > > > > Quote Julien's comment from HTML email to here: > > " As you wrote above, the second part of the MAX is totally arbitrary. > > In fact, it is very likely than if you have more than 64 nodes, you may > > need a lot more than 2 regions per node. > > > > So, for Arm, I would just define NR_NODE_MEMBLKS as an alias to > NR_MEM_BANKS > > so it can be used by common code. > > " > > > > > But here comes the problem: > > > How can we set the NR_MEM_BANKS maximum value, 128 seems an arbitrary > too? > > > > This is based on hardware we currently support (the last time we bumped > the value was, IIRC, for Thunder-X). In the case of booting UEFI, we can > get a lot of small ranges as we discover the RAM using the UEFI memory ma= p. > > > > Thanks for the background. > > > > > > If #define NR_MEM_BANKS (CONFIG_NR_NUMA_NODES * N)? And what N should > be. > > > > N would have to be the maximum number of ranges you can find in a NUMA > node. > > > > We would also need to make sure this doesn't break existing platforms. > So N would have to be quite large or we need a MAX as Stefano suggested. > > > > But I would prefer to keep the existing 128 and allow to configure it a= t > build time (not necessarily in this series). This avoid to have different > way to define the value based NUMA vs non-NUMA. > > In this case, can we use Stefano's > "#define NR_NODE_MEMBLKS MAX(NR_MEM_BANKS, (CONFIG_NR_NUMA_NODES * 2))" > in next version. If yes, should we change x86 part? Because NR_MEM_BANKS > has not been defined in x86. What I meant by configuring dynamically is allowing NR_MEM_BANKS to be set by the user. The second part of the MAX makes no sense to me (at least on Arm). So I really prefer if this is not part of the initial version. We can refine the value, or introduce the MAX in the future if we have a justification for it. > > > And maybe the definition could be common with x86 if we define > > > NR_MEM_BANKS to 128 on x86 too. > > > > Julien had comment here, I will continue in that email. > --000000000000f9a62205ccf7b7d3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, 27 Sep 2021, 12:22 Wei Chen, <Wei.Chen@arm.com> wrote:
Hi Julien,

From: Julien Grall <julien.grall.oss@gmail.com>
Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 15:36
To: Wei Chen <Wei.Chen@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>; xen-devel &l= t;xen-devel@lists.xenproject.org>; Bertrand Marquis &l= t;Bertrand.Marquis@arm.com>; Jan Beulich <jbeulich@suse.com<= /a>>; Roger Pau Monn=C3=A9 <roger.pau@citrix.com>; Andrew Co= oper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override default NR= _NODE_MEMBLKS


On Mon, 27 Sep 2021, 08:53 Wei Chen, <mailto:Wei.Chen@arm.com> wrot= e:
Hi Julien,

> -----Original Message-----
> From: Xen-devel <mailto:xen-devel-bounces@lists= .xenproject.org> On Behalf Of Wei
> Chen
> Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 14:46
> To: Stefano Stabellini <mailto:sstabellini@kernel.org> > Cc: mailto:xen-devel@lists.xenproject.org; mailto:julien@x= en.org; Bertrand Marquis
> <mailto:Bertrand.Marquis@arm.com>; mailto:jbeulich@suse= .com; mailto:roger.pau@citrix.com;
> mailto:andrew.cooper3@citrix.com
> Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override defau= lt
> NR_NODE_MEMBLKS
>
> Hi Stefano, Julien,
>
> > -----Original Message-----
> > From: Stefano Stabellini <mailto:sstabellini@kernel.org>
> > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 13:00
> > To: Wei Chen <mailto:Wei.Chen@arm.com>
> > Cc: Stefano Stabellini <mailto:sstabellini@kernel.org&= gt;; xen-
> > mailto:devel@lists.xenproject.org; mailto:julien@xen.org<= /a>; Bertrand Marquis
> > <mailto:Bertrand.Marquis@arm.com>; mailto:jbeulich= @suse.com; mailto:roger.pau@citrix.com;
> > mailto:andrew.cooper3@citrix.com
> > Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to override = default
> > NR_NODE_MEMBLKS
> >
> > +x86 maintainers
> >
> > On Mon, 27 Sep 2021, Wei Chen wrote:
> > > > -----Original Message-----
> > > > From: Stefano Stabellini <mailto:sstabellini@ker= nel.org>
> > > > Sent: 2021=E5=B9=B49=E6=9C=8827=E6=97=A5 11:26
> > > > To: Wei Chen <mailto:Wei.Chen@arm.com>
> > > > Cc: Stefano Stabellini <mailto:sstabellini@kerne= l.org>; xen-
> > > > mailto:devel@lists.xenproject.org; mailto:<= a href=3D"mailto:julien@xen.org" target=3D"_blank" rel=3D"noreferrer">julie= n@xen.org; Bertrand Marquis
> > > > <mailto:Bertrand.Marquis@arm.com>
> > > > Subject: RE: [PATCH 22/37] xen/arm: use NR_MEM_BANKS to= override
> > default
> > > > NR_NODE_MEMBLKS
> > > >
> > > > On Sun, 26 Sep 2021, Wei Chen wrote:
> > > > > > -----Original Message-----
> > > > > > From: Stefano Stabellini <mailto:sst= abellini@kernel.org>
> > > > > > Sent: 2021=E5=B9=B49=E6=9C=8824=E6=97=A5 9:35=
> > > > > > To: Wei Chen <mailto:Wei.Chen@arm.com>= ;
> > > > > > Cc: mailto:xen-devel@lists.xenpro= ject.org; mailto:sstabellini@kernel.org;
> > > > mailto:julien@xen.org;
> > > > > > Bertrand Marquis <mailto:Bertrand.Ma= rquis@arm.com>
> > > > > > Subject: Re: [PATCH 22/37] xen/arm: use NR_ME= M_BANKS to override
> > > > default
> > > > > > NR_NODE_MEMBLKS
> > > > > >
> > > > > > On Thu, 23 Sep 2021, Wei Chen wrote:
> > > > > > > As a memory range described in device tr= ee cannot be split
> > across
> > > > > > > multiple nodes. So we define NR_NODE_MEM= BLKS as NR_MEM_BANKS
> in
> > > > > > > arch header.
> > > > > >
> > > > > > This statement is true but what is the goal o= f this patch? Is it
> > to
> > > > > > reduce code size and memory consumption?
> > > > > >
> > > > >
> > > > > No, when Julien and I discussed this in last versi= on[1], we hadn't
> > > > thought
> > > > > so deeply. We just thought a memory range describe= d in DT cannot
> be
> > > > split
> > > > > across multiple nodes. So NR_MEM_BANKS should be e= qual to
> > NR_MEM_BANKS.
> > > > >
> > > > > https:= //lists.xenproject.org/archives/html/xen-devel/2021-
> > > > 08/msg00974.html
> > > > >
> > > > > > I am asking because NR_MEM_BANKS is 128 and > > > > > > NR_NODE_MEMBLKS=3D2*MAX_NUMNODES which is 64 = by default so again
> > > > > > NR_NODE_MEMBLKS is 128 before this patch.
> > > > > >
> > > > > > In other words, this patch alone doesn't = make any difference; at
> > least
> > > > > > doesn't make any difference unless CONFIG= _NR_NUMA_NODES is
> > increased.
> > > > > >
> > > > > > So, is the goal to reduce memory usage when C= ONFIG_NR_NUMA_NODES
> > is
> > > > > > higher than 64?
> > > > > >
> > > > >
> > > > > I also thought about this problem when I was writi= ng this patch.
> > > > > CONFIG_NR_NUMA_NODES is increasing, but NR_MEM_BAN= KS is a fixed
> > > > > value, then NR_MEM_BANKS can be smaller than CONFI= G_NR_NUMA_NODES
> > > > > at one point.
> > > > >
> > > > > But I agree with Julien's suggestion, NR_MEM_B= ANKS and
> > NR_NODE_MEMBLKS
> > > > > must be aware of each other. I had thought to add = some ASSERT
> check,
> > > > > but I don't know how to do it better. So I pos= t this patch for
> more
> > > > > suggestion.
> > > >
> > > > OK. In that case I'd say to get rid of the previous= definition of
> > > > NR_NODE_MEMBLKS as it is probably not necessary, see be= low.
> > > >
> > > >
> > > >
> > > > > >
> > > > > > > And keep default NR_NODE_MEMBLKS in comm= on header
> > > > > > > for those architectures NUMA is disabled= .
> > > > > >
> > > > > > This last sentence is not accurate: on x86 NU= MA is enabled and
> > > > > > NR_NODE_MEMBLKS is still defined in xen/inclu= de/xen/numa.h
> (there
> > is
> > > > no
> > > > > > x86 definition of it)
> > > > > >
> > > > >
> > > > > Yes.
> > > > >
> > > > > >
> > > > > > > Signed-off-by: Wei Chen <mailto:wei.che= n@arm.com>
> > > > > > > ---
> > > > > > >=C2=A0 xen/include/asm-arm/numa.h | 8 +++= ++++-
> > > > > > >=C2=A0 xen/include/xen/numa.h=C2=A0 =C2= =A0 =C2=A0| 2 ++
> > > > > > >=C2=A0 2 files changed, 9 insertions(+), = 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/xen/include/asm-arm/numa.h = b/xen/include/asm-
> > arm/numa.h
> > > > > > > index 8f1c67e3eb..21569e634b 100644
> > > > > > > --- a/xen/include/asm-arm/numa.h
> > > > > > > +++ b/xen/include/asm-arm/numa.h
> > > > > > > @@ -3,9 +3,15 @@
> > > > > > >
> > > > > > >=C2=A0 #include <xen/mm.h>
> > > > > > >
> > > > > > > +#include <asm/setup.h>
> > > > > > > +
> > > > > > >=C2=A0 typedef u8 nodeid_t;
> > > > > > >
> > > > > > > -#ifndef CONFIG_NUMA
> > > > > > > +#ifdef CONFIG_NUMA
> > > > > > > +
> > > > > > > +#define NR_NODE_MEMBLKS NR_MEM_BANKS > > > > > > > +
> > > > > > > +#else
> > > > > > >
> > > > > > >=C2=A0 /* Fake one node for now. See also= node_online_map. */
> > > > > > >=C2=A0 #define cpu_to_node(cpu) 0
> > > > > > > diff --git a/xen/include/xen/numa.h b/xe= n/include/xen/numa.h
> > > > > > > index 1978e2be1b..1731e1cc6b 100644
> > > > > > > --- a/xen/include/xen/numa.h
> > > > > > > +++ b/xen/include/xen/numa.h
> > > > > > > @@ -12,7 +12,9 @@
> > > > > > >=C2=A0 #define MAX_NUMNODES=C2=A0 =C2=A0 = 1
> > > > > > >=C2=A0 #endif
> > > > > > >
> > > > > > > +#ifndef NR_NODE_MEMBLKS
> > > > > > >=C2=A0 #define NR_NODE_MEMBLKS (MAX_NUMNO= DES*2)
> > > > > > > +#endif
> > > >
> > > > This one we can remove it completely right?
> > >
> > > How about define NR_MEM_BANKS to:
> > > #ifdef CONFIG_NR_NUMA_NODES
> > > #define NR_MEM_BANKS (CONFIG_NR_NUMA_NODES * 2)
> > > #else
> > > #define NR_MEM_BANKS 128
> > > #endif
> > > for both x86 and Arm. For those architectures do not support= or enable
> > > NUMA, they can still use "NR_MEM_BANKS 128". And r= eplace all
> > NR_NODE_MEMBLKS
> > > in NUMA code to NR_MEM_BANKS to remove NR_NODE_MEMBLKS compl= etely.
> > > In this case, NR_MEM_BANKS can be aware of the changes of > > CONFIG_NR_NUMA_NODES.
> >
> > x86 doesn't have NR_MEM_BANKS as far as I can tell. I guess y= ou also
> > meant to rename NR_NODE_MEMBLKS to NR_MEM_BANKS?
> >
>
> Yes.
>
> > But NR_MEM_BANKS is not directly related to CONFIG_NR_NUMA_NODES = because
> > there can be many memory banks for each numa node, certainly more= than
> > 2. The existing definition on x86:
> >
> > #define NR_NODE_MEMBLKS (MAX_NUMNODES*2)
> >
> > Doesn't make a lot of sense to me. Was it just an arbitrary l= imit for
> > the lack of a better way to set a maximum?
> >
>
> At that time, this was probably the most cost-effective approach.
> Enough and easy. But, if more nodes need to be supported in the
> future, it may bring more memory blocks. And this maximum value
> might not apply. The maximum may need to support dynamic extension. >
> >
> > On the other hand, NR_MEM_BANKS and NR_NODE_MEMBLKS seem to be re= lated.
> > In fact, what's the difference?
> >
> > NR_MEM_BANKS is the max number of memory banks (with or without > > numa-node-id).
> >
> > NR_NODE_MEMBLKS is the max number of memory banks with NUMA suppo= rt
> > (with numa-node-id)?
> >
> > They are basically the same thing. On ARM I would just do:
> >
>
> Probably not, NR_MEM_BANKS will count those memory ranges without
> numa-node-id in boot memory parsing stage (process_memory_node or
> EFI parser). But NR_NODE_MEMBLKS will only count those memory ranges > with numa-node-id.
>
> > #define NR_NODE_MEMBLKS MAX(NR_MEM_BANKS, (CONFIG_NR_NUMA_NODES *= 2))
> >
> >

> Quote Julien's comment from HTML email to here:
> " As you wrote above, the second part of the MAX is totally arbit= rary.
> In fact, it is very likely than if you have more than 64 nodes, you ma= y
> need a lot more than 2 regions per node.
>
> So, for Arm, I would just define NR_NODE_MEMBLKS as an alias to NR_MEM= _BANKS
> so it can be used by common code.
> "
>
> > But here comes the problem:
> > How can we set the NR_MEM_BANKS maximum value, 128 seems an arbit= rary too?
>
> This is based on hardware we currently support (the last time we bumpe= d the value was, IIRC, for Thunder-X). In the case of booting UEFI, we can = get a lot of small ranges as we discover the RAM using the UEFI memory map.=
>

Thanks for the background.

>
> > If #define NR_MEM_BANKS (CONFIG_NR_NUMA_NODES * N)? And what N sh= ould be.
>
> N would have to be the maximum number of ranges you can find in a NUMA= node.
>
> We would also need to make sure this doesn't break existing platfo= rms. So N would have to be quite large or we need a MAX as Stefano suggeste= d.
>
> But I would prefer to keep the existing 128 and allow to configure it = at build time (not necessarily in this series). This avoid to have differen= t way to define the value based NUMA vs non-NUMA.

In this case, can we use Stefano's
"#define NR_NODE_MEMBLKS MAX(NR_MEM_BANKS, (CONFIG_NR_NUMA_NODES * 2))= "
in next version. If yes, should we change x86 part? Because NR_MEM_BANKS has not been defined in x86.

=
What I meant by configuring dynamically is allowing= NR_MEM_BANKS to be set by the user.

The second part of the MAX makes no sense to me (at least on A= rm). So I really prefer if this is not part of the initial version.

We can refine the value, or int= roduce the MAX in the future if we have a justification for it.


> > And maybe the definition could be common with x86 if we define > > NR_MEM_BANKS to 128 on x86 too.
>
> Julien had comment here, I will continue in that email.
--000000000000f9a62205ccf7b7d3--