From patchwork Tue Feb 19 14:41:11 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabien Chouteau X-Patchwork-Id: 221718 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 421CE2C0099 for ; Wed, 20 Feb 2013 03:06:02 +1100 (EST) Received: from localhost ([::1]:50167 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U7phg-0003aG-3E for incoming@patchwork.ozlabs.org; Tue, 19 Feb 2013 11:06:00 -0500 Received: from eggs.gnu.org ([208.118.235.92]:36395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U7phD-00031f-TZ for qemu-devel@nongnu.org; Tue, 19 Feb 2013 11:05:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1U7ph2-00085x-VQ for qemu-devel@nongnu.org; Tue, 19 Feb 2013 11:05:31 -0500 Received: from mel.act-europe.fr ([194.98.77.210]:47233) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U7oNl-00079D-Du for qemu-devel@nongnu.org; Tue, 19 Feb 2013 09:41:21 -0500 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 727C4290042; Tue, 19 Feb 2013 15:41:19 +0100 (CET) X-Virus-Scanned: amavisd-new at eu.adacore.com Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LS8nkLYvQkKG; Tue, 19 Feb 2013 15:41:19 +0100 (CET) Received: from PomPomGalli.act-europe.fr (pompomgalli.act-europe.fr [10.10.1.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id EBE04290006; Tue, 19 Feb 2013 15:41:18 +0100 (CET) From: Fabien Chouteau To: Date: Tue, 19 Feb 2013 15:41:11 +0100 Message-Id: <1361284872-26126-1-git-send-email-chouteau@adacore.com> X-Mailer: git-send-email 1.7.9.5 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 194.98.77.210 Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org, Alexander Graf , Avi Kivity , Henning Schild , Paolo Bonzini , Luiz Capitulino , Wenchao Xia , Aurelien Jarno Subject: [Qemu-devel] [PATCH] Save memory allocation in the elf loader X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The current elf loader uses too much memory. For example, I have a executable with a bss section of 400 MB and I set the ram size to 512 MB. Qemu uses about 780MB of RAM (which is fine), but there's a peak at 1.6 GB during initialization (this is not fine). This patch fixes two things: 1) do not allocate each elf program twice. 2) do not allocate memory for areas that are only zeros. For this we need a new field in Rom: "datasize" which is the size of the allocated data. If datasize is less than romsize, it means that the area from datasize to romsize is filled with zeros. Signed-off-by: Fabien Chouteau --- hw/elf_ops.h | 19 ++++++++------- hw/loader.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++---------- hw/loader.h | 2 ++ 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/hw/elf_ops.h b/hw/elf_ops.h index 531a425..acc701e 100644 --- a/hw/elf_ops.h +++ b/hw/elf_ops.h @@ -197,7 +197,7 @@ static int glue(load_elf, SZ)(const char *name, int fd, struct elfhdr ehdr; struct elf_phdr *phdr = NULL, *ph; int size, i, total_size; - elf_word mem_size; + elf_word mem_size, file_size; uint64_t addr, low = (uint64_t)-1, high = 0; uint8_t *data = NULL; char label[128]; @@ -252,14 +252,16 @@ static int glue(load_elf, SZ)(const char *name, int fd, for(i = 0; i < ehdr.e_phnum; i++) { ph = &phdr[i]; if (ph->p_type == PT_LOAD) { - mem_size = ph->p_memsz; - /* XXX: avoid allocating */ - data = g_malloc0(mem_size); + mem_size = ph->p_memsz; /* Size of the ROM */ + file_size = ph->p_filesz; /* Size of the allocated data */ + data = g_malloc0(file_size); if (ph->p_filesz > 0) { - if (lseek(fd, ph->p_offset, SEEK_SET) < 0) + if (lseek(fd, ph->p_offset, SEEK_SET) < 0) { goto fail; - if (read(fd, data, ph->p_filesz) != ph->p_filesz) + } + if (read(fd, data, file_size) != file_size) { goto fail; + } } /* address_offset is hack for kernel images that are linked at the wrong physical address. */ @@ -281,7 +283,9 @@ static int glue(load_elf, SZ)(const char *name, int fd, } snprintf(label, sizeof(label), "phdr #%d: %s", i, name); - rom_add_blob_fixed(label, data, mem_size, addr); + + /* rom_add_elf_program() seize the ownership of 'data' */ + rom_add_elf_program(label, data, file_size, mem_size, addr); total_size += mem_size; if (addr < low) @@ -289,7 +293,6 @@ static int glue(load_elf, SZ)(const char *name, int fd, if ((addr + mem_size) > high) high = addr + mem_size; - g_free(data); data = NULL; } } diff --git a/hw/loader.c b/hw/loader.c index 995edc3..bd2b52d 100644 --- a/hw/loader.c +++ b/hw/loader.c @@ -533,7 +533,14 @@ typedef struct Rom Rom; struct Rom { char *name; char *path; + + /* datasize is the amount of memory allocated in "data". If datasize is less + * than romsize, it means that the area from datasize to romsize is filled + * with zeros. + */ size_t romsize; + size_t datasize; + uint8_t *data; int isrom; char *fw_dir; @@ -589,14 +596,15 @@ int rom_add_file(const char *file, const char *fw_dir, rom->fw_dir = g_strdup(fw_dir); rom->fw_file = g_strdup(file); } - rom->addr = addr; - rom->romsize = lseek(fd, 0, SEEK_END); - rom->data = g_malloc0(rom->romsize); + rom->addr = addr; + rom->romsize = lseek(fd, 0, SEEK_END); + rom->datasize = rom->romsize; + rom->data = g_malloc0(rom->datasize); lseek(fd, 0, SEEK_SET); - rc = read(fd, rom->data, rom->romsize); - if (rc != rom->romsize) { + rc = read(fd, rom->data, rom->datasize); + if (rc != rom->datasize) { fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected %zd)\n", - rom->name, rc, rom->romsize); + rom->name, rc, rom->datasize); goto err; } close(fd); @@ -637,16 +645,37 @@ int rom_add_blob(const char *name, const void *blob, size_t len, { Rom *rom; - rom = g_malloc0(sizeof(*rom)); - rom->name = g_strdup(name); - rom->addr = addr; - rom->romsize = len; - rom->data = g_malloc0(rom->romsize); + rom = g_malloc0(sizeof(*rom)); + rom->name = g_strdup(name); + rom->addr = addr; + rom->romsize = len; + rom->datasize = len; + rom->data = g_malloc0(rom->datasize); memcpy(rom->data, blob, len); rom_insert(rom); return 0; } +/* This function is specific for elf program because we don't need to allocate + * all the rom. We just allocate the first part and the rest is just zeros. This + * is why romsize and datasize are different. Also, this function seize the + * memory ownership of "data", so we don't have to allocate and copy the buffer. + */ +int rom_add_elf_program(const char *name, void *data, size_t datasize, + size_t romsize, hwaddr addr) +{ + Rom *rom; + + rom = g_malloc0(sizeof(*rom)); + rom->name = g_strdup(name); + rom->addr = addr; + rom->datasize = datasize; + rom->romsize = romsize; + rom->data = data; + rom_insert(rom); + return 0; +} + int rom_add_vga(const char *file) { return rom_add_file(file, "vgaroms", 0, -1); @@ -668,7 +697,7 @@ static void rom_reset(void *unused) if (rom->data == NULL) { continue; } - cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize); + cpu_physical_memory_write_rom(rom->addr, rom->data, rom->datasize); if (rom->isrom) { /* rom needs to be written only once */ g_free(rom->data); @@ -756,13 +785,33 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size) d = dest + (rom->addr - addr); s = rom->data; - l = rom->romsize; + l = rom->datasize; if ((d + l) > (dest + size)) { l = dest - d; } memcpy(d, s, l); + + if (rom->romsize > rom->datasize) { + /* If datasize is less than romsize, it means that we didn't + * allocate all the ROM because the trailing data are only zeros. + */ + + d += l; + l = rom->romsize - rom->datasize; + + if ((d + l) > (dest + size)) { + /* Rom size doesn't fit in the destination area. Adjust to avoid + * overflow. + */ + l = dest - d; + } + + if (l > 0) { + memset(d, 0x0, l); + } + } } return (d + l) - dest; diff --git a/hw/loader.h b/hw/loader.h index 5e61c95..0958f06 100644 --- a/hw/loader.h +++ b/hw/loader.h @@ -27,6 +27,8 @@ int rom_add_file(const char *file, const char *fw_dir, hwaddr addr, int32_t bootindex); int rom_add_blob(const char *name, const void *blob, size_t len, hwaddr addr); +int rom_add_elf_program(const char *name, void *data, size_t datasize, + size_t romsize, hwaddr addr); int rom_load_all(void); void rom_set_fw(void *f); int rom_copy(uint8_t *dest, hwaddr addr, size_t size);