Add support for disabling eager (and fix 0 byte eager max size) #794
Conversation
This file contains 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
Parts of the freelist assume entry_size is larger than 0 bytes, but the interface does not check for size and does not specify restrictions either way. Improve the situation by overriding a 0 byte entry size to be 8 bytes (plus any additional size increases caused by the alignment and redzone requirements). This seems better than a bunch of 0 byte checks through the rest of the code. Signed-off-by: Brian Barrett <bbarrett@amazon.com>
yexiang-aws
reviewed
Feb 21, 2025
yexiang-aws
reviewed
Feb 21, 2025
Previously, we could only set the eager limit to 0, and even then there was logic to always allow 0 byte messages to be sent eager. This patch makes -1 a valid value for the eager size, which will disable eager messages entirely (including posting buffers). Signed-off-by: Brian Barrett <bbarrett@amazon.com>
We don't think we need eager with the performance improvements on P5 and P5en, so set the default to -1 (disabled). Signed-off-by: Brian Barrett <bbarrett@amazon.com>
bwbarrett
force-pushed
the
feature/really-no-more-eager
branch
from
1673899 to
b072322
Compare
yexiang-aws
approved these changes
Feb 24, 2025
rauteric
approved these changes
Feb 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two changes that ended up being the same code: fix 0 byte eager max size and add support for disabling eager messages completely.
0 byte eager support broke with #614, with a number of problems in the code. First, freelist code doesn't work with 0 byte entry size. #789 got us closer, fixing the assert, but some versions of the EFA provider (like that in 1.22 with zero copy) will return an EINVAL when trying to post a receive with len = 0. We split the eager size configuration into the
eager_rx_buff_sizeandeager_send_sizeexplicitly so that the buffer size can be larger than the max send size, and make sure that theeager_rx_buff_sizeis always larger than 0.Previously, the EAGER_MAX_SIZE environment variable was an unsigned integer, meaning that the smallest legal value was 0. Since the size is the max size that will be eager sent, that means that there's no way to completely avoid eager messages. This patch series also changes the environment variable (and related code) to be a signed integer, so that we can specify
-1as a legal eager size and completely disable eager messages. When the eager size is less than 0, we don't post rx buffers on the eager endpoint or create the eager free list.Finally, this patch series sets the default EAGER_MAX_SIZE to -1 to disable eager messages for performance analysis.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.