prov/shm: Revert to a lock-unlock inject pool#12109
Open
zachdworkin wants to merge 6 commits into
Open
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
zachdworkin
force-pushed
the
lock
branch
5 times, most recently
from
edd82eb to
f1014f7
Compare
zachdworkin
force-pushed
the
lock
branch
9 times, most recently
from
d956d04 to
4bff538
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>
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>
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. Remove the smr_format_inject function. We need to try to get a tx_buf and try again if we run out as soon as possible. Since we always need a tx_buf we are avoiding the ofi_buf_alloc call in the case where there are no more inject buffers. Pulling this code out of the format_inject function makes that function copy into the tx_buf and set the proto to inject which does not need to be its own function. Instead we can order the operations from that function in more optimal locations inside of do_inject function. 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>
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 check and WARN the user if we accidentally do a 0-byte copy in SAR. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com>
Remove smr_proto_inject as a selectable proto for ofi_op_read_req. This will revert the behavior to be the same as 2.4.x for this protocol. We want to choose iov here because we can avoid the bottleneck issue of when a receiver runs out of inject-buffers, and we can use the command to do an rma operation instead of copy-in copy-out operation of the inject-protocol. Try to grab an inject buffer before formatting anything else. This is one of the most expensive lookups we need to do and it is not guaranteed to always get a buffer. Since we always need one, try to get it first so that if it fails, the fail case is quicker at failing. Signed-off-by: Zach Dworkin <zachary.dworkin@intel.com>
Contributor
|
bot:aws:retest |
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
5 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.