ompi/oshmem: fix bug in shmem_finalize. #4982
Conversation
|
Excuse me - you don't have to open a new PR every time you want to make a change to it. You can simply push your change to the existing PR. This will save on the repeated emails being generated. If you don't want the multiple commits to show, then just: $ git rebase -i master
...squash all commits to one...
$ git push -fThanks |
|
@rhc54 sorry about that. there was an error on author name/email and I cannot fix it by force pushing. Will force push next time. |
|
Actually - you can do that too! Just amend your commit message (with git commit --amend) and re-push it. |
xinzhao3
force-pushed
the
topic/shmem-final
branch
from
61834b8 to
f1ef514
Compare
|
@rhc54 yeah, you are right. I do not need rebase. Thanks! |
xinzhao3
force-pushed
the
topic/shmem-final
branch
2 times, most recently
from
5ea692a to
ce7fbc4
Compare
|
@alex-mikheev Alex can you please have a look. |
|
bot:retest |
|
it looks ok. Is there a test in verifier for the explicit calling of shmem_finalize() ? |
|
bot:ompi:retest |
|
@alex-mikheev there is a test in ompi-tests/test-uh, path is ompi-tests/test-uh/feature_tests/C/test_shmem_finalize.c |
|
@gpaulsen FYI. |
|
Per https://www.mail-archive.com/devel@lists.open-mpi.org/msg20547.html: bot:ompi:retest |
|
@xinzhao3 The test is now passing for me. Verified failure before the fix was applied as well. |
|
@sam6258 that's great! Thanks for verifying it. |
|
@xinzhao3 can you add a test to the https://github.com/openshmem-org/tests-mellanox |
|
@alex-mikheev I will add the test. |
| { | ||
| int ret = OSHMEM_SUCCESS; | ||
| static int32_t finalize_has_already_started = 0; | ||
| int32_t _tmp = 0; |
There was a problem hiding this comment.
This value is no longer used.
There was a problem hiding this comment.
Thanks! Will re-push soon.
There was a problem hiding this comment.
@sam6258 I think I already deleted it in this commit, it is a '-' at beginning :)
There was a problem hiding this comment.
@xinzhao3 I was referring to _tmp
|
bot:ompi:retest |
Signed-off-by: Xin Zhao <xinz@mellanox.com>
xinzhao3
force-pushed
the
topic/shmem-final
branch
from
ce7fbc4 to
4aad386
Compare
|
@sam6258 Thanks, I just delete _tmp and re-pushed. |
|
bot:ompi:retest Brian broke the 32bit builds... |
|
ubuntu unhappy here too. |
|
@xinzhao3 please open PRs against 3.0.x and 3.1.x branches. |
8 participants
Originally there is a "oshmem_shmem_globalexit_status" variable to track if current shmem_finalize is the implicit one at end of main, and shmem_finalize does real work when it is, otherwise it does nothing.
This is not correct, if shmem_finalize is the explicit one in the program, it should still guarantee the completion of pending communications, according to specification 1.3 page 10.
This PR modified use of "oshmem_shmem_globalexit_status" variable so that if shmem_finalize is not the one at end of main, it still waits for all oshmem communications and release oshmem resources.
Signed-off-by: Xin Zhao xinz@mellanox.com