Skip to content

Change how cm's priority is calculated #1040

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

Merged
merged 4 commits into from
Oct 19, 2015
Merged

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Oct 19, 2015

@bosilca Please comment. This is a companion to or replacement for #1039.

@bosilca
Copy link
Member

bosilca commented Oct 19, 2015

Looks good. 👍

@yburette
Copy link
Member

@hjelmn With this priority scheme, one needs to specify which PML to use in order to be able to use the OFI MTL. Is there a way around this?

--mca mtl ofi --mca mtl_ofi_provider_include "psm" doesn't work; still selects ob1
--mca mtl ofi --mca mtl_ofi_provider_include "psm" --mca pml cm works

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

I didn't adjust ofi's priority. I didn't think it was considered ready for prime time. You can set using -mca mtl_ofi_priority 100 to force it to run. Should we increase the priority?

@yburette
Copy link
Member

I don't think we need to increase the priority yet.

I was just hoping there would be a way to automagically select cm when an MTL is specified on the command line :-)

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

You need to still force cm (or bump ofi's priority) because it shouldn't be selected before ob1 in production. Right now the component initialization code can not tell the difference between only one component being available and the user selecting a single component.

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

I should not it worked before because someone added ofi as one of the components that bumped cm's priority to 30.

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]>
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]>
The mca_base_select function uses returned priorities to select the
best component/module. This priority may be of use to the caller so
pass that information back in an optional argument. If the priority is
not needed pass NULL.

Signed-off-by: Nathan Hjelm <[email protected]>
This commit changes the priority of mtl components to be relative to
pml/ob1 and updates the mtl interface to expose this priority. cm now
sets its own priority based on the priority of the selected mtl
component.

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

So if I understand well, we should increase OFI MTL's priority to 30 in order to have what we had before the change, right?

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

Sort of. We need to bump psm to 31 first to keep it ahead of ofi. I left ofi's relative priority alone here (10).

We could also just set ofi between 20 (ob1) and 30 (psm).

@bosilca
Copy link
Member

bosilca commented Oct 19, 2015

My understanding of the code is that the CM and PML will now inherit the priorities of their underlying modules (the CM the max of all available MTL, and OB1 the max of all available BTL), or is getting it's own priority form the corresponding MCA parameter. Thus if we have a purely priority-based solution we can make everything work.

By default we can order the MTL in whatever order we consider adequate, but always higher than any BTL. Thus, as the OB1 will inherit the max priority of the BTLs, if any MTL is ready to work the CM (who will inherit this priority) will be picked up. Selecting the OB1 PML become as simple as increasing it's priority to something larger than any MTL.

Sounds simple to implement, and pretty logical for the users. It does change the default (MTL instead of PML), but the change is sensible and desirable.

@hjelmn
Copy link
Member Author

hjelmn commented Oct 19, 2015

@bosilca Pretty much. The only difference is the BTLs don't have a priority since there may be many transports in use. So with this commit ob1 still uses its own priority. In the future it might be useful to determine the priority for ob1 using the active btls. This way we could select ob1 when running only intra-node since vader is much faster than both mxm and psm.

hjelmn added a commit that referenced this pull request Oct 19, 2015
Change how cm's priority is calculated
@hjelmn hjelmn merged commit 9602484 into open-mpi:master Oct 19, 2015
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Sep 19, 2016
…c-barrier-bug

oshmem/scoll: Fix bug in basic/barrier algorithm
@hjelmn hjelmn deleted the mtl_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