| Message ID | 1467104499-27517-10-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 28/06/2016 11:01, Peter Lieven wrote: > a classic use for mmap here. > > Signed-off-by: Peter Lieven <pl@kamp.de> They are never freed, why does mmap help? Paolo > --- > hw/core/loader.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 53e0e41..f217edc 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -55,6 +55,7 @@ > #include "exec/address-spaces.h" > #include "hw/boards.h" > #include "qemu/cutils.h" > +#include "qemu/mmap-alloc.h" > > #include <zlib.h> > > @@ -837,7 +838,7 @@ int rom_add_file(const char *file, const char *fw_dir, > { > MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > Rom *rom; > - int rc, fd = -1; > + int fd = -1; > char devpath[100]; > > rom = g_malloc0(sizeof(*rom)); > @@ -867,12 +868,9 @@ int rom_add_file(const char *file, const char *fw_dir, > } > > rom->datasize = rom->romsize; > - rom->data = g_malloc0(rom->datasize); > - lseek(fd, 0, SEEK_SET); > - 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->datasize); > + rom->data = mmap(NULL, rom->datasize, PROT_READ, MAP_SHARED, fd, 0); > + if (rom->data == MAP_FAILED) { > + fprintf(stderr, "rom: file %-20s: mmap error\n", rom->name); > goto err; > } > close(fd); > @@ -915,7 +913,7 @@ err: > if (fd != -1) > close(fd); > > - g_free(rom->data); > + qemu_anon_ram_munmap(rom->data, rom->romsize); > g_free(rom->path); > g_free(rom->name); > if (fw_dir) { > @@ -1013,7 +1011,7 @@ static void rom_reset(void *unused) > } > if (rom->isrom) { > /* rom needs to be written only once */ > - g_free(rom->data); > + qemu_anon_ram_munmap(rom->data, rom->datasize); > rom->data = NULL; > } > /* >
Am 28.06.2016 um 12:41 schrieb Paolo Bonzini: > > On 28/06/2016 11:01, Peter Lieven wrote: >> a classic use for mmap here. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> > They are never freed, why does mmap help? Actually it is. Adding some debug output to rom_add_file and rom_reset reveals the following: rom load: /home/lieven/git/qemu/pc-bios/bios-256k.bin rom load: /home/lieven/git/qemu/pc-bios/kvmvapic.bin rom_reset I think the rom is copied to system memory? Peter
Am 28.06.2016 um 12:41 schrieb Paolo Bonzini: > > On 28/06/2016 11:01, Peter Lieven wrote: >> a classic use for mmap here. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> > They are never freed, why does mmap help? checked again. qemu_system_reset (which calls rom_reset) is invoked after pc_macine_init. So the memory is indeed freed again. Peter
diff --git a/hw/core/loader.c b/hw/core/loader.c index 53e0e41..f217edc 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -55,6 +55,7 @@ #include "exec/address-spaces.h" #include "hw/boards.h" #include "qemu/cutils.h" +#include "qemu/mmap-alloc.h" #include <zlib.h> @@ -837,7 +838,7 @@ int rom_add_file(const char *file, const char *fw_dir, { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); Rom *rom; - int rc, fd = -1; + int fd = -1; char devpath[100]; rom = g_malloc0(sizeof(*rom)); @@ -867,12 +868,9 @@ int rom_add_file(const char *file, const char *fw_dir, } rom->datasize = rom->romsize; - rom->data = g_malloc0(rom->datasize); - lseek(fd, 0, SEEK_SET); - 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->datasize); + rom->data = mmap(NULL, rom->datasize, PROT_READ, MAP_SHARED, fd, 0); + if (rom->data == MAP_FAILED) { + fprintf(stderr, "rom: file %-20s: mmap error\n", rom->name); goto err; } close(fd); @@ -915,7 +913,7 @@ err: if (fd != -1) close(fd); - g_free(rom->data); + qemu_anon_ram_munmap(rom->data, rom->romsize); g_free(rom->path); g_free(rom->name); if (fw_dir) { @@ -1013,7 +1011,7 @@ static void rom_reset(void *unused) } if (rom->isrom) { /* rom needs to be written only once */ - g_free(rom->data); + qemu_anon_ram_munmap(rom->data, rom->datasize); rom->data = NULL; } /*
a classic use for mmap here. Signed-off-by: Peter Lieven <pl@kamp.de> --- hw/core/loader.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)