From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v6 11/23] libxl: functions to build
 vmemranges for PV guest
Date: Mon, 2 Mar 2015 15:41:21 +0000
Message-ID: <1425310881.21151.92.camel@citrix.com>
References: <1424966166-2388-1-git-send-email-wei.liu2@citrix.com>
	<1424966166-2388-12-git-send-email-wei.liu2@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1424966166-2388-12-git-send-email-wei.liu2@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Elena Ufimtseva <ufimtseva@gmail.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Thu, 2015-02-26 at 15:55 +0000, Wei Liu wrote:
> Introduce a arch-independent routine to generate one vmemrange per
> vnode. Also introduce arch-dependent routines for different
> architectures because part of the process is arch-specific -- ARM has
> yet have NUMA support and E820 is x86 only.
> 
> For those x86 guests who care about machine E820 map (i.e. with
> e820_host=1), vnode is further split into several vmemranges to
> accommodate memory holes.  A few stubs for libxl_arm.c are created.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although one comment:

> +    e820_count = 0;
> +    nr_vmemrange = 0;
> +    vmemranges = NULL;
> +    for (nid = 0; nid < b_info->num_vnuma_nodes; nid++) {
> +        libxl_vnode_info *p = &b_info->vnuma_nodes[nid];
> +        uint64_t remaining_bytes = (p->memkb << 10), bytes;
> +
> +        while (remaining_bytes > 0) {
> +            if (e820_count >= nr_e820) {
> +                rc = ERROR_NOMEM;
> +                goto out;
> +            }
> +
> +            /* Skip non RAM region */
> +            if (map[e820_count].type != E820_RAM) {
> +                e820_count++;
> +                continue;
> +            }
> +
> +            GCREALLOC_ARRAY(vmemranges, nr_vmemrange+1);

Once we've hit the limit this is going to reallocate every time (and the
limit starts as zero, so that's every time). Which means an aweful lot
of reallocing every time.

Perhaps start with some non-zero size and increase by a non-linear
amount as needed? That would require tracking the array size separate
from nr_vmemrange of course.

Ian.