Skip to content

fix osc rdma origin/target extent mixup -- Fixes 3569 #3570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

markalle
Copy link
Contributor

The MPI_Accumulate call has a local count/type and a target count/type
and a range check was being performed with remote_count * target_extent.
I changed them both to target.

Signed-off-by: Mark Allen [email protected]

The MPI_Accumulate call has a local count/type and a target count/type
and a range check was being performed with remote_count * target_extent.
I changed them both to target.

Signed-off-by: Mark Allen <[email protected]>
@bwbarrett
Copy link
Member

bot:ompi:retest

Looks like the ARM builder crashed last night.

@jjhursey
Copy link
Member

References Issue #3569

@markalle add the "Fixes 3569" to the body of the PR description not the title.

@bwbarrett
Copy link
Member

This patch and #3572 are the same thing. Someone should sort that out...

Copy link
Member

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

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

I like this better than #3572, but you should rename existing uses of extent to origin_extent to be more clear. Fix both the immediate bug and the likelihood of more bugs down the line...

@markalle
Copy link
Contributor Author

I'm okay with using 3572. It looks like we're both fixing the same bug. Overall this code seems to be oriented toward frequent calls to ompi_datatype_get_extent() with extent being reused and referring to whatever was its most recent call.

I probably would have gone the other way with a target_extent and origin_extent calculated once, but I don't mind the existing coding style.

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