-
Notifications
You must be signed in to change notification settings - Fork 936
v5.0.x: Clean up OFI common code and delay patcher initialization until needed #9449
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
v5.0.x: Clean up OFI common code and delay patcher initialization until needed #9449
Conversation
Remove cruft from common_ofi, including unnecessary includes, declarations, and OPAL_DECLSPEC annotations on function definitions (they are only needed on declarations). Also move one function back into the C_DECLS block and properly OPAL_DECLSPEC the function. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 00a5108)
Test for the features needed to support injecting a memory monitor in Libfabric, rather than testing for a Libfabric version. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 947b3fe)
The documentation in the header file for the ofi common code had been neglected, so show some love. Also add some helpful comments in understanding the memory monitor injection code. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 348152a)
Rename initialization calls to match the MCA terminology and restructure code to match the calling pattern of normal components (so during component_register, OFI components should call common_ofi_mca_register). Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit bf36d24)
Delay initializing the import memory monitor from component open to a function which should be called immediately before the Libfabric domain is initialized. Registering the import memory monitor requires initializing the patcher memory hooks, which is not necessary if the OFI fabrics were not selected. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 2349f6b)
|
@bwbarrett Would it be ok if I cherry picked 1797341 to this PR (it's the OFI warning fix from #9450)? The cherry pick doesn't apply cleanly to v5.0.x, but does apply cleanly to this branch. |
|
I'll add it; I left this WIP because there's another fix that needs to go in w.r.t the output stream. |
Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 1797341)
The ofi common refactoring patches broke handling of the verbose flag because I forgot that the input variable has to live longer than the function (sigh). Move the output level variable to be a global variable. Add error checking to the entire mca registration path, since it was notably lacking. Even before the refactor, we didn't handle re-setting verbosity after registering synonyms properly. This patch probably isn't perfect in that regards, but at least is *consistent* in what we do. Signed-off-by: Brian Barrett <[email protected]> (cherry picked from commit 5432c32)
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.
Backport of #9441. References #8822.