Skip to content
This repository has been archived by the owner on Sep 30, 2022. It is now read-only.

opal/cma: improve Linux CMA detection #1306

Closed
wants to merge 1 commit into from
Closed

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Aug 8, 2016

This commit improves the CMA detection when the installed glibc doesn't
have support for CMA. In this case we need to verify that the syscall
numbers in opal/include/opal/sys/cma.h are valid for the architecture.
This verification is done by attempting to use CMA while including the
internal header.

This was back-ported to v2.0.x from master and does not include
support for ARMv8.

Signed-off-by: Nathan Hjelm hjelmn@me.com

(cherry picked from commit open-mpi/ompi@4a2bd83)

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

@hjelmn
Copy link
Member Author

hjelmn commented Aug 8, 2016

:bot:milestone:v2.0.1
:bot:assign: @hppritcha

This commit enables Linux CMA support by default. This will help shared memory performance.

@ompiteam-bot ompiteam-bot added this to the v2.0.1 milestone Aug 8, 2016
@lanl-ompi
Copy link
Contributor

Test FAILed.

1 similar comment
@lanl-ompi
Copy link
Contributor

Test FAILed.

@ibm-ompi
Copy link

ibm-ompi commented Aug 8, 2016

Build Failed with XL compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/1e504c62707ca698974c73d9950275fd

@ibm-ompi
Copy link

ibm-ompi commented Aug 8, 2016

Build Failed with GNU compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/8d5398ba793aa2bc4494abc3b6f7da49

@hjelmn
Copy link
Member Author

hjelmn commented Aug 8, 2016

Opps, error in three-way merge. Fixing.

@mellanox-github
Copy link

Test FAILed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2067/ for details.

@lanl-ompi
Copy link
Contributor

Test FAILed.

This commit improves the CMA detection when the installed glibc doesn't
have support for CMA. In this case we need to verify that the syscall
numbers in opal/include/opal/sys/cma.h are valid for the architecture.
This verification is done by attempting to use CMA while including the
internal header.

This was back-ported to v2.0.x from master and does not include
support for ARMv8.

Signed-off-by: Nathan Hjelm <hjelmn@me.com>

(cherry picked from commit open-mpi/ompi@4a2bd83)

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

hjelmn commented Aug 8, 2016

Threw away the merged version and just updated to master - the summary stuff.

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2068/ for details.

@hppritcha
Copy link
Member

looks good. @jsquyres you may want to double check the configury change.

@hppritcha hppritcha modified the milestones: v2.1.0, v2.0.1 Aug 16, 2016
@hppritcha
Copy link
Member

postponing this to 2.1


# If the user specifically requests CMA go ahead and enable it even
# if the glibc version does not support process_vm_readv
if test "x$with_cma" = "xyes" || test "$ompi_check_cma_happy" = "yes" ; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite grok this: why would we enable CMA if $ompi_check_cma_happy isn't yes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably can be improved but it doesn't hurt. This only happens if the user tries to force CMA using --with-cma=yes. The user will get a compile-time error if they try to force CMA to be enabled and either 1) we do not have the syscall numbers, or 2) the syscall numbers are wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not really the way we do this in the rest of OMPI. If the user specifies --with-FOO[=yes] and we can't compile/link with FOO support, then configure should fail (vs. failing much later in the make).

@jsquyres
Copy link
Member

I realize this is already on master, but it might still be worth strengthening the master version, too.

@jsquyres
Copy link
Member

Please see open-mpi/ompi#1997.

@jsquyres
Copy link
Member

@hjelmn I have merged open-mpi/ompi#1997 to master -- do you want to pull that commit in here?

@hppritcha
Copy link
Member

reassigning to @hjelmn to get configury fixes per @jsquyres recommendations

@jsquyres
Copy link
Member

The ompi-release repo is now closed; all release branches have been moved to the ompi repo (https://github.com/open-mpi/ompi). Please re-open this PR on the corresponding branch on the ompi repo.

@jsquyres
Copy link
Member

jsquyres commented Feb 7, 2017

This repo is now closed.

@jsquyres jsquyres closed this Feb 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants