-
Notifications
You must be signed in to change notification settings - Fork 463
prov/cxi: implement fi_writedata support #11684
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
base: main
Are you sure you want to change the base?
Conversation
1997e1c to
c5b3403
Compare
|
@ryanhankins - Nit, individual commit heading should be "prov/cxi" or "man/fi_cxi" for these commits. The last commit in particular should also clarify "prov/cxi" as cxi unit tests are specifically being added along with your man page update. |
swelch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments for possible changes. Also, in the unit tests commit could you also update prov/cxi/test/test.sh to include a new long test suite option that sets "FI_CXI_ENABLE_WRITEDATA=1" and filters on "rma*/*". This will verify both existing RMA tests are not impacted when writedata is enabled and that the new writedata works as advertised. Thanks for getting this together.
c5b3403 to
b2db29c
Compare
81123bd to
c329019
Compare
swelch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Ryan for the changes. One more comment on documentation and two changes to consider on forcing FI_MR_PROV_KEY usage. Changes look good.
128e65f to
1089aec
Compare
swelch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
5b1cb52 to
8fe511b
Compare
8fe511b to
a73b5e7
Compare
|
@shijin-aws - any worries on the AWS CI failure, changes are self-contained to cxi? |
|
bot:aws:retest |
3 similar comments
|
bot:aws:retest |
|
bot:aws:retest |
|
bot:aws:retest |
swelch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See latest comments regarding a few changes.
f8d15ed to
1c01479
Compare
|
Thank you for looking at this. I believe I've resolved the comments. |
Signed-off-by: Ryan Hankins <[email protected]>
Bumped the CXI provider minor version to indicate added support for the fi_writedata API. Integrated writedata handling by gating RMA writedata support on domain->rma_cq_data_size and updated MR and RMA code paths to use this indicator; adjusted event and key handling for writedata dual-entry mode. Signed-off-by: Ryan Hankins <[email protected]>
Add fi_writedata test and update fi_cxi.7.md Signed-off-by: Ryan Hankins <[email protected]>
1c01479 to
c5afe01
Compare
|
sielicki@e6f28a9 would supplant this |
Implement fi_writedata infrastructure and support in the CXI provider, updating
MR and RMA code paths and adjusting event/key handling for writedata dual-entry mode. Gate
RMA writedata on domain->rma_cq_data_size and bump the CXI provider minor version to indicate
the new API support. Add a fi_writedata test and update the fi_cxi.7 manpage.