Fork support #4
Open
Fork support #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add msk_reset_lib() interface that allows to reinitialize the library global structure in a child process after a fork. It doesn't free the rdma resources that may have been allocated by parent and haven't been freed before fork (they are leaked in the child process), but prevents some hangs and crashes if the process doesn't exec() after fork(). Better than nothing. msk_reset_lib() also calls Mellanox-specific routine rdma_lib_reset() to reset librdmacm state if Mooshika has been compiled with Mellanox's librdmacm.
Leak less fd's in child process by closing epoll fd stored in global structure.
eventfd's + some memory
martinetd
reviewed
May 17, 2019
| /* Brutal re-initialization of global state */ | ||
| memset(msk_global_state, 0, sizeof(*msk_global_state)); | ||
|
|
||
| msk_global_state->run_threads = 0; |
Collaborator
There was a problem hiding this comment.
I think we should do that beforefreeing the various items/closing fd and try to join threads like in msk_internals_fini
Actually we might be able to reuse these directly, it will free/realloc msk_global_state but the cost is negligible compared to thread joins etc so just something like msk_internals_fini(); rdma_lib_reset(); msk_internals_init(); is probably cleanest?
martinetd
reviewed
May 17, 2019
| free(msk_global_state->worker_pool.thrids); | ||
|
|
||
| if (msk_global_state->worker_pool.wd_queue) | ||
| free(msk_global_state->worker_pool.wd_queue); |
Collaborator
There was a problem hiding this comment.
(these are missing in msk_internals_fini and should be added there regardless of reusing the functions; thanks for finding these)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here is a PR that adds to Mooshika some kind of support for applications that forks.
It adds two things:
msk_fork_init() : an helper that exposes ibv_fork_init() in the Mooshika API
msk_lib_reset() : a function a process can call to reset the library global state (to be called by child process after fork())
mks_lib_reset() doesn't free the rdma resources that may have been allocated by parent and haven't been freed before fork (they are leaked in the child process), but prevents some leaks, hangs and crashes if the process doesn't exec() after fork(). It is a "better than nothing" solution.
msk_reset_lib() also calls Mellanox-specific routine rdma_lib_reset() to reset librdmacm state if Mooshika has been compiled with Mellanox's librdmacm.
In our product (IO interception via a LD_PRELOAD'ed library and redirection to a proxy via IB), we need these interfaces, because we don't know in advance what is the behavior of the processes we're instrumenting. We don't if they will fork() or not, and we must be prepared for this case.