mmap fails with out of memory if map big read-only file with MAP_PRIVATE flag

[X512]

This is hrev57493.

map_file syscall used in mmap implementation fails to map read-only file on disk if mapped size is larger than total amount of RAM. It cause Git to fail to fetch big repositories:

> git fetch origin-new
remote: Enumerating objects: 433106, done.
remote: Counting objects: 100% (175641/175641), done.
remote: Total 433106 (delta 175641), reused 175641 (delta 175641), pack-reused 257465
Receiving objects: 100% (433106/433106), 992.65 MiB | 3.78 MiB/s, done.
fatal: packfile .git/objects/pack/pack-ab36c9a68f870060114341fac8c873d549467667.pack cannot be mapped: Out of memory
fatal: fetch-pack: invalid index-pack output

Changing MAP_PRIVATE flag to MAP_SHARED fixes problem and Git fetch succeeds.

As I understand there are no difference between MAP_PRIVATE and MAP_SHARED if file is mapped without write access.

Quick patch that fixes issue:

src/system/libroot/posix/sys/mman.cpp

@@ -125 +125 @@
        }
 
        // translate mapping, address specification, and protection
-       int mapping = (flags & MAP_SHARED) != 0
+       int mapping = (flags & MAP_SHARED) != 0 || (protection & PROT_WRITE) == 0
                ? REGION_NO_PRIVATE_MAP : REGION_PRIVATE_MAP;
 
        uint32 addressSpec;

[waddlesplash]

I think this optimization needs to be done in the kernel, because I think it's checked whether the FD was opened read-write or read-only? If the FD was opened read-only, then yes, we can use MAP_SHARED, I think. But I'd have to check.

[waddlesplash]

As I understand there are no difference between MAP_PRIVATE and MAP_SHARED if file is mapped without write access.

This is true, however, files mapped without MAP_SHARED (i.e. with REGION_PRIVATE_MAP) can have PROT_WRITE added later on (via mprotect), after mmap was called, and all will work as expected. If we were to optimize this inside mmap or in the kernel, then that would prevent applications from doing that.

So, I don't think we can make this change either in libroot or in the kernel directly.

Instead, what we could do is to avoid committing memory in this case, and then only commit it when mprotect() was called (and thus fail in mprotect if memory can't be reserved.)

It looks like set_area_protection already alters commitment on changing from non-writable -> writable, but mprotect doesn't.

[X512]

files mapped without MAP_SHARED (i.e. with REGION_PRIVATE_MAP) can have PROT_WRITE added later on (via mprotect)

I suspect mprotect should fail with out of memory in this case.

So, I don't think we can make this change either in libroot

Yes, I am aware that my quick patch is not a proper solution. I need this patch to be able to pull large Git repositories such as Webkit.

[waddlesplash]

Test case attached that reproduces this problem (requires creating a file larger than system RAM for testing.)

Proposed fix: ​https://review.haiku-os.org/c/haiku/+/7339

[waddlesplash]

Fixed in hrev57517.