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

coll/base: fix memory allocation in mca_coll_base_alltoall_intra_basi… #686

Conversation

ggouaillardet
Copy link
Contributor

…c_inplace

(cherry picked from commit open-mpi/ompi@0f23037)

@mellanox-github
Copy link

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

@ggouaillardet
Copy link
Contributor Author

:bot:assign: @bosilca
:bot🏷️bug
:bot:milestone:v2.0.0

@ompiteam-bot ompiteam-bot added this to the v2.0.0 milestone Oct 19, 2015
@jsquyres
Copy link
Member

@bosilca gave this a 👍 in #687.

jsquyres added a commit that referenced this pull request Oct 19, 2015
…lltoall_intra_basic_inplace

coll/base: fix memory allocation in mca_coll_base_alltoall_intra_basi…
@jsquyres jsquyres merged commit 55b03ed into open-mpi:v2.x Oct 19, 2015
@bosilca
Copy link
Member

bosilca commented Oct 19, 2015

Actually, I was holding on this one, because I think the whole allocation of memory for collective operations is wrong. But, I need a little bit more time to think about

@jsquyres
Copy link
Member

@bosilca Doh. Should I revert?

@bosilca
Copy link
Member

bosilca commented Oct 19, 2015

Leave it as is by now. Once I have a better understanding of the issue, I
will try to propose a cleaner approach.

On Mon, Oct 19, 2015 at 1:52 PM, Jeff Squyres [email protected]
wrote:

@bosilca https://github.com/bosilca Doh. Should I revert?


Reply to this email directly or view it on GitHub
#686 (comment)
.

@bosilca
Copy link
Member

bosilca commented Oct 21, 2015

The more I look at this patch, the more confused I get. The only thing this change does is slightly reducing the allocated memory. However, now the allocated memory and the max_size are out of sync, as one includes the gap while the other doesn't. In addition the allocated buffer is not consistent with the amount of data allocated, as it has not been moved back by the true_lb (as in the other functions).

@bosilca
Copy link
Member

bosilca commented Oct 21, 2015

I just pushed a PR to clean the handling of temporary buffers (mainly in collectives). Please take a look @ open-mpi/ompi#1047. However, it is just a bandaid, the root of all these problems still exists.

@ggouaillardet
Copy link
Contributor Author

my patch does change the size of the allocated memory.
if extent < true_extent (in Fortran, think of a matrix row), the allocated memory is slightly increased
i agree this patch does not handle non-zero true_lb.
i will review open-mpi/ompi#1047 from now

@bosilca
Copy link
Member

bosilca commented Oct 21, 2015

You're right, in the case where the extent was altered by a resize you used to formula that gives the right span of the data.

alinask pushed a commit to alinask/ompi-release that referenced this pull request Dec 10, 2015
…-fixes

bool: use SIZEOF__BOOL, not SIZEOF_BOOL
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.

5 participants