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
verbs: Introduce verbs_cq for extended CQ operations #777
Conversation
libibverbs/verbs.h
Outdated
pthread_mutex_t mutex; | ||
pthread_cond_t cond; | ||
uint32_t comp_events_completed; | ||
uint32_t async_events_completed; |
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.
We should prevent changing a structure that is exposed from verbs.h , there might be some application that was written with direct usage of one on applicable/exposed fields (e.g. channel, cq_context, etc.) that post of such change might fail to compile.
ibv_qp_ex was introduced from day one with qp_base comparing this one.
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.
You're definitely right, missed that.
struct ibv_cq cq; | ||
struct ibv_cq_ex cq_ex; | ||
}; | ||
}; |
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.
What is the use case that you come to solve with this patch ? is it only some preparation for future work ? we usually prefer having some series with some added value / usage and not only preparations.
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.
This is a cleanup patch, preventing the need for explicit casts. The use case is the providers supporting extended CQ, mlx4/5 and any future providers.
Extended QP implementation has a nice verbs_qp abstraction which does not exist for extended CQs, this patch modifies CQ to be similar. The verbs_cq struct is a union of both CQ and extended CQ, which allows access for both. In addition, converting a CQ to the provider's CQ can now be done using container_of, without explicit casts. Signed-off-by: Gal Pressman <galpress@amazon.com>
e0300f1
to
195c919
Compare
Kind reminder. |
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.
Gal, the patch was already taken into our regression, once will get a green light may take it.
Extended QP implementation has a nice verbs_qp abstraction which does
not exist for extended CQs, this patch modifies CQ to be similar.
The verbs_cq struct is a union of both CQ and extended CQ, which allows
access for both. In addition, converting a CQ to the provider's CQ can
now be done using container_of, without explicit casts.
Signed-off-by: Gal Pressman galpress@amazon.com