prov/shm: Revert to a lock-unlock inject pool#12109
Open
zachdworkin wants to merge 7 commits intoofiwg:mainfrom
Open
prov/shm: Revert to a lock-unlock inject pool#12109zachdworkin wants to merge 7 commits intoofiwg:mainfrom
zachdworkin wants to merge 7 commits intoofiwg:mainfrom
Conversation
Contributor
Author
|
bot:aws:retest |
1 similar comment
Contributor
|
bot:aws:retest |
j-xiong
previously approved these changes
Apr 5, 2026
Contributor
Author
|
bot:aws:retest |
Contributor
Author
|
@sunkuamzn what is the AWS failure with this PR? |
Contributor
|
@zachdworkin There is single node unexpected message test timeout |
zachdworkin
force-pushed
the
lock
branch
2 times, most recently
from
6ab0fc8 to
7d18904
Compare
zachdworkin
force-pushed
the
lock
branch
3 times, most recently
from
fe058b6 to
affab85
Compare
zachdworkin
force-pushed
the
lock
branch
2 times, most recently
from
915f9f0 to
30340a0
Compare
Place lighter protocol fields in the same cache-line grab as the atomic-queue cmd_entry grab. This way we if we are using a cpu without prefetcing algorithms (to grab the adjacent cacheline for us) we are optimizing the access of the fields we need for the lightweight/fast protocols. The heavier/slower protocols which use the second cache line fields will be unaffected by this change on older cpus since they already need to access both cache-lines. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com> Signed-off-by: Alexia Ingerson <alexia.ingerson@intel.com>
Remove the parallel command-inject resources and revert to using a lock-unlock inject buffer pool. Update the inject protocol to use the old method. There is a performance regression when using the "new shm" command-inject parallel data structure. This is due to the sender not being able to complete its transmission until the receiver returns the sender's command to the sender's return queue. In the old lock-unlock method the sender would allocate receiver side resources, copy its data into the receiver inject buffer and then complete. The old method allows MPIs and applications to assume that their inject message transmissions will complete quickly and since the new method does not complete `as` quickly it is likely the reason for this regression. This will also revert to the "old-shm" method of buffering all unexpected inject messages Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com> Signed-off-by: Alexia Ingerson <alexia.ingerson@intel.com>
Replace hdr.status with hdr.smr_flags to indicate any error. This error will use the flag SMR_OP_ERROR for the sender to process its errors on return cmd. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com> Signed-off-by: Alexia Ingerson <alexia.ingerson@intel.com>
zachdworkin
force-pushed
the
lock
branch
2 times, most recently
from
bf4cfb0 to
edd82eb
Compare
Command stack is less likely to be used in the inject protocol when resources are on the receiver side. If the inject pool is above it then we have to jump less, and do not have to jump over the command stack, when accessing it. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com>
SAR should never be handling 0 byte copies anymore since the inject protocol can handle delivery complete. Instead we will assert to make sure we aren't accidentally doing a 0-byte copy in SAR. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com>
Signed-off-by: Alexia Ingerson <alexia.ingerson@intel.com>
Signed-off-by: Alexia Ingerson <alexia.ingerson@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the parallel command-inject resources and revert to using a lock-unlock inject buffer pool.
Update the inject protocol to use the old method.
There is a performance regression when using the "new shm" command-inject parallel data structure. This is due to the sender not being able to complete its transmission until the receiver returns the sender's command to the sender's return queue. In the old lock-unlock method the sender would allocate receiver side resources, copy its data into the receiver inject buffer and then complete. The old method allows MPIs and applications to assume that their inject message transmissions will complete quickly and since the new method does not complete
asquickly it is likely the reason for this regression.In addition we need to move the inject stack above the cmd stack in the shm region. This is because most of the time the new inject protocol will not need to use the cmd stack. If we swap the ordering in the region then we have to do less jumps through memory to get the resources we need.
We can also use the ofi freestack for the cmd stack because we only need to pop and push on the sender side so we don't need the extra functionality of the smr_freestack.