-
Notifications
You must be signed in to change notification settings - Fork 26
lower_half/mmap64.c: mmaps[]: add sentinel #351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
82eef31 to
8e53f1d
Compare
mpi-proxy-split/lower-half/mmap64.c
Outdated
| for (int i = 0; i < numRegions; i++) { | ||
| for (int i = 0; i < numMmapRegions; i++) { | ||
| memset(&mmaps[i], 0, sizeof(mmaps[i])); | ||
| mmaps[i].addr = MAP_FAILED; // Mark those mmaps past numMmapRegions, for GDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memset should take care of setting all the variables to 0x0. We do not need to set MAP_FAILED separately. Also, the comment does not reflect the code. The loop is only till numMmapRegions, not past that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add MAP_FAILED after the for loop? The value of i would then be numMmapRegions.
8e53f1d to
eb55bed
Compare
mpi-proxy-split/lower-half/mmap64.c
Outdated
| for (int i = 0; i < numRegions; i++) { | ||
| for (int i = 0; i < numMmapRegions; i++) { | ||
| memset(&mmaps[i], 0, sizeof(mmaps[i])); | ||
| mmaps[i].addr = MAP_FAILED; // Mark those mmaps past numMmapRegions, for GDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add MAP_FAILED after the for loop? The value of i would then be numMmapRegions.
eb55bed to
69eedbb
Compare
69eedbb to
2a5cfbf
Compare
|
@JainTwinkle, Thanks for the comment about:
Good catch! I've now changed it to set the 'addr' to MAP_FAILED for all mmaps elements. Since 'numMmapRegions' is 0 at the end, I could set it just for the zero-th element. But it's safer to set it for all 'mmaps' elements. |
This adds a sentinel (MAP_FAILED) to the mmaps[] array, and then checks for it in the mpi_plugin.cpp:DMTCP_EVENT_PRECHECKPOINT.
It also renames numRegions to numMmapRegions in lower_half, and improves the documentation in mmap64.c where we reserve a large mmap'ed region for mmap64.c,, starting at 0x2aab00000000;.