Properly del_proc spawned procs upon instance finalize#13667
Properly del_proc spawned procs upon instance finalize#13667Dupratj wants to merge 2 commits intoopen-mpi:mainfrom
Conversation
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 61ca5fd: [pml/ubcl] Removed bad assert: a PML should suport...
1f1f633: Properly del_proc spawned procs upon instance fina...
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
Signed-off-by: DUPRAT, JULIEN <julien.duprat@eviden.com>
Signed-off-by: DUPRAT, JULIEN <julien.duprat@eviden.com>
Dupratj
force-pushed
the
spawn_del_proc_main
branch
from
61ca5fd to
4379f15
Compare
Dupratj
changed the title
|
The NVIDIA failure is not specific to this PR, something about NVIDIA CI breakage that's been going on for weeks so we'll ignore it. |
| * A: Most likely not, but it would be good to check. | ||
| */ | ||
| #define PREDEFINED_INSTANCE_PAD 512 | ||
| #define PREDEFINED_INSTANCE_PAD 1024 |
There was a problem hiding this comment.
this is a bit problematic as it represents an ABI break. any code using MPI_SESSION_NULL would get a runtime warning about an object being resized if it was originally linked against libmpi.so from the 5.0.x release but was not being run against the libmpi.so from main or v6.0.x. Did you have to increase this to get the code to compile?
There was a problem hiding this comment.
I did some run the spawn tests in the ompi-tests repo and did not see any regressions from main. Note several tests continued to fail but that's not due to changes in this PR.
There was a problem hiding this comment.
I didn't realize it was an ABI break.
Yes the instance size is bigger than this PREDEFINED_INSTANCE_PAD with the additions of this PR. One thing I could do is use a pointer to a struct and allocate the needed memory after that way we would not exceed the 512 bytes limit.
Unless you see a better way
There was a problem hiding this comment.
Another option would be to change the char i_name[MPI_MAX_OBJECT_NAME]; to being a pointer. See for example commit 2d68804 for how this was done for communicators
There was a problem hiding this comment.
you may want to do that sort of a change in separate PR.
|
@Dupratj please address @hppritcha comment, as is, it can't go in |
3 participants
Upon finalize the ressources allocated for a spawned proc are not all released. This Pull request is a take over of 13445