| Message ID | 20230913224657.11606-5-viktor@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | elf2dmp: improve Win2022, Win11 and large dumps | expand |
On 2023/09/14 7:46, Viktor Prutyanov wrote: > Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and > MAP_PRIVATE. This leads to premature physical memory allocation of dump > file size on Linux hosts and may fail. On Linux, mapping the file with > MAP_NORESERVE limits the allocation by available memory. > > Signed-off-by: Viktor Prutyanov <viktor@daynix.com> > --- > contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++------- > contrib/elf2dmp/qemu_elf.h | 4 +++ > 2 files changed, 58 insertions(+), 12 deletions(-) > > diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c > index ebda60dcb8..94a8c3ad15 100644 > --- a/contrib/elf2dmp/qemu_elf.c > +++ b/contrib/elf2dmp/qemu_elf.c > @@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe) > return true; > } > > -int QEMU_Elf_init(QEMU_Elf *qe, const char *filename) > +static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename) > { > +#ifdef CONFIG_LINUX Here CONFIG_LINUX is used while qemu_elf.h uses CONFIG_POSIX. I also wonder if GLib implementation is really necessary. > + struct stat st; > + > + printf("Using Linux's mmap\n"); > + > + qe->fd = open(filename, O_RDONLY, 0); > + if (qe->fd == -1) { > + eprintf("Failed to open ELF dump file \'%s\'\n", filename); > + return 1; > + } > + > + if (fstat(qe->fd, &st)) { > + eprintf("Failed to get size of ELF dump file\n"); > + close(qe->fd); > + return 1; > + } > + qe->size = st.st_size; > + > + qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0); It should be possible to close the file immediately after mmap().
> On 2023/09/14 7:46, Viktor Prutyanov wrote: > >> Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and >> MAP_PRIVATE. This leads to premature physical memory allocation of dump >> file size on Linux hosts and may fail. On Linux, mapping the file with >> MAP_NORESERVE limits the allocation by available memory. >> >> Signed-off-by: Viktor Prutyanov <viktor@daynix.com> >> --- >> contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++------- >> contrib/elf2dmp/qemu_elf.h | 4 +++ >> 2 files changed, 58 insertions(+), 12 deletions(-) >> >> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c >> index ebda60dcb8..94a8c3ad15 100644 >> --- a/contrib/elf2dmp/qemu_elf.c >> +++ b/contrib/elf2dmp/qemu_elf.c >> @@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe) >> return true; >> } >> >> -int QEMU_Elf_init(QEMU_Elf *qe, const char *filename) >> +static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename) >> { >> +#ifdef CONFIG_LINUX > > Here CONFIG_LINUX is used while qemu_elf.h uses CONFIG_POSIX. Thank you, the right one is CONFIG_LINUX. > I also wonder if GLib implementation is really necessary. GLib implementation is for non-Linux OS. Some of them have mmap, but don't have MAP_NORESERVE. Some other, such as Windows, don't have POSIX calls at all. > >> + struct stat st; >> + >> + printf("Using Linux's mmap\n"); >> + >> + qe->fd = open(filename, O_RDONLY, 0); >> + if (qe->fd == -1) { >> + eprintf("Failed to open ELF dump file \'%s\'\n", filename); >> + return 1; >> + } >> + >> + if (fstat(qe->fd, &st)) { >> + eprintf("Failed to get size of ELF dump file\n"); >> + close(qe->fd); >> + return 1; >> + } >> + qe->size = st.st_size; >> + >> + qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE, >> + MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0); > > It should be possible to close the file immediately after mmap().
diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c index ebda60dcb8..94a8c3ad15 100644 --- a/contrib/elf2dmp/qemu_elf.c +++ b/contrib/elf2dmp/qemu_elf.c @@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe) return true; } -int QEMU_Elf_init(QEMU_Elf *qe, const char *filename) +static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename) { +#ifdef CONFIG_LINUX + struct stat st; + + printf("Using Linux's mmap\n"); + + qe->fd = open(filename, O_RDONLY, 0); + if (qe->fd == -1) { + eprintf("Failed to open ELF dump file \'%s\'\n", filename); + return 1; + } + + if (fstat(qe->fd, &st)) { + eprintf("Failed to get size of ELF dump file\n"); + close(qe->fd); + return 1; + } + qe->size = st.st_size; + + qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0); + if (qe->map == MAP_FAILED) { + eprintf("Failed to map ELF file\n"); + close(qe->fd); + return 1; + } +#else GError *gerr = NULL; - int err = 0; + + printf("Using GLib's mmap\n"); qe->gmf = g_mapped_file_new(filename, TRUE, &gerr); if (gerr) { @@ -179,29 +206,44 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename) qe->map = g_mapped_file_get_contents(qe->gmf); qe->size = g_mapped_file_get_length(qe->gmf); +#endif + + return 0; +} + +static void QEMU_Elf_unmap(QEMU_Elf *qe) +{ +#ifdef CONFIG_LINUX + munmap(qe->map, qe->size); + close(qe->fd); +#else + g_mapped_file_unref(qe->gmf); +#endif +} + +int QEMU_Elf_init(QEMU_Elf *qe, const char *filename) +{ + if (QEMU_Elf_map(qe, filename)) { + return 1; + } if (!check_ehdr(qe)) { eprintf("Input file has the wrong format\n"); - err = 1; - goto out_unmap; + QEMU_Elf_unmap(qe); + return 1; } if (init_states(qe)) { eprintf("Failed to extract QEMU CPU states\n"); - err = 1; - goto out_unmap; + QEMU_Elf_unmap(qe); + return 1; } return 0; - -out_unmap: - g_mapped_file_unref(qe->gmf); - - return err; } void QEMU_Elf_exit(QEMU_Elf *qe) { exit_states(qe); - g_mapped_file_unref(qe->gmf); + QEMU_Elf_unmap(qe); } diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h index b2f0d9cbc9..2a71beca8e 100644 --- a/contrib/elf2dmp/qemu_elf.h +++ b/contrib/elf2dmp/qemu_elf.h @@ -32,7 +32,11 @@ typedef struct QEMUCPUState { int is_system(QEMUCPUState *s); typedef struct QEMU_Elf { +#ifdef CONFIG_POSIX + int fd; +#else GMappedFile *gmf; +#endif size_t size; void *map; QEMUCPUState **state;
Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and MAP_PRIVATE. This leads to premature physical memory allocation of dump file size on Linux hosts and may fail. On Linux, mapping the file with MAP_NORESERVE limits the allocation by available memory. Signed-off-by: Viktor Prutyanov <viktor@daynix.com> --- contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++------- contrib/elf2dmp/qemu_elf.h | 4 +++ 2 files changed, 58 insertions(+), 12 deletions(-)