Re: [PATCH 4/5] elf2dmp: use Linux mmap with MAP_NORESERVE when possible

[Viktor Prutyanov]

> 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().