Skip to content

cm: fix selection priority #1039

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

Closed
wants to merge 2 commits into from
Closed

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Oct 19, 2015

This patch removes a priority check that disables cm if the previous
pml had higher priority. The check was incorrect as coded and is
unnecessary as we finalize all but one pml anyway.

Fixes #1035

Signed-off-by: Nathan Hjelm [email protected]

This patch removes a priority check that disables cm if the previous
pml had higher priority. The check was incorrect as coded and is
unnecessary as we finalize all but one pml anyway.

Fixes open-mpi#1035

Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn hjelmn mentioned this pull request Oct 19, 2015
@jsquyres
Copy link
Member

Per #1035 (comment), should the same check be made to ob1?

This commit removes code that checks the ob1 priority vs the previous
priority. The previous priority is meaningless here and may only cause
ob1 to disable itself when it shouldn't.

Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

@jsquyres Removed the ob1 check as well. I see no reason to disable any pml component in this manner.

@bosilca
Copy link
Member

bosilca commented Oct 19, 2015

With this approach we don't need the field default_priority in the CM PLM.

@jsquyres
Copy link
Member

@yburette Does this fix your openib / PSM issue?

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

@bosilca I will remove the default priority in CM completely in another commit. I want to have the mtl components set the priority of cm if a different manner. Should have the PR up soon.

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

For history's sake here is the commit that added the extra selection checks:
a94101fa

Since dr is dead I think it is safe to remove them.

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

Will handle this all in #1040.

@hjelmn hjelmn closed this Oct 19, 2015
jsquyres added a commit to jsquyres/ompi that referenced this pull request Aug 23, 2016
…-barrier-bug

v2.x: oshmem/scoll: Fix bug in basic/barrier algorithm
@hjelmn hjelmn deleted the pml_priority branch July 12, 2018 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants