Skip to content

Conversation

@bharatpotnuri
Copy link
Contributor

The rdmacm CPC in the openib BTL is not thread safe. The rdmacm CPC
should disqualify itself (instead of failing in random ways) if
MPI_THREAD_MULTIPLE is the thread level.

Fixes #1848

Signed-off-by: Potnuri Bharat Teja bharat@chelsio.com

@bharatpotnuri
Copy link
Contributor Author

Above commit is merged with master branch and needs to be cherry-picked into v2.x as well.
thanks.

@hjelmn hjelmn added this to the v2.1.0 milestone Oct 12, 2016
@hjelmn hjelmn self-assigned this Oct 12, 2016
@hjelmn hjelmn added the bug label Oct 12, 2016
Copy link
Member

@hjelmn hjelmn left a comment

Choose a reason for hiding this comment

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

You need to remove the include of the ompi header here.

#include "btl_openib_ip.h"
#include "btl_openib_ini.h"

#include "ompi/runtime/mpiruntime.h"
Copy link
Member

Choose a reason for hiding this comment

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

@hjelmn is referring to this line -- you can't include an OMPI header in OPAL source code.

The rdmacm CPC in the openib BTL is not thread safe. The rdmacm CPC
should disqualify itself (instead of failing in random ways) if
MPI_THREAD_MULTIPLE is the thread level.

Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
@hppritcha
Copy link
Member

adding pushback label here till mpiruntime.h is removed

@bharatpotnuri
Copy link
Contributor Author

@hjelmn @jsquyres My bad the header inclusion is unnecessary, I removed it.
Thanks.

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

@hppritcha Once @hjelmn approves the changes and CI finishes, good to go.

@hppritcha hppritcha merged commit c1bdb10 into open-mpi:v2.x Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants