From xen-devel-bounces@lists.xen.org Mon Nov 24 20:46:15 2014 Received: (at maildrop) by bugs.xenproject.org; 24 Nov 2014 20:46:15 +0000 Received: from lists.xen.org ([50.57.142.19]) by bugs.xenproject.org with esmtp (Exim 4.80) (envelope-from ) id 1Xt0WV-0003ww-Q0 for xen-devel-maildrop-Eithu9ie@bugs.xenproject.org; Mon, 24 Nov 2014 20:46:15 +0000 Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Xt0Mg-00012P-3t; Mon, 24 Nov 2014 20:36:06 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Xt0Me-00012K-QF for xen-devel@lists.xensource.com; Mon, 24 Nov 2014 20:36:05 +0000 Received: from [193.109.254.147] by server-9.bemta-14.messagelabs.com id 7A/FC-02712-4B693745; Mon, 24 Nov 2014 20:36:04 +0000 X-Env-Sender: dslutz@verizon.com X-Msg-Ref: server-6.tower-27.messagelabs.com!1416861361!14531599!1 X-Originating-IP: [199.249.25.207] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTk5LjI0OS4yNS4yMDcgPT4gMjk3MjAw\n X-StarScan-Received: X-StarScan-Version: 6.12.4; banners=-,-,- X-VirusChecked: Checked Received: (qmail 2911 invoked from network); 24 Nov 2014 20:36:03 -0000 Received: from omzsmtpe04.verizonbusiness.com (HELO omzsmtpe04.verizonbusiness.com) (199.249.25.207) by server-6.tower-27.messagelabs.com with DHE-RSA-AES256-SHA encrypted SMTP; 24 Nov 2014 20:36:03 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=verizon.com; i=dslutz@verizon.com; q=dns/txt; s=corp; t=1416861363; x=1448397363; h=from:message-id:date:mime-version:to:cc:subject: references:in-reply-to:content-transfer-encoding; bh=8O7lYN5Ams/6iKlsEeleK7CNdU8olm/f0k9QTbh8lN8=; b=iDNyYoDOnKjK29ZGECiyZenpdqqfDajY5XTwO0irYv5F3spzZgjCwtaE tGYSTz3mtmW29g7T1nDpcaDu3IA4DdeNiZOU5SIXwCKmEZnDPWDQ62xQ0 MIGcVnPkk8Bw19cTwITLli1qWPkNEQRVJbUZ1zkdO7Z5WMZrruLLQQ+Hi A=; X-IronPort-Anti-Spam-Filtered: false Received: from unknown (HELO fldsmtpi01.verizon.com) ([166.68.71.143]) by omzsmtpe04.verizonbusiness.com with ESMTP; 24 Nov 2014 20:36:00 +0000 From: Don Slutz X-VzAPP: 1 X-IronPort-AV: E=Sophos;i="5.07,451,1413244800"; d="scan'208";a="913381535" Received: from unknown (HELO don-760.CloudSwitch.com) ([70.105.109.80]) by fldsmtpi01.verizon.com with ESMTP; 24 Nov 2014 20:35:59 +0000 Message-ID: <547396AE.7050806@terremark.com> Date: Mon, 24 Nov 2014 15:35:58 -0500 User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Stefano Stabellini , Don Slutz References: <1416474814.29243.59.camel@citrix.com> <1416483731.14429.8.camel@citrix.com> <1416494946.14429.13.camel@citrix.com> <20141121173437.GA14331@laptop.dumpdata.com> <20141121190757.GA16038@laptop.dumpdata.com> <20141124150649.GB3946@laptop.dumpdata.com> <5473582C.6080000@terremark.com> In-Reply-To: Cc: xen-devel@lists.xensource.com, Wei Liu , Ian Campbell , Ian Jackson , hanyandong Subject: Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org On 11/24/14 12:07, Stefano Stabellini wrote: > On Mon, 24 Nov 2014, Don Slutz wrote: >> On 11/24/14 10:26, Stefano Stabellini wrote: >>> On Mon, 24 Nov 2014, Konrad Rzeszutek Wilk wrote: >>>> On Mon, Nov 24, 2014 at 12:17:12PM +0000, Stefano Stabellini wrote: >>>>> On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: >>>>>> On Fri, Nov 21, 2014 at 06:48:53PM +0000, Stefano Stabellini wrote: >>>>>>> On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: >>>>>>>> On Fri, Nov 21, 2014 at 05:11:09PM +0000, Stefano Stabellini >>>>>>>> wrote: >>>>>>>>> The rom is used for pxebooting. We don't need to allow >>>>>>>>> pxebooting from >>>>>>>>> more than one network card. Loading a romfile for every NIC >>>>>>>>> wastes >>>>>>>> Why not? Why can't we PXE boot from each network card? >>>>>>> I take it back: you are right, at the moment it is actually possible >>>>>>> to >>>>>>> pxe boot from the fourth nic for example. Maybe we could just load >>>>>>> the >>>>>>> first four romfiles and skip the others (four is the limit before >>>>>>> QEMU >>>>>>> fails to allocate any more memory). >>>>>> The limit is in the count of ROMs or the memory consumption? >>>>> The limit is memory consumption. >>>> Um, how big are those ROMs? Gigs? >>> I think they are 60K each in the case of rtl8139. >>> However they are accounted on top of the ram of the guest, that's why >>> the allocation fails. Strictly speaking I guess it shouldn't work even >>> the first time around. >> My extra QEMU debug says: >> >> xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42010000 >> mr.name=rtl8139.rom >> e1000 vhdr enabled >> xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42050000 >> mr.name=e1000.rom >> >> And that matches the max of 4. >> >> #define LIBXL_MAXMEM_CONSTANT 1024 >> >> is what controls this. >> >>> Maybe the bug is in libxl memory allocation, that doesn't account for >>> rom sizes. >> Yes. > Here is a better patch. > > --- > > libxl: account for romfile memory > > Account for memory needed for emulated network card rom files. > Assume 256K for each romfile. > > Signed-off-by: Stefano Stabellini This looks to be a good idea, just not fully done. > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index de23fec..2edb194 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -4527,6 +4527,33 @@ out: > > /******************************************************************************/ > > +int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config) > +{ > + int i, count_rom, rc; > + libxl_domain_config local_d_config; > + libxl_ctx *ctx = libxl__gc_owner(gc); > + > + if (d_config == NULL) { > + libxl_domain_config_init(&local_d_config); > + rc = libxl_retrieve_domain_configuration(ctx, domid, &local_d_config); > + if (rc < 0) > + return rc; > + d_config = &local_d_config; > + } > + > + if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) > + return 0; > + > + for (i = 0, count_rom = 0; i < d_config->num_nics; i++) { > + if (d_config->nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) > + count_rom++; > + } > + > + return count_rom*LIBXL_ROMSIZE_KB; > + > + return 0; 2 return statements? > +} > + > int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb) > { > GC_INIT(ctx); > @@ -4550,11 +4577,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb) > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n"); > goto out; > } > - rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT); > + rc = xc_domain_setmaxmem(ctx->xch, domid, > + max_memkb + LIBXL_MAXMEM_CONSTANT > + + libxl__get_rom_memory_kb(gc, domid, NULL)); Here (and the rest) need to check for error's. Also I think that LIBXL_MAXMEM_CONSTANT can be dropped here. I found out that #define LIBXL_HVM_EXTRA_MEMORY 2048 Should be #define LIBXL_HVM_EXTRA_MEMORY (LIBXL_MAXMEM_CONSTANT + 1024) if you change the size of the ROM memory for QEMU. I have only done the static change. -Don Slutz > if (rc != 0) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > "xc_domain_setmaxmem domid=%d memkb=%d failed " > - "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc); > + "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT + > + libxl__get_rom_memory_kb(gc, domid, NULL), rc); > goto out; > } > > @@ -4770,11 +4800,12 @@ retry_transaction: > if (enforce) { > memorykb = new_target_memkb; > rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + > - LIBXL_MAXMEM_CONSTANT); > + LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, NULL)); > if (rc != 0) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > "xc_domain_setmaxmem domid=%d memkb=%d failed " > - "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); > + "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT + > + libxl__get_rom_memory_kb(gc, domid, NULL), rc); > abort_transaction = 1; > goto out; > } > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 74ea84b..ca254f9 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -406,7 +406,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > } > > if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + > - LIBXL_MAXMEM_CONSTANT) < 0) { > + LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, d_config)) < 0) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory"); > return ERROR_FAIL; > } > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 4361421..33826ea 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -90,6 +90,7 @@ > #define LIBXL_XENCONSOLE_LIMIT 1048576 > #define LIBXL_XENCONSOLE_PROTOCOL "vt100" > #define LIBXL_MAXMEM_CONSTANT 1024 > +#define LIBXL_ROMSIZE_KB 256 > #define LIBXL_PV_EXTRA_MEMORY 1024 > #define LIBXL_HVM_EXTRA_MEMORY 2048 > #define LIBXL_MIN_DOM0_MEM (128*1024) > @@ -1023,6 +1024,12 @@ _hidden char * libxl__domain_pvcontrol_read(libxl__gc *gc, > _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t, > uint32_t domid, const char *cmd); > > +/* Returns the amount of extra mem required to allocate roms or an libxl > + * error code on error. > + * The *d_config parameter is optional. > + */ > +_hidden int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config); > + > /* from xl_device */ > _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend); > _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel