Skip to content

ch4/ofi: make seg_sz a multiple of datatype size - #7897

Open
colleeneb wants to merge 3 commits into
pmodels:mainfrom
colleeneb:issue_7881
Open

ch4/ofi: make seg_sz a multiple of datatype size#7897
colleeneb wants to merge 3 commits into
pmodels:mainfrom
colleeneb:issue_7881

Conversation

@colleeneb

@colleeneb colleeneb commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Pull Request Description

This PR changes seg_sz to a multiple of the element size in MPIDI_OFI_am_isend_pipeline.

This is done because the packed_size is being made a multiple of the element size
in MPIR_Typerep_pack, but seg_sz has no such restriction, so in some cases they do not
match.

This is a potential fix for Issue #7881 .

With MPIR_CVAR_CH4_OFI_ENABLE_RMA=0 the reproducer in that issue hits
Assertion failed in file [...]/ofi/ofi_am_impl.h at line 413: packed_size == seg_sz,
which led to this PR.

Possibly there are other ways to get this work, but changing seg_sz to a multiple of
the element size seemed the cleanest to me. The code patch here is copied from
similar code in MPIR_Typerep_pack to match it exactly.

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@colleeneb colleeneb changed the title ch4/netmod/ofi: make seg_sz a multiple of the datatype extent ch4/netmod/ofi: make seg_sz a multiple of datatype size Jul 23, 2026
@colleeneb colleeneb changed the title ch4/netmod/ofi: make seg_sz a multiple of datatype size ch4/ofi: make seg_sz a multiple of datatype size Jul 23, 2026
Comment thread src/mpid/ch4/netmod/ofi/ofi_am_impl.h Outdated
if (elem_sz) {
seg_sz -= (seg_sz % elem_sz);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not robust since it assumes 1. contiguous datatype, 2. no alignment requirement. Rather, we should adjust am_hdr_sz so it landed on MAX_ALIGNMENT. I'll push a commit on top of this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks!

colleeneb and others added 2 commits July 23, 2026 17:38
The am payload data need land on MAX_ALIGNMENT or the limited seg_sz may
chop the data elements in half, resulting either ubsan error or
MPIR_Typerep_{pack,unpack} issue.
@hzhou

hzhou commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

test:mpich/ch4/ofi

We never call MPIDI_OFI_am_init_sreq with MPIDI_OFI_AMREQUEST(sreq,
sreq_hdr) set. Remove the dead branch.
@hzhou

hzhou commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

test:mpich/ch4/ofi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants