Skip to content

Conversation

@hjelmn
Copy link
Member

@hjelmn hjelmn commented Sep 24, 2015

The allocator, rcache, and mpool frameworks were moved down to opal
some time ago. The framework open calls should have been moved as
well.

Signed-off-by: Nathan Hjelm hjelmn@lanl.gov

The allocator, rcache, and mpool frameworks were moved down to opal
some time ago. The framework open calls should have been moved as
well.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
The mca_mpool_base_init function did nothing other than set a few
static variables. These variables are not used ever. If mpool
components wish to know that threads are in use they can use
opal_using_threads ().

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
@hjelmn
Copy link
Member Author

hjelmn commented Sep 24, 2015

@jsquyres I just noticed something. ompi_mpi_init/finalize use opal_init/finalize_util and not the full opal_init/finalize. Do you know if there is a reason for that? Seems like a bug to me. We are getting lucky because orte does use opal_init/finalize.

opal_init is responsible for loading frameworks that will be used by
the ompi layer. Many of these frameworks are not loaded by
opal_init_util. Before this commit things were working because
orte_init does use opal_init.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
@jsquyres
Copy link
Member

Generally looks ok to me. @bosilca, can you confirm?

I'm trying to remember if there's a reason that the OMPI layer uses opal_init|finalize_util()... Best I can think of is that we wanted to initialize as little of OPAL as possible before initializing the RTE, which would assumedly initialize the rest of OPAL. But if we ran into a failure somewhere before ORTE initialized OPAL, then we might as well have initialized as little of OPAL as possible.

That being said, perhaps that's a false assumption, anyway: the RTE may not be ORTE. Hence, we should probably fully initialize OPAL, anyway.

@bosilca @hjelmn Does this sound right to you?

@bosilca
Copy link
Member

bosilca commented Sep 29, 2015

  1. The 3 frameworks you mentioned (allocator, rcache, and mpool) were opened by the MPI layer in purpose. The design was to minimize all impact on the code, in other words if you don't need them don't open them. For the same reason the BTLs are opened explicitly by the BML (and never by default by the OPAL layer).
  2. opal_init vs. opal_init_utils. We wanted to be able to change the MCA parameters before OPAL opened and initialized all the frameworks. Thus, we opal_init_utils just to setup the MCA params. I have a vague recollection about some issue with the event library ...
  3. It was expected that the RTE_open will do an opal_init. However, as we refcount the opal_init we can now do multiple init/fini, so we can add a safeguard opal_init in ompi_mpi_init.

@bosilca
Copy link
Member

bosilca commented Nov 5, 2015

I don't see why we need to force the opening of all OPAL components from the beginning. I left the allocator, mpool, rcache and btl up in purpose in the OMPI layer as it was the first place where they were needed. Do we now have a reason to open everything down in OPAL?

@hjelmn
Copy link
Member Author

hjelmn commented Nov 10, 2015

i will close this. i plan to move the mpool open down to opal (since the mpool has changed quite a bit) but will leave the others where they are.

@hjelmn hjelmn closed this Nov 10, 2015
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Sep 19, 2016
…bs-usnic-by-default

v1.10: verbs_usnic: do not build by default
@hjelmn hjelmn deleted the framework_fixes 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