Fixed a bug in Util::mmap_fixed_noreplace#1197
Conversation
gc00
left a comment
gc00
left a comment
There was a problem hiding this comment.
Approving in advance. Please seem my inline comments.
src/util_misc.cpp
Outdated
| // by calling mlock page by page. If the mlock fails and the error | ||
| // is ENOMEM, we know that this page is not mapped. | ||
| // | ||
| // This only works for private memory, not shared memory. |
There was a problem hiding this comment.
This only works -> Using mlock here only works
src/util_misc.cpp
Outdated
| munlock(addr, len); | ||
| break; | ||
| } else { | ||
| fprintf(stderr, "DMTCP cannot map addr %p\n", addr); |
There was a problem hiding this comment.
Minor, but we could add:
fprintf(stderr, "%s(%d): DMTCP cannot map addr %p\n", __FILE__, __LINE__, addr);
It save the user from grep'ing for this line.
| } else if (addr2 != MAP_FAILED) { | ||
| // undo the mmap | ||
| munmap(addr2, len); | ||
| // We check if the memory range has any overlap with existing memory |
There was a problem hiding this comment.
Please reformat this to the usual 80 columns, but let's add a comment like:
// The Linux kernel is observed to _not_ use the address hint even for mapping a single page, when the previous and next pages are mapped, but the current page address is unmapped
Util::mmap_fixed_noreplace emulates the mmap() with MAP_FIXED_NOREPLACE flag for old systems. It uses a regular mmap() with the given address and check if the return value is the same as the given address. However, we found a case that the kernel returns a different address although the requirest memory range is available. Therefore, the mmap_fixed_noreplace function returns an error message EEXIST, which is not expectec. Now we check if the requested memory range is available by calling mlock() page by page. The mlock function will return -1 with an errono of ENOMEM. If there's no already mapped pages in the request memory range, the function will call mmap() with MAP_FIXED. Otherwise, the function will return MAP_FAILED with an errno of EEXIST.
xuyao0127
force-pushed
the
fix_mmap_fixd_noreplace
branch
from
7f9a5c4 to
43fdb32
Compare
| // This flag should force: 'addr == addr2' or 'addr2 == MAP_FAILED' | ||
| flags |= MAP_FIXED_NOREPLACE; | ||
| #endif | ||
| void *addr2 = mmap(addr, len, prot, flags, fd, offset); |
There was a problem hiding this comment.
Since this kernel issue happens rarely, can we try the previous route of using vanilla mmap and see if the kernel returns the correct address. If it doesn't return the correct address, we can go ahead and try the slow route of using mlocks on each page to see what's going on.
There was a problem hiding this comment.
I will add this in another PR. This function is only used when creating the sharedDatatHeader for now.
3 participants
Util::mmap_fixed_noreplace emulates the mmap() with MAP_FIXED_NOREPLACE
flag for old systems. It uses a regular mmap() with the given address
and check if the return value is the same as the given address. However,
we found a case that the kernel returns a different address although the
requirest memory range is available. Therefore, the mmap_fixed_noreplace
function returns an error message EEXIST, which is not expectec.
Now we check if the requested memory range is available by calling mlock()
page by page. The mlock function will return -1 with an errono of ENOMEM.
If there's no already mapped pages in the request memory range, the function
will call mmap() with MAP_FIXED. Otherwise, the function will return
MAP_FAILED with an errno of EEXIST.