Skip to content
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

mmap shared memory transport #1086

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

davidozog
Copy link
Member

This new on-node transport mmap's the symmetric data and heap segments of all other PE's, much like XPMEM does. The intent is to provide an alternative to XPMEM for shared memory transport when XPMEM is not available. The RMA performance looks very similar to XPMEM.

To create a file-backed mmap of the data segment, I copy the whole segment to the file during initialization. This may be risky if another thread/entity can somehow update the data segment before the mmap succeeds. But I haven't seen any problems in practice. (MacOS does not support this because writing to the file fails for some reason.)

Thanks to @halitdogan and Pardo for the help on this!

@davidozog davidozog changed the title Wip/mmap xpmem heap mmap shared memory transport Mar 11, 2023

/* Write all current contents of the data segment to the file */
FILE *fp = fdopen(fd, "wb");
size_t ret = fwrite(base, shm_size, 1, fp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share a reference to docs for this? As far as I've been able to track down, fdopen on a descriptor returned by shm_open is described as having undefined behavior.

Copy link
Member Author

@davidozog davidozog Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find anything referencing this either, probably because it's a complete hack ;).

BTW, there's a good chance we won't merge this PR, at least as it is, for this very reason. Maybe it should just be a topic branch until the symmetric data segment is deprecated or something. The reason this WIP PR exists is because I'd like to evaluate some internal systems where there's no chance XPMEM will ever be installed, so we have to think out of the box.

Yes, manpages say fdopen on a shared memory object is undefined, but I think that's because there are many operations one may not be able to do on shared memory "files" (like seek, lock via fcntl, chmod, etc.) and possibly because there could be other processes writing to the shared region while the file is being opened. So it's usually a very bad idea to do this.

But I'm wondering if it may be relatively harmless to do this within SOS init on Linux, because we're not doing any fancy ops on the file (just writing to it and then truncating it) and no other PE or thread will write to the region until after shmem_init (but I think there's a risk of say some crazy interrupt or something leading to changes to the data segment before it's mmap'd).

I'm surprised this approach always seems to work on my end (based on the unit tests) - but I've only tried a few flavors of Linux. MacOS definitely doesn't like it at all, I get a failure on fwrite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (shm_size == 0) return NULL;

shm_unlink(key);
int fd = shm_open(key, O_RDWR | O_CREAT | O_TRUNC, 0666);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can permissions be tighter, say 0600?

@davidozog
Copy link
Member Author

@markbrown314 - do you think it's possible to use tmpfs instead of the shm_ APIs to avoid some of the tricky issues mentioned above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants