Skip to content

Conversation

@alinask
Copy link
Member

@alinask alinask commented Oct 10, 2017

  • The sshmem verbs component will disqualify itself if this verb isn't
    present on the build host.
  • In case where support was requested but not found, don't stop the
    build - continue without this component.

Signed-off-by: Alina Sklarevich alinas@mellanox.com

@alinask
Copy link
Member Author

alinask commented Oct 10, 2017

@yosefe @miked-mellanox @alex-mikheev @jladd-mlnx Please review.

ibv_exp_reg_shared_mr(&in_smr);
]])],
[oshmem_have_exp_reg_shared_mr=1],
[oshmem_have_exp_reg_shared_mr=0])
Copy link
Contributor

Choose a reason for hiding this comment

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

can we set oshmem_verbs_sm_build_verbs=0 directly from here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, but this is how it is also done in the next check in this file so i kept the consistency

AS_IF(
[test "$oshmem_verbs_sm_build_verbs" = "1"],
[
OSHMEM_LIBSHMEM_EXTRA_LDFLAGS="$OSHMEM_LIBSHMEM_EXTRA_LDFLAGS $oshmem_verbs_LDFLAGS"
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it needed to set these variables?

AC_SUBST([oshmem_verbs_LDFLAGS])
AC_SUBST([oshmem_verbs_LIBS])

oshmem_have_exp_reg_shared_mr=0
Copy link
Contributor

Choose a reason for hiding this comment

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

looking at the m4 file, seems like this check(AC_COMPILE_IFELSE) should replace these lines:

            AC_CHECK_DECLS([IBV_EXP_ACCESS_ALLOCATE_MR,IBV_EXP_ACCESS_SHARED_MR_USER_READ],
                   [oshmem_have_mpage=3], [],
                   [#include <infiniband/verbs.h>])

because it's also possible that the "old" API of shared_mr is supported (oshmem_have_mpage=2) and then no need to disable the verbs component

@hppritcha
Copy link
Member

had to kill off armv8 testing.
bot:lanl:retest

@alinask alinask force-pushed the topic/oshmem_config_ibv_exp_reg_shared_mr branch from 0286acc to 4dcc85b Compare October 11, 2017 10:30
@alinask
Copy link
Member Author

alinask commented Oct 11, 2017

@yosefe updated.

AS_IF([test "$oshmem_have_mpage" = "3"],
[
oshmem_verbs_save_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $oshmem_verbs_save_CFLAGS -Wno-strict-prototypes -Werror"
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to add $oshmem_verbs_save_CFLAGS

in_smr.exp_access = access_flags;
ibv_exp_reg_shared_mr(&in_smr);
]])],
[oshmem_verbs_sm_build_verbs=1],
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to set oshmem_verbs_sm_build_verbs=1 here, just set it to 0 in the 'else' case

+ The sshmem verbs component will disqualify itself if this verb isn't
present on the build host.
+ In case where support was requested but not found, don't stop the
build - continue without this component.

Signed-off-by: Alina Sklarevich <alinas@mellanox.com>
@alinask alinask force-pushed the topic/oshmem_config_ibv_exp_reg_shared_mr branch from 4dcc85b to 3008827 Compare October 12, 2017 16:58
@alinask
Copy link
Member Author

alinask commented Oct 12, 2017

@yosefe updated, please review

@yosefe yosefe merged commit 835fa33 into open-mpi:master Oct 13, 2017
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