On 09/03/2015 10:56 AM, Haggai Eran wrote: > This series adds userspace support for on-demand paging. The first patch adds > support for the new extended query device verb. Patch 2 adds the capability and > interface bits related to on-demand paging, and patch 3 adds example code to > the rc_pingpong program to use on-demand paging. I've reviewed this series and I'm OK with it. However, it's currently blocked. We need to fix the API mess that was created when the create/destroy flow verbs were added. I've gone back through the code, and I'm pretty sure I know what Jason's objection to the create/destroy flow implementation was. In particular, we weren't supposed to have this: struct verbs_context { /* "grows up" - new fields go here */ + int (*drv_ibv_destroy_flow) (struct ibv_flow *flow); + int (*lib_ibv_destroy_flow) (struct ibv_flow *flow); + struct ibv_flow * (*drv_ibv_create_flow) (struct ibv_qp *qp, + struct ibv_flow_attr + *flow_attr); + struct ibv_flow * (*lib_ibv_create_flow) (struct ibv_qp *qp, + struct ibv_flow_attr + *flow_attr); There was only supposed to be ibv_create_flow and ibv_destroy_flow, not this silly separate drv/lib versions that just amount to wasted space and confusion. Our choices are: 1) Leave this in place but don't do any more extensions this way, knowing full well that this is wrong but preserving ABI or 2) Clean this up, but break ABI. Just to be clear, neither solution breaks user space app ABI, just the libibverbs/libmlx4 ABI. Option 2 does not require any apps be recompile, only that libibverbs and libmlx4 *must* be upgraded together or downgraded together around the ABI break. For future versions beyond that, we don't have to have this requirement. What are other people's thoughts on this? I'm of a mind to clean it up, but it's been since May 5th of last year that this went out. Certainly the largest portion of users are likely running this code. It will mean that their next upgrade is required to be a joint upgrade between this and libmlx4. > Changes from v1: > - Patch 1: > * move code to revert to legacy ibv_query_device when ibv_query_device_ex > is missing to the inline function. > * add an input parameter to the ibv_query_device_ex verb for future > extension. > * add the size of the ibv_device_attr_ex struct as a parameter to the > ibv_query_device_ex verb, to allow the verb to handle older > applications. > * check the validity of the input parameter and output struct size. > * remove reserved words from ibv_query_device_resp_ex, and remove unused > ibv_device_attr_ex_resp struct. > - Patch 2: > * let print_odp_caps() receive a const pointer instead of a by-value > struct. > * check that the application has enough space for ODP capabilities in the > provided ibv_device_attr_ex struct. > > Eli Cohen (1): > Add support for extended query device capabilities > > Haggai Eran (1): > Add on-demand paging support > > Majd Dibbiny (1): > libibverbs/examples: Support odp in rc_pingpong > > Makefile.am | 3 +- > examples/devinfo.c | 145 +++++++++++++++++++++++++++-------------- > examples/rc_pingpong.c | 31 ++++++++- > include/infiniband/driver.h | 10 +++ > include/infiniband/kern-abi.h | 36 ++++++++++- > include/infiniband/verbs.h | 68 ++++++++++++++++++- > man/ibv_query_device_ex.3 | 70 ++++++++++++++++++++ > man/ibv_reg_mr.3 | 2 + > src/cmd.c | 147 ++++++++++++++++++++++++++++++------------ > src/libibverbs.map | 2 + > 10 files changed, 420 insertions(+), 94 deletions(-) > create mode 100644 man/ibv_query_device_ex.3 > -- Doug Ledford <dledford@xxxxxxxxxx> GPG KeyID: 0E572FDD
Attachment:
signature.asc
Description: OpenPGP digital signature