Shared PD and MR
Subsystems (5)
cover Shared PD and MR
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
Patch-set is logically splits to 4 parts as the following:
- patches 1 to 8 and 18 are preparation steps.
- patches 9 to 14 are the implementation of import PD
- patches 15 to 17 are the implementation of the verb
- patches 19 to 14 are the implementation of import MR
Shamir Rabinovitch (17):
RDMA/uverbs: uobj_get_obj_read should return the ib_uobject RDMA/uverbs: Delete the macro uobj_put_obj_read RDMA/nldev: ib_pd can be pointed by multiple ib_ucontext IB/{core,hw}: ib_pd should not have ib_uobject pointer IB/core: ib_uobject need HW object reference count IB/uverbs: Helper function to initialize ufile member of
uverbs_attr_bundle
IB/uverbs: Add context import lock/unlock helper IB/uverbs: ufile must be freed only when not used anymore IB/verbs: Prototype of HW object clone callback IB/mlx4: Add implementation of clone_pd callback IB/mlx5: Add implementation of clone_pd callback RDMA/rxe: Add implementation of clone_pd callback IB/uverbs: Add clone reference counting to ib_pd IB/uverbs: Add PD import verb
IB/mlx4: Enable import from FD verb
IB/mlx5: Enable import from FD verb
RDMA/rxe: Enable import from FD verb
Yuval Shaia (8):
IB/core: Install clone ib_pd in device ops IB/core: ib_mr should not have ib_uobject pointer IB/core: Install clone ib_mr in device ops IB/mlx4: Add implementation of clone_pd callback IB/mlx5: Add implementation of clone_pd callback RDMA/rxe: Add implementation of clone_pd callback IB/uverbs: Add clone reference counting to ib_mr IB/uverbs: Add MR import verb
drivers/infiniband/core/device.c | 2 + drivers/infiniband/core/nldev.c | 127 ++++- drivers/infiniband/core/rdma_core.c | 52 +- drivers/infiniband/core/uverbs.h | 24 + drivers/infiniband/core/uverbs_cmd.c | 505 +++++++++++++++--- drivers/infiniband/core/uverbs_main.c | 5 + drivers/infiniband/core/uverbs_std_types_mr.c | 1 - drivers/infiniband/core/verbs.c | 4 - drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 1 - drivers/infiniband/hw/mlx4/main.c | 18 +- drivers/infiniband/hw/mlx5/main.c | 34 +- drivers/infiniband/hw/mthca/mthca_qp.c | 3 +- drivers/infiniband/sw/rxe/rxe_verbs.c | 5 + include/rdma/ib_verbs.h | 43 +- include/rdma/uverbs_std_types.h | 11 +- include/uapi/rdma/ib_user_verbs.h | 15 + include/uapi/rdma/rdma_netlink.h | 3 + 17 files changed, 740 insertions(+), 113 deletions(-)
Signature · 1 line
2.20.1
PATCH 1/25 RDMA/uverbs: uobj_get_obj_read should return the ib_uobject
From: Shamir Rabinovitch <[email protected]>
The uobject pointer will be removed from ib_x (ib_pd, ib_mr,...) objects. uobj_get_obj_read and uobj_put_obj_read macros were constructed with the assumption that ib_x can figure the uobject in mind.
Fix this wrong assumption.
--- drivers/infiniband/core/uverbs_cmd.c | 125 ++++++++++++++++++++------- include/rdma/uverbs_std_types.h | 8 +- 2 files changed, 98 insertions(+), 35 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +93 -32
@@ -37,6 +37,7 @@#include <linux/fs.h> #include <linux/slab.h> #include <linux/sched.h> +#include <linux/list.h> #include <linux/uaccess.h>
@@ -711,6 +712,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)struct ib_uverbs_reg_mr cmd;struct ib_uverbs_reg_mr_resp resp;struct ib_uobject *uobj; +struct ib_uobject *pduobj;struct ib_pd *pd;struct ib_mr *mr;int ret;
@@ -731,7 +733,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)if (IS_ERR (uobj))return PTR_ERR (uobj); - pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs); + pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs, + &pduobj);if (!pd) { ret = -EINVAL ; goto err_free;
@@ -799,6 +802,7 @@ static int ib_uverbs_rereg_mr(struct uverbs_attr_bundle *attrs)struct ib_pd *old_pd;int ret;struct ib_uobject *uobj; +struct ib_uobject *pduobj =NULL ; ret =uverbs_request (attrs, &cmd,sizeof (cmd));if (ret)
@@ -831,7 +835,7 @@ static int ib_uverbs_rereg_mr(struct uverbs_attr_bundle *attrs)if (cmd.flags &IB_MR_REREG_PD ) { pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, - attrs); + attrs, &pduobj);if (!pd) { ret = -EINVAL ; goto put_uobjs;
@@ -885,6 +889,7 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)struct ib_uverbs_alloc_mw cmd;struct ib_uverbs_alloc_mw_resp resp;struct ib_uobject *uobj; +struct ib_uobject *pduobj;struct ib_pd *pd;struct ib_mw *mw;int ret;
@@ -898,7 +903,8 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)if (IS_ERR (uobj))return PTR_ERR (uobj); - pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs); + pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs, + &pduobj);if (!pd) { ret = -EINVAL ; goto err_free;
@@ -1117,6 +1123,7 @@ static int ib_uverbs_resize_cq(struct uverbs_attr_bundle *attrs) {struct ib_uverbs_resize_cq cmd;struct ib_uverbs_resize_cq_resp resp = {}; +struct ib_uobject *cquobj;struct ib_cq *cq;int ret = -EINVAL ;
@@ -1124,7 +1131,8 @@ static int ib_uverbs_resize_cq(struct uverbs_attr_bundle *attrs)if (ret)return ret; - cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs); + cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs, + &cquobj);if (!cq)return -EINVAL ;
@@ -1177,6 +1185,7 @@ static int ib_uverbs_poll_cq(struct uverbs_attr_bundle *attrs)struct ib_uverbs_poll_cq_resp resp;u8 __user *header_ptr;u8 __user *data_ptr; +struct ib_uobject *cquobj;struct ib_cq *cq;struct ib_wc wc;int ret;
@@ -1185,7 +1194,8 @@ static int ib_uverbs_poll_cq(struct uverbs_attr_bundle *attrs)if (ret)return ret; - cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs); + cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs, + &cquobj);if (!cq)return -EINVAL ;
@@ -1226,6 +1236,7 @@ static int ib_uverbs_poll_cq(struct uverbs_attr_bundle *attrs)static int ib_uverbs_req_notify_cq (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_req_notify_cq cmd; +struct ib_uobject *cquobj;struct ib_cq *cq;int ret;
@@ -1233,7 +1244,8 @@ static int ib_uverbs_req_notify_cq(struct uverbs_attr_bundle *attrs)if (ret)return ret; - cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs); + cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs, + &cquobj);if (!cq)return -EINVAL ;
@@ -1276,16 +1288,21 @@ static int create_qp(struct uverbs_attr_bundle *attrs, {struct ib_uqp_object *obj;struct ib_device *device; +struct ib_uobject *pd_uobj =ERR_PTR (-ENOENT );struct ib_pd *pd =NULL ;struct ib_xrcd *xrcd =NULL ;struct ib_uobject *xrcd_uobj =ERR_PTR (-ENOENT ); +struct ib_uobject *scq_uobj =ERR_PTR (-ENOENT ); +struct ib_uobject *rcq_uobj =ERR_PTR (-ENOENT );struct ib_cq *scq =NULL , *rcq =NULL ; +struct ib_uobject *srq_uobj =ERR_PTR (-ENOENT );struct ib_srq *srq =NULL ;struct ib_qp *qp;struct ib_qp_init_attr attr = {};struct ib_uverbs_ex_create_qp_resp resp;int ret;struct ib_rwq_ind_table *ind_tbl =NULL ; +struct ib_uobject *ind_tbl_uobj =ERR_PTR (-ENOENT );bool has_sq = true;struct ib_device *ib_dev;
@@ -1303,7 +1320,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,if (cmd->comp_mask &IB_UVERBS_CREATE_QP_MASK_IND_TABLE ) { ind_tbl =uobj_get_obj_read (rwq_ind_table,UVERBS_OBJECT_RWQ_IND_TBL , - cmd->rwq_ind_tbl_handle, attrs); + cmd->rwq_ind_tbl_handle, attrs, + &ind_tbl_uobj);if (!ind_tbl) { ret = -EINVAL ; goto err_put;
@@ -1342,7 +1360,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs, }else {if (cmd->is_srq) { srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , - cmd->srq_handle, attrs); + cmd->srq_handle, attrs, + &srq_uobj);if (!srq || srq->srq_type ==IB_SRQT_XRC ) { ret = -EINVAL ; goto err_put;
@@ -1353,7 +1372,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,if (cmd->recv_cq_handle != cmd->send_cq_handle) { rcq =uobj_get_obj_read ( cq,UVERBS_OBJECT_CQ , - cmd->recv_cq_handle, attrs); + cmd->recv_cq_handle, attrs, + &rcq_uobj);if (!rcq) { ret = -EINVAL ; goto err_put;
@@ -1364,11 +1384,12 @@ static int create_qp(struct uverbs_attr_bundle *attrs,if (has_sq) scq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , - cmd->send_cq_handle, attrs); + cmd->send_cq_handle, attrs, + &scq_uobj);if (!ind_tbl) rcq = rcq ?: scq; pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd->pd_handle, - attrs); + attrs, &pd_uobj);if (!pd || (!scq && has_sq)) { ret = -EINVAL ; goto err_put;
@@ -1663,6 +1684,7 @@ static int ib_uverbs_query_qp(struct uverbs_attr_bundle *attrs) {struct ib_uverbs_query_qp cmd;struct ib_uverbs_query_qp_resp resp; +struct ib_uobject *qpuobj;struct ib_qp *qp;struct ib_qp_attr *attr;struct ib_qp_init_attr *init_attr;
@@ -1679,7 +1701,8 @@ static int ib_uverbs_query_qp(struct uverbs_attr_bundle *attrs) goto out; } - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp) { ret = -EINVAL ; goto out;
@@ -1776,6 +1799,7 @@ static int modify_qp(struct uverbs_attr_bundle *attrs, struct ib_uverbs_ex_modify_qp *cmd) {struct ib_qp_attr *attr; +struct ib_uobject *qpuobj;struct ib_qp *qp;int ret;
@@ -1784,7 +1808,7 @@ static int modify_qp(struct uverbs_attr_bundle *attrs,return -ENOMEM ; qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd->base.qp_handle, - attrs); + attrs, &qpuobj);if (!qp) { ret = -EINVAL ; goto out;
@@ -2018,6 +2042,7 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs)struct ib_uverbs_send_wr *user_wr;struct ib_send_wr *wr =NULL , *last, *next;const struct ib_send_wr *bad_wr; +struct ib_uobject *qpuobj;struct ib_qp *qp;int i, sg_ind;int is_ud;
@@ -2045,7 +2070,8 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs)if (!user_wr)return -ENOMEM ; - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp) { ret = -EINVAL ; goto out;
@@ -2068,6 +2094,7 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs)if (is_ud) {struct ib_ud_wr *ud; +struct ib_uobject *uobj;if (user_wr->opcode !=IB_WR_SEND && user_wr->opcode !=IB_WR_SEND_WITH_IMM ) {
@@ -2083,7 +2110,8 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs) } ud->ah =uobj_get_obj_read (ah,UVERBS_OBJECT_AH , - user_wr->wr.ud.ah, attrs); + user_wr->wr.ud.ah, attrs, + &uobj);if (!ud->ah) {kfree (ud); ret = -EINVAL ;
@@ -2308,6 +2336,7 @@ static int ib_uverbs_post_recv(struct uverbs_attr_bundle *attrs)struct ib_uverbs_post_recv_resp resp;struct ib_recv_wr *wr, *next;const struct ib_recv_wr *bad_wr; +struct ib_uobject *qpuobj;struct ib_qp *qp;int ret, ret2;struct uverbs_req_iter iter;
@@ -2321,7 +2350,8 @@ static int ib_uverbs_post_recv(struct uverbs_attr_bundle *attrs)if (IS_ERR (wr))return PTR_ERR (wr); - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp) { ret = -EINVAL ; goto out;
@@ -2358,6 +2388,7 @@ static int ib_uverbs_post_srq_recv(struct uverbs_attr_bundle *attrs)struct ib_uverbs_post_srq_recv_resp resp;struct ib_recv_wr *wr, *next;const struct ib_recv_wr *bad_wr; +struct ib_uobject *srquobj;struct ib_srq *srq;int ret, ret2;struct uverbs_req_iter iter;
@@ -2371,7 +2402,8 @@ static int ib_uverbs_post_srq_recv(struct uverbs_attr_bundle *attrs)if (IS_ERR (wr))return PTR_ERR (wr); - srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs); + srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs, + &srquobj);if (!srq) { ret = -EINVAL ; goto out;
@@ -2408,6 +2440,7 @@ static int ib_uverbs_create_ah(struct uverbs_attr_bundle *attrs)struct ib_uverbs_create_ah cmd;struct ib_uverbs_create_ah_resp resp;struct ib_uobject *uobj; +struct ib_uobject *pduobj;struct ib_pd *pd;struct ib_ah *ah;struct rdma_ah_attr attr = {};
@@ -2427,7 +2460,8 @@ static int ib_uverbs_create_ah(struct uverbs_attr_bundle *attrs) goto err; } - pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs); + pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs, + &pduobj);if (!pd) { ret = -EINVAL ; goto err;
@@ -2497,6 +2531,7 @@ static int ib_uverbs_destroy_ah(struct uverbs_attr_bundle *attrs)static int ib_uverbs_attach_mcast (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_attach_mcast cmd; +struct ib_uobject *qpuobj;struct ib_qp *qp;struct ib_uqp_object *obj;struct ib_uverbs_mcast_entry *mcast;
@@ -2506,7 +2541,8 @@ static int ib_uverbs_attach_mcast(struct uverbs_attr_bundle *attrs)if (ret)return ret; - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp)return -EINVAL ;
@@ -2546,6 +2582,7 @@ static int ib_uverbs_detach_mcast(struct uverbs_attr_bundle *attrs) {struct ib_uverbs_detach_mcast cmd;struct ib_uqp_object *obj; +struct ib_uobject *qpuobj;struct ib_qp *qp;struct ib_uverbs_mcast_entry *mcast;int ret;
@@ -2555,7 +2592,8 @@ static int ib_uverbs_detach_mcast(struct uverbs_attr_bundle *attrs)if (ret)return ret; - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp)return -EINVAL ;
@@ -2665,6 +2703,8 @@ static int kern_spec_to_ib_spec_action(struct uverbs_attr_bundle *attrs,union ib_flow_spec *ib_spec, struct ib_uflow_resources *uflow_res) { + struct ib_uobject *uobj; + ib_spec->type = kern_spec->type;switch (ib_spec->type) {case IB_FLOW_SPEC_ACTION_TAG :
@@ -2689,7 +2729,7 @@ static int kern_spec_to_ib_spec_action(struct uverbs_attr_bundle *attrs, ib_spec->action.act =uobj_get_obj_read (flow_action,UVERBS_OBJECT_FLOW_ACTION , kern_spec->action.handle, - attrs); + attrs, &uobj);if (!ib_spec->action.act)return -EINVAL ; ib_spec->action.size =
@@ -2707,7 +2747,7 @@ static int kern_spec_to_ib_spec_action(struct uverbs_attr_bundle *attrs,uobj_get_obj_read (counters,UVERBS_OBJECT_COUNTERS , kern_spec->flow_count.handle, - attrs); + attrs, &uobj);if (!ib_spec->flow_count.counters)return -EINVAL ; ib_spec->flow_count.size =
@@ -2909,6 +2949,8 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)struct ib_uverbs_ex_create_wq_resp resp = {};struct ib_uwq_object *obj;int err =0 ; +struct ib_uobject *cquobj; +struct ib_uobject *pduobj;struct ib_cq *cq;struct ib_pd *pd;struct ib_wq *wq;
@@ -2927,13 +2969,15 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)if (IS_ERR (obj))return PTR_ERR (obj); - pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs); + pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd.pd_handle, attrs, + &pduobj);if (!pd) { err = -EINVAL ; goto err_uobj; } - cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs); + cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs, + &cquobj);if (!cq) { err = -EINVAL ; goto err_put_pd;
@@ -3025,6 +3069,7 @@ static int ib_uverbs_ex_destroy_wq(struct uverbs_attr_bundle *attrs)static int ib_uverbs_ex_modify_wq (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_ex_modify_wq cmd; +struct ib_uobject *wquobj;struct ib_wq *wq;struct ib_wq_attr wq_attr = {};int ret;
@@ -3039,7 +3084,8 @@ static int ib_uverbs_ex_modify_wq(struct uverbs_attr_bundle *attrs)if (cmd.attr_mask > (IB_WQ_STATE |IB_WQ_CUR_STATE |IB_WQ_FLAGS ))return -EINVAL ; - wq =uobj_get_obj_read (wq,UVERBS_OBJECT_WQ , cmd.wq_handle, attrs); + wq =uobj_get_obj_read (wq,UVERBS_OBJECT_WQ , cmd.wq_handle, attrs, + &wquobj);if (!wq)return -EINVAL ;
@@ -3104,8 +3150,11 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)for (num_read_wqs =0 ; num_read_wqs < num_wq_handles; num_read_wqs++) { +struct ib_uobject *uobj; + wq =uobj_get_obj_read (wq,UVERBS_OBJECT_WQ , - wqs_handles[num_read_wqs], attrs); + wqs_handles[num_read_wqs], attrs, + &uobj);if (!wq) { err = -EINVAL ; goto put_wqs;
@@ -3166,6 +3215,7 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs) err_free:kfree (wqs_handles);kfree (wqs); +return err; }
@@ -3193,6 +3243,7 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)struct ib_flow *flow_id;struct ib_uverbs_flow_attr *kern_flow_attr;struct ib_flow_attr *flow_attr; +struct ib_uobject *qpuobj;struct ib_qp *qp;struct ib_uflow_resources *uflow_res;struct ib_uverbs_flow_spec_hdr *kern_spec;
@@ -3256,7 +3307,8 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs) goto err_free_attr; } - qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs); + qp =uobj_get_obj_read (qp,UVERBS_OBJECT_QP , cmd.qp_handle, attrs, + &qpuobj);if (!qp) { err = -EINVAL ; goto err_uobj;
@@ -3371,6 +3423,8 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs, {struct ib_uverbs_create_srq_resp resp;struct ib_usrq_object *obj; +struct ib_uobject *cquobj =ERR_PTR (-ENOENT ); +struct ib_uobject *pduobj;struct ib_pd *pd;struct ib_srq *srq;struct ib_uobject *uninitialized_var (xrcd_uobj );
@@ -3406,14 +3460,15 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,if (ib_srq_has_cq (cmd->srq_type)) { attr.ext.cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , - cmd->cq_handle, attrs); + cmd->cq_handle, attrs, &cquobj);if (!attr.ext.cq) { ret = -EINVAL ; goto err_put_xrcd; } } - pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd->pd_handle, attrs); + pd =uobj_get_obj_read (pd,UVERBS_OBJECT_PD , cmd->pd_handle, attrs, + &pduobj);if (!pd) { ret = -EINVAL ; goto err_put_cq;
@@ -3542,6 +3597,7 @@ static int ib_uverbs_create_xsrq(struct uverbs_attr_bundle *attrs)static int ib_uverbs_modify_srq (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_modify_srq cmd; +struct ib_uobject *srquobj;struct ib_srq *srq;struct ib_srq_attr attr;int ret;
@@ -3550,7 +3606,8 @@ static int ib_uverbs_modify_srq(struct uverbs_attr_bundle *attrs)if (ret)return ret; - srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs); + srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs, + &srquobj);if (!srq)return -EINVAL ;
@@ -3570,6 +3627,7 @@ static int ib_uverbs_query_srq(struct uverbs_attr_bundle *attrs)struct ib_uverbs_query_srq cmd;struct ib_uverbs_query_srq_resp resp;struct ib_srq_attr attr; +struct ib_uobject *srquobj;struct ib_srq *srq;int ret;
@@ -3577,7 +3635,8 @@ static int ib_uverbs_query_srq(struct uverbs_attr_bundle *attrs)if (ret)return ret; - srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs); + srq =uobj_get_obj_read (srq,UVERBS_OBJECT_SRQ , cmd.srq_handle, attrs, + &srquobj);if (!srq)return -EINVAL ;
@@ -3689,6 +3748,7 @@ static int ib_uverbs_ex_query_device(struct uverbs_attr_bundle *attrs)static int ib_uverbs_ex_modify_cq (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_ex_modify_cq cmd; +struct ib_uobject *cquobj;struct ib_cq *cq;int ret;
@@ -3702,7 +3762,8 @@ static int ib_uverbs_ex_modify_cq(struct uverbs_attr_bundle *attrs)if (cmd.attr_mask >IB_CQ_MODERATE )return -EOPNOTSUPP ; - cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs); + cq =uobj_get_obj_read (cq,UVERBS_OBJECT_CQ , cmd.cq_handle, attrs, + &cquobj);if (!cq)return -EINVAL ;
include/rdma/uverbs_std_types.h +5 -3
@@ -64,9 +64,11 @@ static inline void *_uobj_get_obj_read(struct ib_uobject *uobj)return NULL ;return uobj->object; } -#define uobj_get_obj_read (_object, _type, _id, _attrs) \ - ((struct ib_##_object *)_uobj_get_obj_read( \ - uobj_get_read(_type, _id, _attrs))) +#define uobj_get_obj_read (_object, _type, _id, _attrs, _uobj) \ +({ \ + *_uobj = uobj_get_read(_type, _id, _attrs); \ + ((struct ib_##_object *)_uobj_get_obj_read(*_uobj)); \ +})#define uobj_get_write (_type, _id, _attrs) \ rdma_lookup_get_uobject(uobj_get_type(_attrs, _type), (_attrs)->ufile, \
Signature · 1 line
2.20.1
PATCH 2/25 RDMA/uverbs: Delete the macro uobj_put_obj_read
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
The macro uobj_put_obj_read assumes an existence of a member with the name uobject on each HW object. Since this member is about to be delete we shouldn't use this macro, instead use uobj_put_read and delete uobj_put_obj_read.
--- drivers/infiniband/core/uverbs_cmd.c | 92 ++++++++++++++-------------- include/rdma/uverbs_std_types.h | 3 - 2 files changed, 46 insertions(+), 49 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +46 -46
@@ -778,7 +778,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)if (ret) goto err_copy; -uobj_put_obj_read (pd); +uobj_put_read (pduobj);return uobj_alloc_commit (uobj, attrs);
@@ -786,7 +786,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)ib_dereg_mr_user (mr,uverbs_get_cleared_udata (attrs)); err_put: -uobj_put_obj_read (pd); +uobj_put_read (pduobj); err_free:uobj_alloc_abort (uobj, attrs);
@@ -864,7 +864,7 @@ static int ib_uverbs_rereg_mr(struct uverbs_attr_bundle *attrs) put_uobj_pd:if (cmd.flags &IB_MR_REREG_PD ) -uobj_put_obj_read (pd); +uobj_put_read (pduobj); put_uobjs:uobj_put_write (uobj);
@@ -936,13 +936,13 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)if (ret) goto err_copy; -uobj_put_obj_read (pd); +uobj_put_read (pduobj);return uobj_alloc_commit (uobj, attrs); err_copy:uverbs_dealloc_mw (mw); err_put: -uobj_put_obj_read (pd); +uobj_put_read (pduobj); err_free:uobj_alloc_abort (uobj, attrs);return ret;
@@ -1144,7 +1144,7 @@ static int ib_uverbs_resize_cq(struct uverbs_attr_bundle *attrs) ret =uverbs_response (attrs, &resp,sizeof (resp)); out: -uobj_put_obj_read (cq); +uobj_put_read (cquobj);return ret; }
@@ -1229,7 +1229,7 @@ static int ib_uverbs_poll_cq(struct uverbs_attr_bundle *attrs) ret =uverbs_output_written (attrs,UVERBS_ATTR_CORE_OUT ); out_put: -uobj_put_obj_read (cq); +uobj_put_read (cquobj);return ret; }
@@ -1252,7 +1252,7 @@ static int ib_uverbs_req_notify_cq(struct uverbs_attr_bundle *attrs)ib_req_notify_cq (cq, cmd.solicited_only ?IB_CQ_SOLICITED :IB_CQ_NEXT_COMP ); -uobj_put_obj_read (cq); +uobj_put_read (cquobj);return 0 ; }
@@ -1505,15 +1505,15 @@ static int create_qp(struct uverbs_attr_bundle *attrs, }if (pd) -uobj_put_obj_read (pd); +uobj_put_read (pd_uobj);if (scq) -uobj_put_obj_read (scq); +uobj_put_read (scq_uobj);if (rcq && rcq != scq) -uobj_put_obj_read (rcq); +uobj_put_read (rcq_uobj);if (srq) -uobj_put_obj_read (srq); +uobj_put_read (srq_uobj);if (ind_tbl) -uobj_put_obj_read (ind_tbl); +uobj_put_read (ind_tbl_uobj);return uobj_alloc_commit (&obj->uevent.uobject, attrs); err_cb:
@@ -1523,15 +1523,15 @@ static int create_qp(struct uverbs_attr_bundle *attrs,if (!IS_ERR (xrcd_uobj))uobj_put_read (xrcd_uobj);if (pd) -uobj_put_obj_read (pd); +uobj_put_read (pd_uobj);if (scq) -uobj_put_obj_read (scq); +uobj_put_read (scq_uobj);if (rcq && rcq != scq) -uobj_put_obj_read (rcq); +uobj_put_read (rcq_uobj);if (srq) -uobj_put_obj_read (srq); +uobj_put_read (srq_uobj);if (ind_tbl) -uobj_put_obj_read (ind_tbl); +uobj_put_read (ind_tbl_uobj);uobj_alloc_abort (&obj->uevent.uobject, attrs);return ret;
@@ -1710,7 +1710,7 @@ static int ib_uverbs_query_qp(struct uverbs_attr_bundle *attrs) ret =ib_query_qp (qp, attr, cmd.attr_mask, init_attr); -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);if (ret) goto out;
@@ -1948,7 +1948,7 @@ static int modify_qp(struct uverbs_attr_bundle *attrs, &attrs->driver_udata); release_qp: -uobj_put_obj_read (qp); +uobj_put_read (qpuobj); out:kfree (attr);
@@ -2216,11 +2216,11 @@ static int ib_uverbs_post_send(struct uverbs_attr_bundle *attrs) ret = ret2; out_put: -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);while (wr) {if (is_ud &&ud_wr (wr)->ah) -uobj_put_obj_read (ud_wr (wr)->ah); +uobj_put_read (ud_wr (wr)->ah->uobject); next = wr->next;kfree (wr); wr = next;
@@ -2360,7 +2360,7 @@ static int ib_uverbs_post_recv(struct uverbs_attr_bundle *attrs) resp.bad_wr =0 ; ret = qp->device->ops.post_recv (qp->real_qp, wr, &bad_wr); -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);if (ret) {for (next = wr; next; next = next->next) { ++resp.bad_wr;
@@ -2412,7 +2412,7 @@ static int ib_uverbs_post_srq_recv(struct uverbs_attr_bundle *attrs) resp.bad_wr =0 ; ret = srq->device->ops.post_srq_recv (srq, wr, &bad_wr); -uobj_put_obj_read (srq); +uobj_put_read (srquobj);if (ret)for (next = wr; next; next = next->next) {
@@ -2501,7 +2501,7 @@ static int ib_uverbs_create_ah(struct uverbs_attr_bundle *attrs)if (ret) goto err_copy; -uobj_put_obj_read (pd); +uobj_put_read (pduobj);return uobj_alloc_commit (uobj, attrs); err_copy:
@@ -2509,7 +2509,7 @@ static int ib_uverbs_create_ah(struct uverbs_attr_bundle *attrs) uverbs_get_cleared_udata(attrs)); err_put: -uobj_put_obj_read (pd); +uobj_put_read (pduobj); err:uobj_alloc_abort (uobj, attrs);
@@ -2573,7 +2573,7 @@ static int ib_uverbs_attach_mcast(struct uverbs_attr_bundle *attrs) out_put:mutex_unlock (&obj->mcast_lock); -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);return ret; }
@@ -2618,7 +2618,7 @@ static int ib_uverbs_detach_mcast(struct uverbs_attr_bundle *attrs) out_put:mutex_unlock (&obj->mcast_lock); -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);return ret; }
@@ -2737,7 +2737,7 @@ static int kern_spec_to_ib_spec_action(struct uverbs_attr_bundle *attrs,flow_resources_add (uflow_res,IB_FLOW_SPEC_ACTION_HANDLE , ib_spec->action.act); -uobj_put_obj_read (ib_spec->action.act); +uobj_put_read (uobj);break ;case IB_FLOW_SPEC_ACTION_COUNT : if (kern_spec->flow_count.size !=
@@ -2755,7 +2755,7 @@ static int kern_spec_to_ib_spec_action(struct uverbs_attr_bundle *attrs,flow_resources_add (uflow_res,IB_FLOW_SPEC_ACTION_COUNT , ib_spec->flow_count.counters); -uobj_put_obj_read (ib_spec->flow_count.counters); +uobj_put_read (uobj);break ;default :return -EINVAL ;
@@ -3022,16 +3022,16 @@ static int ib_uverbs_ex_create_wq(struct uverbs_attr_bundle *attrs)if (err) goto err_copy; -uobj_put_obj_read (pd); -uobj_put_obj_read (cq); +uobj_put_read (pduobj); +uobj_put_read (cquobj);return uobj_alloc_commit (&obj->uevent.uobject, attrs); err_copy:ib_destroy_wq (wq,uverbs_get_cleared_udata (attrs)); err_put_cq: -uobj_put_obj_read (cq); +uobj_put_read (cquobj); err_put_pd: -uobj_put_obj_read (pd); +uobj_put_read (pduobj); err_uobj:uobj_alloc_abort (&obj->uevent.uobject, attrs);
@@ -3097,7 +3097,7 @@ static int ib_uverbs_ex_modify_wq(struct uverbs_attr_bundle *attrs) } ret = wq->device->ops.modify_wq (wq, &wq_attr, cmd.attr_mask, &attrs->driver_udata); -uobj_put_obj_read (wq); +uobj_put_read (wquobj);return ret; }
@@ -3201,7 +3201,7 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)kfree (wqs_handles);for (j =0 ; j < num_read_wqs; j++) -uobj_put_obj_read (wqs[j]); +uobj_put_read (wqs[j]->uobject);return uobj_alloc_commit (uobj, attrs);
@@ -3211,7 +3211,7 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)uobj_alloc_abort (uobj, attrs); put_wqs:for (j =0 ; j < num_read_wqs; j++) -uobj_put_obj_read (wqs[j]); +uobj_put_read (wqs[j]->uobject); err_free:kfree (wqs_handles);kfree (wqs);
@@ -3380,7 +3380,7 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)if (err) goto err_copy; -uobj_put_obj_read (qp); +uobj_put_read (qpuobj);kfree (flow_attr);if (cmd.flow_attr.num_of_specs)kfree (kern_flow_attr);
@@ -3393,7 +3393,7 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs) err_free_flow_attr:kfree (flow_attr); err_put: -uobj_put_obj_read (qp); +uobj_put_read (qpuobj); err_uobj:uobj_alloc_abort (uobj, attrs); err_free_attr:
@@ -3532,9 +3532,9 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs,uobj_put_read (xrcd_uobj);if (ib_srq_has_cq (cmd->srq_type)) -uobj_put_obj_read (attr.ext.cq); +uobj_put_read (cquobj); -uobj_put_obj_read (pd); +uobj_put_read (pduobj);return uobj_alloc_commit (&obj->uevent.uobject, attrs); err_copy:
@@ -3543,11 +3543,11 @@ static int __uverbs_create_xsrq(struct uverbs_attr_bundle *attrs, err_free:kfree (srq); err_put: -uobj_put_obj_read (pd); +uobj_put_read (pduobj); err_put_cq:if (ib_srq_has_cq (cmd->srq_type)) -uobj_put_obj_read (attr.ext.cq); +uobj_put_read (cquobj); err_put_xrcd:if (cmd->srq_type ==IB_SRQT_XRC ) {
@@ -3617,7 +3617,7 @@ static int ib_uverbs_modify_srq(struct uverbs_attr_bundle *attrs) ret = srq->device->ops.modify_srq (srq, &attr, cmd.attr_mask, &attrs->driver_udata); -uobj_put_obj_read (srq); +uobj_put_read (srquobj);return ret; }
@@ -3642,7 +3642,7 @@ static int ib_uverbs_query_srq(struct uverbs_attr_bundle *attrs) ret =ib_query_srq (srq, &attr); -uobj_put_obj_read (srq); +uobj_put_read (srquobj);if (ret)return ret;
@@ -3769,7 +3769,7 @@ static int ib_uverbs_ex_modify_cq(struct uverbs_attr_bundle *attrs) ret =rdma_set_cq_moderation (cq, cmd.attr.cq_count, cmd.attr.cq_period); -uobj_put_obj_read (cq); +uobj_put_read (cquobj);return ret; }
include/rdma/uverbs_std_types.h +0 -3
@@ -98,9 +98,6 @@ static inline void uobj_put_read(struct ib_uobject *uobj)rdma_lookup_put_uobject (uobj,UVERBS_LOOKUP_READ ); } -#define uobj_put_obj_read (_obj) \ - uobj_put_read((_obj)->uobject) -static inline void uobj_put_write (struct ib_uobject *uobj) {rdma_lookup_put_uobject (uobj,UVERBS_LOOKUP_WRITE );
Signature · 1 line
2.20.1
PATCH 3/25 RDMA/nldev: ib_pd can be pointed by multiple ib_ucontext
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
In shared object model ib_pd can belong to 1 or more ib_ucontext. Fix the nldev code so it could report multiple context ids.
--- drivers/infiniband/core/nldev.c | 127 +++++++++++++++++++++++++++++-- include/uapi/rdma/rdma_netlink.h | 3 + 2 files changed, 125 insertions(+), 5 deletions(-)
drivers/infiniband/core/nldev.c +122 -5
@@ -41,6 +41,7 @@#include "core_priv.h" #include "cma_priv.h" #include "restrack.h" +#include "uverbs.h" /* * Sort array elements by the netlink attribute name
@@ -141,6 +142,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID] = { .type = NLA_U32 }, [RDMA_NLDEV_NET_NS_FD] = { .type = NLA_U32 }, [RDMA_NLDEV_SYS_ATTR_NETNS_MODE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_CTX] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_CTX_ENTRY] = { .type = NLA_NESTED }, }; static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
@@ -611,11 +614,84 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin, err:return -EMSGSIZE ; } +struct context_id { +struct list_head list; +u32 id; +}; + +static void pd_context (struct ib_pd *pd,struct list_head *list,int *count) +{ +struct ib_device *device = pd->device; +struct rdma_restrack_entry *res; +struct rdma_restrack_root *rt; +struct ib_uverbs_file *ufile; +struct ib_ucontext *ucontext; +struct ib_uobject *uobj; +unsigned long flags; +unsigned long id; +bool found; + + rt = &device->res[RDMA_RESTRACK_CTX ]; + +xa_lock (&rt->xa); + +xa_for_each (&rt->xa, id, res) { +if (!rdma_is_visible_in_pid_ns (res)) +continue ; + +if (!rdma_restrack_get (res)) +continue ; + +xa_unlock (&rt->xa); + + ucontext = container_of(res, struct ib_ucontext, res); + ufile = ucontext->ufile; + found = false; + +/* See locking requirements in struct ib_uverbs_file */ +down_read (&ufile->hw_destroy_rwsem); +spin_lock_irqsave (&ufile->uobjects_lock, flags); + +list_for_each_entry (uobj, &ufile->uobjects, list) { +if (uobj->object == pd) { + found = true; + goto found; + } + } + +found:spin_unlock_irqrestore (&ufile->uobjects_lock, flags); +up_read (&ufile->hw_destroy_rwsem); + +if (found) { +struct context_id *ctx_id = +kmalloc (sizeof (*ctx_id),GFP_KERNEL ); + +if (WARN_ON_ONCE (!ctx_id)) + goto next; + + ctx_id->id = ucontext->res.id; +list_add (&ctx_id->list, list); + (*count)++; + } + +next:rdma_restrack_put (res); +xa_lock (&rt->xa); + } + +xa_unlock (&rt->xa); +} +static int fill_res_pd_entry (struct sk_buff *msg,bool has_cap_net_admin,struct rdma_restrack_entry *res,uint32_t port) { struct ib_pd *pd = container_of(res, struct ib_pd, res);struct ib_device *dev = pd->device; +struct nlattr *table_attr =NULL ; +struct nlattr *entry_attr =NULL ; +struct context_id *ctx_id; +struct context_id *tmp; +LIST_HEAD (pd_context_ids); +int ctx_count =0 ;if (has_cap_net_admin) { if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,
@@ -633,10 +709,38 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_PDN , res->id)) goto err; -if (!rdma_is_kernel_res (res) && -nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , - pd->uobject->context->res.id)) - goto err; +if (!rdma_is_kernel_res (res)) { +pd_context (pd, &pd_context_ids, &ctx_count); +if (ctx_count ==1 ) { +/* user pd, not shared */ + ctx_id =list_first_entry (&pd_context_ids, + struct context_id, list); +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , + ctx_id->id)) + goto err; + }else if (ctx_count >1 ) { +/* user pd, shared */ + table_attr =nla_nest_start (msg, +RDMA_NLDEV_ATTR_RES_CTX ); +if (!table_attr) + goto err; + +list_for_each_entry (ctx_id, &pd_context_ids, list) { + entry_attr =nla_nest_start (msg, +RDMA_NLDEV_ATTR_RES_CTX_ENTRY ); +if (!entry_attr) + goto err; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , + ctx_id->id)) + goto err; +nla_nest_end (msg, entry_attr); + entry_attr =NULL ; + } + +nla_nest_end (msg, table_attr); + table_attr =NULL ; + } + }if (fill_res_name_pid (msg, res)) goto err;
@@ -644,9 +748,22 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,if (fill_res_entry (dev, msg, res)) goto err; +list_for_each_entry_safe (ctx_id, tmp, &pd_context_ids, list) +kfree (ctx_id); +return 0 ; -err:return -EMSGSIZE ; +err: +if (entry_attr) +nla_nest_end (msg, entry_attr); + +if (table_attr) +nla_nest_end (msg, table_attr); + +list_for_each_entry_safe (ctx_id, tmp, &pd_context_ids, list) +kfree (ctx_id); + +return -EMSGSIZE ; } static int fill_stat_counter_mode(struct sk_buff *msg,
include/uapi/rdma/rdma_netlink.h +3 -0
@@ -525,6 +525,9 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_DEV_DIM ,/* u8 */ +RDMA_NLDEV_ATTR_RES_CTX ,/* nested table */ + RDMA_NLDEV_ATTR_RES_CTX_ENTRY, /* nested table */ +/* * Always the end */
Signature · 1 line
2.20.1
Re: [PATCH 03/25] RDMA/nldev: ib_pd can be pointed by multiple ib_ucontext
Quoted 81 lines · Shamir Rabinovitch
From: Shamir Rabinovitch <[email protected]>
In shared object model
ib_pdcan belong to 1 or moreib_ucontext. Fix the nldev code so it could report multiple context ids.Signed-off-by Shamir Rabinovitch <[email protected]>Signed-off-by Shamir Rabinovitch <[email protected]>--- drivers/infiniband/core/nldev.c | 127 +++++++++++++++++++++++++++++-- include/uapi/rdma/rdma_netlink.h | 3 + 2 files changed, 125 insertions(+), 5 deletions(-)drivers/infiniband/core/nldev.c +47
-0@@ -41,6 +41,7 @@#include "core_priv.h" #include "cma_priv.h" #include "restrack.h" +#include "uverbs.h" /* * Sort array elements by the netlink attribute name @@ -141,6 +142,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID] = { .type = NLA_U32 }, [RDMA_NLDEV_NET_NS_FD] = { .type = NLA_U32 }, [RDMA_NLDEV_SYS_ATTR_NETNS_MODE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_CTX] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_CTX_ENTRY] = { .type = NLA_NESTED }, }; static int put_driver_name_print_type(struct sk_buff *msg, const char *name,@@ -611,11 +614,84 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin, err:return -EMSGSIZE ; } +struct context_id { +struct list_head list; +u32 id; +}; + +static void pd_context (struct ib_pd *pd,struct list_head *list,int *count) +{ +struct ib_device *device = pd->device; +struct rdma_restrack_entry *res; +struct rdma_restrack_root *rt; +struct ib_uverbs_file *ufile; +struct ib_ucontext *ucontext; +struct ib_uobject *uobj; +unsigned long flags; +unsigned long id; +bool found; + + rt = &device->res[RDMA_RESTRACK_CTX ]; + +xa_lock (&rt->xa); + +xa_for_each (&rt->xa, id, res) { +if (!rdma_is_visible_in_pid_ns (res)) +continue ; + +if (!rdma_restrack_get (res)) +continue ; + +xa_unlock (&rt->xa); + + ucontext = container_of(res, struct ib_ucontext, res); + ufile = ucontext->ufile; + found = false; + +/* See locking requirements in struct ib_uverbs_file */ +down_read (&ufile->hw_destroy_rwsem); +spin_lock_irqsave (&ufile->uobjects_lock, flags); + +list_for_each_entry (uobj, &ufile->uobjects, list) { +if (uobj->object == pd) { + found = true; + goto found; + } + }
drivers/infiniband/core/nldev.c static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin, · line 670
Quoted 10 lines
drivers/infiniband/core/nldev.c +10
-0+ +found:spin_unlock_irqrestore (&ufile->uobjects_lock, flags); +up_read (&ufile->hw_destroy_rwsem); + +if (found) { +struct context_id *ctx_id = +kmalloc (sizeof (*ctx_id),GFP_KERNEL ); + +if (WARN_ON_ONCE (!ctx_id)) + goto next;
No WARN_ON on kmalloc failure.
Quoted 28 lines
drivers/infiniband/core/nldev.c +19
-0+ + ctx_id->id = ucontext->res.id; +list_add (&ctx_id->list, list); + (*count)++; + } + +next:rdma_restrack_put (res); +xa_lock (&rt->xa); + } + +xa_unlock (&rt->xa); +} +static int fill_res_pd_entry (struct sk_buff *msg,bool has_cap_net_admin,struct rdma_restrack_entry *res,uint32_t port) { struct ib_pd *pd = container_of(res, struct ib_pd, res);struct ib_device *dev = pd->device; +struct nlattr *table_attr =NULL ; +struct nlattr *entry_attr =NULL ; +struct context_id *ctx_id; +struct context_id *tmp; +LIST_HEAD (pd_context_ids); +int ctx_count =0 ;if (has_cap_net_admin) { if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,
drivers/infiniband/core/nldev.c static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin, · line 714
Quoted 10 lines
drivers/infiniband/core/nldev.c +3
-4@@ -633,10 +709,38 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_PDN , res->id)) goto err; -if (!rdma_is_kernel_res (res) && -nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , - pd->uobject->context->res.id)) - goto err; +if (!rdma_is_kernel_res (res)) { +pd_context (pd, &pd_context_ids, &ctx_count); +if (ctx_count ==1 ) {
Only suggestion, maybe the better approach will be to print first CTXN always and other CTXNs as nested after that? It will give an ability to see at least first PD with old rdmatool. In your case, old rdmatool won't print CTXNs at all.
Thanks
Quoted 73 lines
drivers/infiniband/core/nldev.c +43
-1+/* user pd, not shared */ + ctx_id =list_first_entry (&pd_context_ids, + struct context_id, list); +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , + ctx_id->id)) + goto err; + }else if (ctx_count >1 ) { +/* user pd, shared */ + table_attr =nla_nest_start (msg, +RDMA_NLDEV_ATTR_RES_CTX ); +if (!table_attr) + goto err; + +list_for_each_entry (ctx_id, &pd_context_ids, list) { + entry_attr =nla_nest_start (msg, +RDMA_NLDEV_ATTR_RES_CTX_ENTRY ); +if (!entry_attr) + goto err; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_CTXN , + ctx_id->id)) + goto err; +nla_nest_end (msg, entry_attr); + entry_attr =NULL ; + } + +nla_nest_end (msg, table_attr); + table_attr =NULL ; + } + }if (fill_res_name_pid (msg, res)) goto err;@@ -644,9 +748,22 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,if (fill_res_entry (dev, msg, res)) goto err; +list_for_each_entry_safe (ctx_id, tmp, &pd_context_ids, list) +kfree (ctx_id); +return 0 ; -err:return -EMSGSIZE ; +err: +if (entry_attr) +nla_nest_end (msg, entry_attr); + +if (table_attr) +nla_nest_end (msg, table_attr); + +list_for_each_entry_safe (ctx_id, tmp, &pd_context_ids, list) +kfree (ctx_id); + +return -EMSGSIZE ; } static int fill_stat_counter_mode(struct sk_buff *msg,include/uapi/rdma/rdma_netlink.h +3
-1@@ -525,6 +525,9 @@ enum rdma_nldev_attr {*/ RDMA_NLDEV_ATTR_DEV_DIM ,/* u8 */ +RDMA_NLDEV_ATTR_RES_CTX ,/* nested table */ + RDMA_NLDEV_ATTR_RES_CTX_ENTRY, /* nested table */ +/* * Always the end */ --2.20.1
PATCH 4/25 IB/{core,hw}: ib_pd should not have ib_uobject pointer
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
As a preparation step to shared PD, where ib_pd object will be pointed by one or more ib_uobjects, remove ib_uobject pointer from ib_pd struct.
--- drivers/infiniband/core/uverbs_cmd.c | 1 - drivers/infiniband/core/verbs.c | 1 - drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 1 - drivers/infiniband/hw/mlx5/main.c | 1 - drivers/infiniband/hw/mthca/mthca_qp.c | 3 ++- include/rdma/ib_verbs.h | 1 - 6 files changed, 2 insertions(+), 6 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +0 -1
@@ -432,7 +432,6 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs) } pd->device = ib_dev; - pd->uobject = uobj; pd->__internal_mr =NULL ;atomic_set (&pd->usecnt,0 ); pd->res.type =RDMA_RESTRACK_PD ;
drivers/infiniband/core/verbs.c +0 -1
@@ -263,7 +263,6 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,return ERR_PTR (-ENOMEM ); pd->device = device; - pd->uobject =NULL ; pd->__internal_mr =NULL ;atomic_set (&pd->usecnt,0 ); pd->flags = flags;
drivers/infiniband/hw/hns/hns_roce_hw_v1.c +0 -1
@@ -760,7 +760,6 @@ static int hns_roce_v1_rsv_lp_qp(struct hns_roce_dev *hr_dev) free_mr->mr_free_pd =to_hr_pd (pd); free_mr->mr_free_pd->ibpd.device = &hr_dev->ib_dev; - free_mr->mr_free_pd->ibpd.uobject =NULL ; free_mr->mr_free_pd->ibpd.__internal_mr =NULL ;atomic_set (&free_mr->mr_free_pd->ibpd.usecnt,0 );
drivers/infiniband/hw/mlx5/main.c +0 -1
@@ -4937,7 +4937,6 @@ static int create_dev_resources(struct mlx5_ib_resources *devr)return -ENOMEM ; devr->p0->device = ibdev; - devr->p0->uobject =NULL ;atomic_set (&devr->p0->usecnt,0 ); ret =mlx5_ib_alloc_pd (devr->p0,NULL );
drivers/infiniband/hw/mthca/mthca_qp.c +2 -1
@@ -956,7 +956,8 @@ static int mthca_max_data_size(struct mthca_dev *dev, struct mthca_qp *qp, int dstatic inline int mthca_max_inline_data (struct mthca_pd *pd,int max_data_size) {/* We don't support inline data for kernel QPs (yet). */ -return pd->ibpd.uobject ? max_data_size -MTHCA_INLINE_HEADER_SIZE :0 ; +return !rdma_is_kernel_res (&pd->ibpd.res) ? + max_data_size -MTHCA_INLINE_HEADER_SIZE :0 ; } static void mthca_adjust_qp_caps(struct mthca_dev *dev,
include/rdma/ib_verbs.h +0 -1
@@ -1457,7 +1457,6 @@ struct ib_pd {u32 local_dma_lkey;u32 flags;struct ib_device *device; -struct ib_uobject *uobject;atomic_t usecnt;/* count all resources */ u32 unsafe_global_rkey;
Signature · 1 line
2.20.1
PATCH 5/25 IB/core: ib_uobject need HW object reference count
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
This new refcnt will points to the refcnt member of the HW object and will behaves as expected by refcnt, i.e. will be increased and decreased as a result of usage changes and will destroy the object when reaches to zero. For a non-shared object refcnt will remain NULL.
--- drivers/infiniband/core/rdma_core.c | 23 +++++++++++++++++++++-- include/rdma/ib_verbs.h | 7 +++++++ 2 files changed, 28 insertions(+), 2 deletions(-)
drivers/infiniband/core/rdma_core.c +21 -2
@@ -516,7 +516,26 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,const struct uverbs_obj_idr_type *idr_type =container_of (uobj->uapi_object->type_attrs, struct uverbs_obj_idr_type, type); -int ret = idr_type->destroy_object (uobj, why, attrs); +static DEFINE_MUTEX (lock); +int ret, count; + +mutex_lock (&lock); + +if (uobj->refcnt) { + count =atomic_dec_return (uobj->refcnt); +WARN_ON (count <0 );/* use after free! */ +if (count) { +mutex_unlock (&lock); + goto skip; + } + } + + ret = idr_type->destroy_object (uobj, why, attrs); + +if (ret) +atomic_inc (uobj->refcnt); + +mutex_unlock (&lock);/* * We can only fail gracefully if the user requested to destroy the
@@ -525,7 +544,7 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,*/ if (ib_is_destroy_retryable (ret, why, uobj))return ret; - +skip:if (why ==RDMA_REMOVE_ABORT )return 0 ;
include/rdma/ib_verbs.h +7 -0
@@ -1444,6 +1444,13 @@ struct ib_uobject {struct rcu_head rcu;/* kfree_rcu() overhead */ const struct uverbs_api_object *uapi_object; + +/* +* ib_X HW object sharing support +* - NULL for HW objects that are not shareable +* - Pointer to ib_X reference counter for shareable HW objects +*/ +atomic_t *refcnt;/* ib_X object ref count */ };struct ib_udata {
Signature · 1 line
2.20.1
PATCH 6/25 IB/uverbs: Helper function to initialize ufile member of uverbs_attr_bundle
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Helper function to initialize ufile member of structure uverbs_attr_bundle.
This helper produce structure uverbs_attr_bundle that is safe for the below operations on the given ufile:
- uobj_get_[read|write]
- uobj_alloc_commit
- uobj_alloc_abort
The last 2 ops are more complicated. Abort for example triggers the below which pass the uverbs_attr_bundle driver_udata down to the drivers level:
uobj_alloc_abort
rdma_alloc_abort_uobject
uverbs_destroy_uobject
uobj->uapi_object->type_class->destroy_hw
destroy_hw_idr_uobject
idr_type->destroy_object
uverbs_free_pd
ib_dealloc_pd_user
pd->device->ops.dealloc_pd
mlx4_ib_dealloc_pdFor more information about the potential issues, please see commit f89adedaf3fe ("RDMA/uverbs: Initialize udata struct on destroy flows")
--- drivers/infiniband/core/uverbs_cmd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +18 -0
@@ -3773,6 +3773,24 @@ static int ib_uverbs_ex_modify_cq(struct uverbs_attr_bundle *attrs)return ret; } +/** +* uverbs_init_attrs_ufile - Helper function to create minimal +* uverbs_attr_bundle out of ib_uverbs_file that is suitable +* for the below operations: +* +* - uobj_get_[read|write] +* - uobj_alloc_commit +* - uobj_alloc_abort +*/ +static void uverbs_init_attrs_ufile (struct uverbs_attr_bundle *attrs_bundle, +struct ib_uverbs_file *ufile) +{ + *attrs_bundle = (struct uverbs_attr_bundle ) { + .ufile = ufile, + .context = ufile->ucontext, + }; +} +/* * Describe the input structs for write(). Some write methods have an input * only struct, most have an input and output. If the struct has an output then
Signature · 1 line
2.20.1
PATCH 7/25 IB/uverbs: Add context import lock/unlock helper
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
The lock/unlock helpers will be used in every import verb.
--- drivers/infiniband/core/uverbs.h | 2 + drivers/infiniband/core/uverbs_cmd.c | 73 +++++++++++++++++++++++++++ drivers/infiniband/core/uverbs_main.c | 1 + 3 files changed, 76 insertions(+)
drivers/infiniband/core/uverbs.h +2 -0
@@ -163,6 +163,8 @@ struct ib_uverbs_file {struct page *disassociate_page;struct xarray idr; + +struct file *filp; };struct ib_uverbs_event {
drivers/infiniband/core/uverbs_cmd.c +73 -0
85 lines not on this page · raw message
drivers/infiniband/core/uverbs_main.c +1 -0
@@ -1091,6 +1091,7 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)mutex_init (&file->umap_lock);INIT_LIST_HEAD (&file->umaps); + file->filp = filp; filp->private_data = file;list_add_tail (&file->list, &dev->uverbs_file_list);mutex_unlock (&dev->lists_mutex);
Signature · 1 line
2.20.1
PATCH 8/25 IB/uverbs: ufile must be freed only when not used anymore
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
ufile (&ucontext) with the process who own them must not be released when there are other ufile (&ucontext) that depens at them.
--- drivers/infiniband/core/rdma_core.c | 29 +++++++++++++++++++++++++++ drivers/infiniband/core/uverbs.h | 22 ++++++++++++++++++++ drivers/infiniband/core/uverbs_cmd.c | 16 +++++++++++++++ drivers/infiniband/core/uverbs_main.c | 4 ++++ 4 files changed, 71 insertions(+)
drivers/infiniband/core/rdma_core.c +29 -0
@@ -841,6 +841,33 @@ static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, ufile->ucontext =NULL ; } +static void __uverbs_ufile_refcount (struct ib_uverbs_file *ufile) +{ +int wait; + +if (ufile->parent) { +pr_debug ("%s: release parent ufile. ufile %p parent %p\n" , + __func__, ufile, ufile->parent); +if (atomic_dec_and_test (&ufile->parent->refcount)) +complete (&ufile->parent->context_released); + } + +if (!atomic_dec_and_test (&ufile->refcount)) { +wait: + wait =wait_for_completion_interruptible_timeout ( + &ufile->context_released,3 *HZ ); +if (wait == -ERESTARTSYS ) { +WARN_ONCE (1 , +"signal while waiting for context release! ufile %p\n" , + ufile); + }else if (wait ==0 ) { +pr_debug ("%s: timeout while waiting for context release! ufile %p\n" , + __func__, ufile); + goto wait; + } + } +} +static int __uverbs_cleanup_ufile (struct ib_uverbs_file *ufile,enum rdma_remove_reason reason) {
@@ -923,6 +950,8 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,if (!list_empty (&ufile->uobjects))__uverbs_cleanup_ufile (ufile, reason); +__uverbs_ufile_refcount (ufile); +ufile_destroy_ucontext (ufile, reason); done:
drivers/infiniband/core/uverbs.h +22 -0
@@ -165,6 +165,28 @@ struct ib_uverbs_file {struct xarray idr;struct file *filp; + +/* context sharing support */ + +/* +* refcount=1 : zero external context depend on this context +* refcount>1 : (refcount-1) external context depend on this context +*/ +atomic_t refcount; + +/* +* parent =NULL : this context has not imported any objects +* parent!=NULL : this context imported objects from parent context +*/ +struct ib_uverbs_file *parent; + +/* +* context_released completion is signalled by the last +* user of this context in case of shared context. +* any pre matured release of the context will be delayed +* till the last user of this context is gone. +*/ +struct completion context_released; };struct ib_uverbs_event {
drivers/infiniband/core/uverbs_cmd.c +16 -0
@@ -3816,6 +3816,10 @@ static int ib_uverbs_import_lock(struct uverbs_attr_bundle *attrs,struct ib_uverbs_device *fd_dev;int ret =0 ; +/* shared context cannot import */ +if (atomic_read (&file->refcount) >1 ) +return -EINVAL ; + *filep =fget (fd);if (!*filep)return -EINVAL ;
@@ -3829,6 +3833,18 @@ static int ib_uverbs_import_lock(struct uverbs_attr_bundle *attrs, *ufile = (*filep)->private_data; fd_dev = (*ufile)->device; +if (file->parent) { +/* multiple import must use same parent context! */ +if (file->parent != *ufile) { + ret = -EINVAL ; + goto file; + } + }else { +/* first import must update parent refcount */ + file->parent = *ufile; +atomic_inc (&(*ufile)->refcount); + } +/* check that both files belong to same ib_device */ if (dev != fd_dev) { ret = -EINVAL ;
drivers/infiniband/core/uverbs_main.c +4 -0
@@ -1081,6 +1081,10 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp) goto err; } +/* shared context support */ +atomic_set (&file->refcount,1 ); +init_completion (&file->context_released); + file->device = dev;kref_init (&file->ref);mutex_init (&file->ucontext_lock);
Signature · 1 line
2.20.1
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 4 lines · Shamir Rabinovitch
From: Shamir Rabinovitch <[email protected]>
ufile (&ucontext) with the process who own them must not be released when there are other ufile (&ucontext) that depens at them.
We already have a kref, why do we need more? Especially wrongly done refcounts with atomics?
Trying to sequence the destroy of the ucontext seems inherently wrong to me. If the driver has to link the PD/MR to data in the ucontext it can't support sharing.
Quoted 24 lines
Signed-off-by Shamir Rabinovitch <[email protected]>Signed-off-by Shamir Rabinovitch <[email protected]>drivers/infiniband/core/rdma_core.c | 29 +++++++++++++++++++++++++++ drivers/infiniband/core/uverbs.h | 22 ++++++++++++++++++++ drivers/infiniband/core/uverbs_cmd.c | 16 +++++++++++++++ drivers/infiniband/core/uverbs_main.c | 4 ++++ 4 files changed, 71 insertions(+)drivers/infiniband/core/rdma_core.c +10
-0+++ b/drivers/infiniband/core/rdma_core.c@@ -841,6 +841,33 @@ static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, ufile->ucontext =NULL ; } +static void __uverbs_ufile_refcount (struct ib_uverbs_file *ufile) +{ +int wait; + +if (ufile->parent) { +pr_debug ("%s: release parent ufile. ufile %p parent %p\n" , + __func__, ufile, ufile->parent); +if (atomic_dec_and_test (&ufile->parent->refcount)) +complete (&ufile->parent->context_released);
drivers/infiniband/core/rdma_core.c static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, · line 862
Quoted 10 lines
drivers/infiniband/core/rdma_core.c +10
-0+ } + +if (!atomic_dec_and_test (&ufile->refcount)) { +wait: + wait =wait_for_completion_interruptible_timeout ( + &ufile->context_released,3 *HZ ); +if (wait == -ERESTARTSYS ) { +WARN_ONCE (1 , +"signal while waiting for context release! ufile %p\n" , + ufile);
????
Jason
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 8 lines · Jason Gunthorpe
Quoted 4 lines · Shamir Rabinovitch
From: Shamir Rabinovitch <[email protected]>
ufile (&ucontext) with the process who own them must not be released when there are other ufile (&ucontext) that depens at them.
We already have a kref, why do we need more? Especially wrongly done refcounts with atomics?
Yes. Will fix in v2.
Quoted 4 lines
Trying to sequence the destroy of the ucontext seems inherently wrong to me. If the driver has to link the PD/MR to data in the ucontext it can't support sharing.
The issue we try to solve here is this:
[process 1] [process 2]
- alloc mr & point mr to -
context 1
- share context - - - import mr - exit - -- ufile_destroy_ucontext - --- ib_mr is not destroyed - --- context 1 is destroyed - - - exit - -- ufile_destroy_ucontext - --- driver dereg_mr is called - ---- ib_umem_release on umem from
previously destroyed context 1
If I recall correctly, you suggested the shere and shree concept.
We also talked with Mellanox architecture team and they suggested that the shrere will be bullet proof process that *only* create and share objects.
The whole thing directly links to the next step we talked about which is sharing objects via file system rathen then via FD.
Quoted 38 lines
Quoted 25 lines
Signed-off-by Shamir Rabinovitch <[email protected]>Signed-off-by Shamir Rabinovitch <[email protected]>drivers/infiniband/core/rdma_core.c | 29 +++++++++++++++++++++++++++ drivers/infiniband/core/uverbs.h | 22 ++++++++++++++++++++ drivers/infiniband/core/uverbs_cmd.c | 16 +++++++++++++++ drivers/infiniband/core/uverbs_main.c | 4 ++++ 4 files changed, 71 insertions(+)drivers/infiniband/core/rdma_core.c +10
-0+++ b/drivers/infiniband/core/rdma_core.c@@ -841,6 +841,33 @@ static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, ufile->ucontext =NULL ; } +static void __uverbs_ufile_refcount (struct ib_uverbs_file *ufile) +{ +int wait; + +if (ufile->parent) { +pr_debug ("%s: release parent ufile. ufile %p parent %p\n" , + __func__, ufile, ufile->parent); +if (atomic_dec_and_test (&ufile->parent->refcount)) +complete (&ufile->parent->context_released);drivers/infiniband/core/rdma_core.c static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, · line 862
Quoted 10 lines
drivers/infiniband/core/rdma_core.c +10
-0+ } + +if (!atomic_dec_and_test (&ufile->refcount)) { +wait: + wait =wait_for_completion_interruptible_timeout ( + &ufile->context_released,3 *HZ ); +if (wait == -ERESTARTSYS ) { +WARN_ONCE (1 , +"signal while waiting for context release! ufile %p\n" , + ufile);????
Quoted 1 line
Jason
I copied the behaviour I saw in the rest of the kernel as for what to do when wait_for_completion_interruptible_timeout exit due to interrupt.
From the above reason I think we need to delay the shrere process exit so it will not close the context prematurely *unless* it receive signal.
In that case I'd expect that process to soot some clear warning and do whatevere id need to do for the given signal.
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 23 lines · Shamir Rabinovitch
Quoted 8 lines · Jason Gunthorpe
Quoted 4 lines · Shamir Rabinovitch
From: Shamir Rabinovitch <[email protected]>
ufile (&ucontext) with the process who own them must not be released when there are other ufile (&ucontext) that depens at them.
We already have a kref, why do we need more? Especially wrongly done refcounts with atomics?
Yes. Will fix in v2.
Quoted 4 lines
Trying to sequence the destroy of the ucontext seems inherently wrong to me. If the driver has to link the PD/MR to data in the ucontext it can't support sharing.
The issue we try to solve here is this:
[process 1] [process 2]
- alloc mr & point mr to -
context 1
- share context -
Review
Quoted 7 lines
(quoted diff) +0
-6- - import mr - exit - - - exit - -- ufile_destroy_ucontext - --- driver dereg_mr is called - ---- ib_umem_release on umem from previously destroyed context 1
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Quoted 20 lines
Quoted 20 lines
Quoted 7 lines
drivers/infiniband/core/rdma_core.c +7
-0+int wait; + +if (ufile->parent) { +pr_debug ("%s: release parent ufile. ufile %p parent %p\n" , + __func__, ufile, ufile->parent); +if (atomic_dec_and_test (&ufile->parent->refcount)) +complete (&ufile->parent->context_released);drivers/infiniband/core/rdma_core.c static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, · line 862
Quoted 10 lines
drivers/infiniband/core/rdma_core.c +10
-0+ } + +if (!atomic_dec_and_test (&ufile->refcount)) { +wait: + wait =wait_for_completion_interruptible_timeout ( + &ufile->context_released,3 *HZ ); +if (wait == -ERESTARTSYS ) { +WARN_ONCE (1 , +"signal while waiting for context release! ufile %p\n" , + ufile);????
Quoted 4 lines
Quoted 1 line
Jason
I copied the behaviour I saw in the rest of the kernel as for what to do when
wait_for_completion_interruptible_timeoutexit due to interrupt.
It doesn't really make sense here, we can't block release() like this
Jason
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 32 lines · Jason Gunthorpe
Quoted 23 lines · Shamir Rabinovitch
Quoted 8 lines · Jason Gunthorpe
Quoted 4 lines · Shamir Rabinovitch
From: Shamir Rabinovitch <[email protected]>
ufile (&ucontext) with the process who own them must not be released when there are other ufile (&ucontext) that depens at them.
We already have a kref, why do we need more? Especially wrongly done refcounts with atomics?
Yes. Will fix in v2.
Quoted 4 lines
Trying to sequence the destroy of the ucontext seems inherently wrong to me. If the driver has to link the PD/MR to data in the ucontext it can't support sharing.
The issue we try to solve here is this:
[process 1] [process 2]
- alloc mr & point mr to -
context 1
- share context -Review
Quoted 7 lines
(quoted diff) +0
-6- - import mr - exit - - - exit - -- ufile_destroy_ucontext - --- driver dereg_mr is called - ---- ib_umem_release on umem from previously destroyed context 1
Quoted 2 lines
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
Quoted 29 lines
Quoted 21 lines
Quoted 21 lines
Quoted 8 lines
drivers/infiniband/core/rdma_core.c +7
-0+int wait; + +if (ufile->parent) { +pr_debug ("%s: release parent ufile. ufile %p parent %p\n" , + __func__, ufile, ufile->parent); +if (atomic_dec_and_test (&ufile->parent->refcount)) +complete (&ufile->parent->context_released);drivers/infiniband/core/rdma_core.c static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile, · line 862
Quoted 10 lines
drivers/infiniband/core/rdma_core.c +10
-0+ } + +if (!atomic_dec_and_test (&ufile->refcount)) { +wait: + wait =wait_for_completion_interruptible_timeout ( + &ufile->context_released,3 *HZ ); +if (wait == -ERESTARTSYS ) { +WARN_ONCE (1 , +"signal while waiting for context release! ufile %p\n" , + ufile);????
Quoted 4 lines
Quoted 1 line
Jason
I copied the behaviour I saw in the rest of the kernel as for what to do when
wait_for_completion_interruptible_timeoutexit due to interrupt.It doesn't really make sense here, we can't block
release()like thisJason
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
ok, how about this: we know that for MR to be shared the memory behinds it should also be shared.
In this case, i know it sounds horrifying but do we care that the process that originally created this MR exits? i.e. how about just before the process leaves this world we will find some other ucontext to hold these memory mappings that driver holds?
Or how about moving this mapping from ucontext pointed by ib_mr directly to ib_mr?
Yuval
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 11 lines · Yuval Shaia
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
Quoted 7 lines
ok, how about this: we know that for MR to be shared the memory behinds it should also be shared.
In this case, i know it sounds horrifying but do we care that the process that originally created this MR exits? i.e. how about just before the process leaves this world we will find some other ucontext to hold these memory mappings that driver holds?
Could you create a better cover letter for this.
What is the use case for all this?
What protections are in place for accesses between processes? Why is it not sufficient to share the entire IB context with another process using SCM_RIGHTS?
I've been trying to create a reliable method for an admin to identify processes which may have pinned file backed pages (specifically with FS DAX). The best idea I have involves using the struct file of the ucontext to track the file pins through procfs. Jason and I discussed it here:
https://lkml.org/lkml/2019/6/7/548
Therefore, sharing to another ucontext _might_ work. But only if the file pin information is properly duplicated to the new ucontext. This complicates (at a minimum) having the generic GUP code handle the tracking of this information...
Generically, this whole thread scares me because now we are proposing something even more obscure than just the sharing of a file descriptor to other processes which point to the same IB context. (Or at least that is what I infer from reading the very short cover letter.) But if there is a good use case ... we need to starting thinking about something generic to track these "memory maps" which are not mmaps.
Ira
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
In reply to Yuval ShaiaQuoted 14 lines · Yuval Shaia
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
ok, how about this: we know that for MR to be shared the memory behinds it should also be shared.
Quoted 6 lines
In this case, i know it sounds horrifying but do we care that the process that originally created this MR exits? i.e. how about just before the process leaves this world we will find some other ucontext to hold these memory mappings that driver holds?
Or how about moving this mapping from ucontext pointed byib_mrdirectly toib_mr?
What are you worrying about? My point is we don't need to *anything* if the driver objects for PD and MR don't rely on the ucontext. This appears to be the normal case.
MRs already work fine if they outlive the creating process.
Jason
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 22 lines · Jason Gunthorpe
Quoted 14 lines · Yuval Shaia
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
ok, how about this: we know that for MR to be shared the memory behinds it should also be shared.
Quoted 6 lines
In this case, i know it sounds horrifying but do we care that the process that originally created this MR exits? i.e. how about just before the process leaves this world we will find some other ucontext to hold these memory mappings that driver holds?
Or how about moving this mapping from ucontext pointed byib_mrdirectly toib_mr?
Quoted 3 lines
What are you worrying about? My point is we don't need to *anything* if the driver objects for PD and MR don't rely on the ucontext. This appears to be the normal case.
but we saw that mlx4 (and i think also 5) do use the ucontext, i think to undo umem_get stuff.
Quoted 2 lines
MRs already work fine if they outlive the creating process.
You mean if we leave the creating process alive?
Quoted 2 lines
Jason
Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore
Quoted 23 lines · Yuval Shaia
Quoted 22 lines · Jason Gunthorpe
Quoted 20 lines · Yuval Shaia
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yuval Shaia
Quoted 2 lines · Jason Gunthorpe
Like I said, drivers that require the creating ucontext as part of the PD and MR cannot support sharing.
Even if we can make sure the process that creates the MR stays alive until all reference to this MR completes?
The kernel can't rely on userspace to do that.
ok, how about this: we know that for MR to be shared the memory behinds it should also be shared.
In this case, i know it sounds horrifying but do we care that the process that originally created this MR exits? i.e. how about just before the process leaves this world we will find some other ucontext to hold these memory mappings that driver holds?
Or how about moving this mapping from ucontext pointed byib_mrdirectly toib_mr?
Quoted 6 lines
Quoted 3 lines
What are you worrying about? My point is we don't need to *anything* if the driver objects for PD and MR don't rely on the ucontext. This appears to be the normal case.
but we saw that mlx4 (and i think also 5) do use the ucontext, i think to undo
umem_getstuff.
It is unnecessary, remove it.
Quoted 3 lines
Quoted 1 line
MRs already work fine if they outlive the creating process.
You mean if we leave the creating process alive?
No, let it die, it is fine
Jason
PATCH 9/25 IB/verbs: Prototype of HW object clone callback
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Define prototype for clone callback. The clone callback is used by the driver layer to supply the uverbs a way to clone IB HW object driver data to rdma-core user space provider. The clone callback is used when new IB HW object is created and every time it is imported to some ib_ucontext. Drivers that wish to enable share of some IB HW object (ib_pd, ib_mr, etc..) must supply valid clone callback for that type.
--- include/rdma/ib_verbs.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
include/rdma/ib_verbs.h +26 -0
@@ -2213,6 +2213,18 @@ struct iw_cm_conn_param;#define DECLARE_RDMA_OBJ_SIZE (ib_struct) size_t size_##ib_struct +/* +* Prototype for IB HW object clone callback +* +* Define prototype for clone callback. The clone callback is used +* by the driver layer to supply the uverbs a way to clone IB HW +* object driver data to rdma-core user space provider. The clone +* callback is used when new IB HW object is created and every time +* it is imported to some ib_ucontext. +*/ +#define clone_callback (ib_type) \ + int (*clone_##ib_type)(struct ib_udata *udata, struct ib_type *obj) +/** * struct ib_device_ops - InfiniBand device operations * This structure defines all the InfiniBand device operations, providers will
@@ -2523,6 +2535,9 @@ struct ib_device_ops {*/ int (*counter_update_stats)(struct rdma_counter *counter); +/* Object sharing callbacks */ +clone_callback (ib_pd); +DECLARE_RDMA_OBJ_SIZE (ib_ah);DECLARE_RDMA_OBJ_SIZE (ib_cq);DECLARE_RDMA_OBJ_SIZE (ib_pd);
@@ -2530,6 +2545,17 @@ struct ib_device_ops {DECLARE_RDMA_OBJ_SIZE (ib_ucontext); }; +/* Implementation of trivial clone callback */ +#define trivial_clone_callback (ib_type) \ +static inline int trivial_clone_##ib_type(struct ib_udata *udata, \ + struct ib_type *obj) \ +{ \ + return 0; \ +} + +/* Shared IB HW object support */ +trivial_clone_callback (ib_pd); +struct ib_core_device {/* device must be the first element in structure until, * union of ib_core_device and device exists in ib_device.
Signature · 1 line
2.20.1
PATCH 10/25 IB/core: Install clone ib_pd in device ops
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
Install clone_ib_pd in the device ops.
--- drivers/infiniband/core/device.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/core/device.c +1 -0
@@ -2572,6 +2572,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)SET_DEVICE_OP (dev_ops, set_vf_guid);SET_DEVICE_OP (dev_ops, set_vf_link_state);SET_DEVICE_OP (dev_ops, unmap_fmr); +SET_DEVICE_OP (dev_ops, clone_ib_pd);SET_OBJ_SIZE (dev_ops, ib_ah);SET_OBJ_SIZE (dev_ops, ib_cq);
Signature · 1 line
2.20.1
PATCH 11/25 IB/mlx4: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Copy mlx4 ib_pd to user-space.
--- drivers/infiniband/hw/mlx4/main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx4/main.c +13 -1
@@ -1178,6 +1178,13 @@ static int mlx4_ib_mmap(struct ib_ucontext *context, struct vm_area_struct *vma) } } +static int mlx4_ib_clone_pd (struct ib_udata *udata,struct ib_pd *ibpd) +{ +struct mlx4_ib_pd *pd =to_mpd (ibpd); + +return udata ?ib_copy_to_udata (udata, &pd->pdn,sizeof (__u32)) :0 ; +} +static int mlx4_ib_alloc_pd (struct ib_pd *ibpd,struct ib_udata *udata) {struct mlx4_ib_pd *pd =to_mpd (ibpd);
@@ -1188,10 +1195,12 @@ static int mlx4_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)if (err)return err; -if (udata &&ib_copy_to_udata (udata, &pd->pdn,sizeof (__u32))) { + err =mlx4_ib_clone_pd (udata, ibpd); +if (err) {mlx4_pd_free (to_mdev (ibdev)->dev, pd->pdn);return -EFAULT ; } +return 0 ; }
@@ -2564,6 +2573,9 @@ static const struct ib_device_ops mlx4_ib_dev_ops = { .rereg_user_mr = mlx4_ib_rereg_user_mr, .resize_cq = mlx4_ib_resize_cq, +/* Object sharing callbacks */ + .clone_ib_pd = mlx4_ib_clone_pd, +INIT_RDMA_OBJ_SIZE (ib_ah, mlx4_ib_ah, ibah),INIT_RDMA_OBJ_SIZE (ib_cq, mlx4_ib_cq, ibcq), INIT_RDMA_OBJ_SIZE(ib_pd, mlx4_ib_pd, ibpd),
Signature · 1 line
2.20.1
PATCH 12/25 IB/mlx5: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Copy mlx5 ib_pd to user-space.
--- drivers/infiniband/hw/mlx5/main.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)
drivers/infiniband/hw/mlx5/main.c +22 -7
@@ -2464,11 +2464,24 @@ int mlx5_ib_dealloc_dm(struct ib_dm *ibdm, struct uverbs_attr_bundle *attrs)return 0 ; } +static int mlx5_ib_clone_pd (struct ib_udata *udata,struct ib_pd *ibpd) +{ +struct mlx5_ib_pd *pd =to_mpd (ibpd); +struct mlx5_ib_alloc_pd_resp resp; +int ret =0 ; + +if (udata) { + resp.pdn = pd->pdn; + ret =ib_copy_to_udata (udata, &resp,sizeof (resp)); + } + +return ret; +} +static int mlx5_ib_alloc_pd (struct ib_pd *ibpd,struct ib_udata *udata) {struct mlx5_ib_pd *pd =to_mpd (ibpd);struct ib_device *ibdev = ibpd->device; -struct mlx5_ib_alloc_pd_resp resp;int err;u32 out[MLX5_ST_SZ_DW (alloc_pd_out)] = {};u32 in[MLX5_ST_SZ_DW (alloc_pd_in)] = {};
@@ -2486,12 +2499,11 @@ static int mlx5_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata) pd->pdn =MLX5_GET (alloc_pd_out, out, pd); pd->uid = uid; -if (udata) { - resp.pdn = pd->pdn; -if (ib_copy_to_udata (udata, &resp,sizeof (resp))) { -mlx5_cmd_dealloc_pd (to_mdev (ibdev)->mdev, pd->pdn, uid); -return -EFAULT ; - } + + err =mlx5_ib_clone_pd (udata, ibpd); +if (err) { +mlx5_cmd_dealloc_pd (to_mdev (ibdev)->mdev, pd->pdn, uid); +return err; }return 0 ;
@@ -6301,6 +6313,9 @@ static const struct ib_device_ops mlx5_ib_dev_ops = { .rereg_user_mr = mlx5_ib_rereg_user_mr, .resize_cq = mlx5_ib_resize_cq, +/* Object sharing callbacks */ + .clone_ib_pd = mlx5_ib_clone_pd, +INIT_RDMA_OBJ_SIZE (ib_ah, mlx5_ib_ah, ibah),INIT_RDMA_OBJ_SIZE (ib_cq, mlx5_ib_cq, ibcq), INIT_RDMA_OBJ_SIZE(ib_pd, mlx5_ib_pd, ibpd),
Signature · 1 line
2.20.1
PATCH 13/25 RDMA/rxe: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Copy rxe ib_pd to user-space.
--- drivers/infiniband/sw/rxe/rxe_verbs.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/infiniband/sw/rxe/rxe_verbs.c +3 -0
@@ -1148,6 +1148,9 @@ static const struct ib_device_ops rxe_dev_ops = { .req_notify_cq = rxe_req_notify_cq, .resize_cq = rxe_resize_cq, +/* Object sharing callbacks */ + .clone_ib_pd = trivial_clone_ib_pd, +INIT_RDMA_OBJ_SIZE (ib_ah, rxe_ah, ibah),INIT_RDMA_OBJ_SIZE (ib_cq, rxe_cq, ibcq), INIT_RDMA_OBJ_SIZE(ib_pd, rxe_pd, ibpd),
Signature · 1 line
2.20.1
PATCH 14/25 IB/uverbs: Add clone reference counting to ib_pd
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
To keep track of shared object life time add ref count to ib_pd
--- drivers/infiniband/core/uverbs_cmd.c | 3 +++ include/rdma/ib_verbs.h | 3 +++ 2 files changed, 6 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +3 -0
@@ -436,12 +436,15 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs) pd->__internal_mr =NULL ;atomic_set (&pd->usecnt,0 ); pd->res.type =RDMA_RESTRACK_PD ; +/* number of uobj using this ib_pd */ +atomic_set (&pd->refcnt,1 ); ret = ib_dev->ops.alloc_pd (pd, &attrs->driver_udata);if (ret) goto err_alloc; uobj->object = pd; + uobj->refcnt = &pd->refcnt;memset (&resp,0 ,sizeof resp); resp.pd_handle = uobj->id;rdma_restrack_uadd (&pd->res);
include/rdma/ib_verbs.h +3 -0
@@ -1473,6 +1473,9 @@ struct ib_pd {*/ struct ib_mr *__internal_mr;struct rdma_restrack_entry res; + +/* number of uobj using this ib_pd */ +atomic_t refcnt; };struct ib_xrcd {
Signature · 1 line
2.20.1
PATCH 15/25 IB/uverbs: Add PD import verb
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Implementation of new verb which enable to import ib_pd object.
--- drivers/infiniband/core/uverbs_cmd.c | 95 ++++++++++++++++++++++++++++ include/uapi/rdma/ib_user_verbs.h | 14 ++++ 2 files changed, 109 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +95 -0
@@ -3883,6 +3883,96 @@ static void ib_uverbs_import_unlock(struct ib_uobject *uobj,fput (filep); } +static void ib_uverbs_clone_ib_pd (union ib_uverbs_import_fr_fd_resp *resp, +struct ib_pd *pd, +__u32 handle) +{ + resp->alloc_pd.pd_handle = handle; +} + +static int ib_uverbs_import_ib_pd (struct uverbs_attr_bundle *attrs) +{ +union ib_uverbs_import_fr_fd_resp resp = {0 }; +struct ib_uobject *src_uobj =NULL ; +struct ib_uobject *dst_uobj =NULL ; +struct ib_uverbs_import_fr_fd cmd; +struct ib_uverbs_file *src_file; +struct ib_device *ibdev; +struct ib_pd *pd; +struct file *filep; +int ret; + + ret =uverbs_request (attrs, &cmd,sizeof (cmd)); +if (ret) +return ret; + + ret =ib_uverbs_import_lock (attrs, cmd.fd,UVERBS_OBJECT_PD , + cmd.handle, &src_uobj, &filep, + &src_file); +if (ret) +return ret; + + pd = src_uobj->object; + + dst_uobj =uobj_alloc (UVERBS_OBJECT_PD , attrs, &ibdev); +if (IS_ERR (dst_uobj)) { + ret = -ENOMEM ; + goto uobj; + } + +if (!ibdev->ops.clone_ib_pd) { + ret = -EINVAL ; + goto uobj; + } + + dst_uobj->object = src_uobj->object; + dst_uobj->refcnt = src_uobj->refcnt; + +if (WARN_ON (!atomic_inc_not_zero (src_uobj->refcnt))) { + ret = -EINVAL ; + goto uobj; + } + + ret = ibdev->ops.clone_ib_pd (&attrs->driver_udata, pd); +if (ret) + goto uobj; + +ib_uverbs_clone_ib_pd (&resp, dst_uobj->object, dst_uobj->id); + + ret =uverbs_response (attrs, &resp,sizeof (resp)); +if (ret) + goto uobj; + + ret =uobj_alloc_commit (dst_uobj, attrs); + +ib_uverbs_import_unlock (src_uobj, filep); + +return ret; + +uobj: +if (!IS_ERR_OR_NULL (dst_uobj)) +uobj_alloc_abort (dst_uobj, attrs); + +return ret; +} + +static int ib_uverbs_import_fr_fd (struct uverbs_attr_bundle *attrs) +{ +struct ib_uverbs_import_fr_fd cmd; +int ret; + + ret =uverbs_request (attrs, &cmd,sizeof (cmd)); +if (ret) +return ret; + +switch (cmd.type) { +case UVERBS_OBJECT_PD : +return ib_uverbs_import_ib_pd (attrs); + } + +return -EINVAL ; +} +/* * Describe the input structs for write(). Some write methods have an input * only struct, most have an input and output. If the struct has an output then
@@ -4015,6 +4105,11 @@ const struct uapi_definition uverbs_def_write_intf[] = { UAPI_DEF_WRITE_IO(struct ib_uverbs_query_port, struct ib_uverbs_query_port_resp), UAPI_DEF_METHOD_NEEDS_FN(query_port)), + DECLARE_UVERBS_WRITE( + IB_USER_VERBS_CMD_IMPORT_FR_FD, + ib_uverbs_import_fr_fd, + UAPI_DEF_WRITE_IO(struct ib_uverbs_import_fr_fd, + union ib_uverbs_import_fr_fd_resp)), DECLARE_UVERBS_WRITE_EX( IB_USER_VERBS_EX_CMD_QUERY_DEVICE, ib_uverbs_ex_query_device,
include/uapi/rdma/ib_user_verbs.h +14 -0
@@ -88,6 +88,7 @@ enum ib_uverbs_write_cmds {IB_USER_VERBS_CMD_CLOSE_XRCD ,IB_USER_VERBS_CMD_CREATE_XSRQ ,IB_USER_VERBS_CMD_OPEN_QP , + IB_USER_VERBS_CMD_IMPORT_FR_FD, }; enum {
@@ -1299,6 +1300,19 @@ struct ib_uverbs_ex_modify_cq { __u32 reserved; }; +/* object sharing support */ +struct ib_uverbs_import_fr_fd { +__u64 response; +__u32 fd; +__u32 handle; +__u16 type; +__u8 reserved[6 ]; +}; + +union ib_uverbs_import_fr_fd_resp { +struct ib_uverbs_alloc_pd_resp alloc_pd; +}; +#define IB_DEVICE_NAME_MAX 64#endif /* IB_USER_VERBS_H */
Signature · 1 line
2.20.1
Re: [PATCH 15/25] IB/uverbs: Add PD import verb
drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4113
Quoted 13 lines · Shamir Rabinovitch
/*
* Describe the input structs forwrite(). Some write methods have an input
* only struct, most have an input and output. If the struct has an output thendrivers/infiniband/core/uverbs_cmd.c +5
-0@@ -4015,6 +4105,11 @@ const struct uapi_definition uverbs_def_write_intf[] = { UAPI_DEF_WRITE_IO(struct ib_uverbs_query_port, struct ib_uverbs_query_port_resp), UAPI_DEF_METHOD_NEEDS_FN(query_port)), + DECLARE_UVERBS_WRITE( + IB_USER_VERBS_CMD_IMPORT_FR_FD, + ib_uverbs_import_fr_fd, + UAPI_DEF_WRITE_IO(struct ib_uverbs_import_fr_fd, + union ib_uverbs_import_fr_fd_resp)),
New non-ioctl interfaces are not allowed now
Jason
Re: [PATCH 15/25] IB/uverbs: Add PD import verb
Quoted 17 lines · Jason Gunthorpe
drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4113
Quoted 13 lines · Shamir Rabinovitch
/*
* Describe the input structs forwrite(). Some write methods have an input
* only struct, most have an input and output. If the struct has an output thendrivers/infiniband/core/uverbs_cmd.c +5
-0@@ -4015,6 +4105,11 @@ const struct uapi_definition uverbs_def_write_intf[] = { UAPI_DEF_WRITE_IO(struct ib_uverbs_query_port, struct ib_uverbs_query_port_resp), UAPI_DEF_METHOD_NEEDS_FN(query_port)), + DECLARE_UVERBS_WRITE( + IB_USER_VERBS_CMD_IMPORT_FR_FD, + ib_uverbs_import_fr_fd, + UAPI_DEF_WRITE_IO(struct ib_uverbs_import_fr_fd, + union ib_uverbs_import_fr_fd_resp)),New non-ioctl interfaces are not allowed now
Quoted 1 line
Jason
Oh. Can you suggest commit(s) that can be used as ref for new ioctl in rdma & rdma-core ?
Thanks
PATCH 16/25 IB/mlx4: Enable import from FD verb
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Turn on import_fr_fd bit in uverbs command mask.
--- drivers/infiniband/hw/mlx4/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx4/main.c +2 -1
@@ -2692,7 +2692,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev) (1ull <<IB_USER_VERBS_CMD_QUERY_SRQ ) | (1ull <<IB_USER_VERBS_CMD_DESTROY_SRQ ) | (1ull <<IB_USER_VERBS_CMD_CREATE_XSRQ ) | - (1ull <<IB_USER_VERBS_CMD_OPEN_QP ); + (1ull <<IB_USER_VERBS_CMD_OPEN_QP ) | + (1ull <<IB_USER_VERBS_CMD_IMPORT_FR_FD );ib_set_device_ops (&ibdev->ib_dev, &mlx4_ib_dev_ops); ibdev->ib_dev.uverbs_ex_cmd_mask |=
Signature · 1 line
2.20.1
PATCH 17/25 IB/mlx5: Enable import from FD verb
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Turn on import_fr_fd bit in uverbs command mask.
--- drivers/infiniband/hw/mlx5/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/main.c +2 -1
@@ -6386,7 +6386,8 @@ static int mlx5_ib_stage_caps_init(struct mlx5_ib_dev *dev) (1ull <<IB_USER_VERBS_CMD_QUERY_SRQ ) | (1ull <<IB_USER_VERBS_CMD_DESTROY_SRQ ) | (1ull <<IB_USER_VERBS_CMD_CREATE_XSRQ ) | - (1ull <<IB_USER_VERBS_CMD_OPEN_QP ); + (1ull <<IB_USER_VERBS_CMD_OPEN_QP ) | + (1ull <<IB_USER_VERBS_CMD_IMPORT_FR_FD ); dev->ib_dev.uverbs_ex_cmd_mask = (1ull <<IB_USER_VERBS_EX_CMD_QUERY_DEVICE ) | (1ull << IB_USER_VERBS_EX_CMD_CREATE_CQ) |
Signature · 1 line
2.20.1
PATCH 18/25 RDMA/rxe: Enable import from FD verb
In reply to Shamir RabinovitchFrom: Shamir Rabinovitch <[email protected]>
Turn on import_fr_fd bit in uverbs command mask.
--- drivers/infiniband/sw/rxe/rxe_verbs.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/sw/rxe/rxe_verbs.c +1 -0
@@ -1208,6 +1208,7 @@ int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name) | BIT_ULL(IB_USER_VERBS_CMD_DESTROY_AH) |BIT_ULL (IB_USER_VERBS_CMD_ATTACH_MCAST ) |BIT_ULL (IB_USER_VERBS_CMD_DETACH_MCAST ) + |BIT_ULL (IB_USER_VERBS_CMD_IMPORT_FR_FD ) ;ib_set_device_ops (dev, &rxe_dev_ops);
Signature · 1 line
2.20.1
PATCH 19/25 IB/core: ib_mr should not have ib_uobject pointer
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
As a preparation step to shared MR, where ib_mr object will be pointed by one or more ib_uobjects, remove ib_uobject pointer from ib_mr struct.
--- drivers/infiniband/core/uverbs_cmd.c | 1 - drivers/infiniband/core/uverbs_std_types_mr.c | 1 - drivers/infiniband/core/verbs.c | 3 --- include/rdma/ib_verbs.h | 3 ++- 4 files changed, 2 insertions(+), 6 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +0 -1
@@ -765,7 +765,6 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs) mr->type =IB_MR_TYPE_USER ; mr->dm =NULL ; mr->sig_attrs =NULL ; - mr->uobject = uobj;atomic_inc (&pd->usecnt); mr->res.type =RDMA_RESTRACK_MR ;rdma_restrack_uadd (&mr->res);
drivers/infiniband/core/uverbs_std_types_mr.c +0 -1
@@ -130,7 +130,6 @@ static int UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)( mr->pd = pd; mr->type =IB_MR_TYPE_DM ; mr->dm = dm; - mr->uobject = uobj;atomic_inc (&pd->usecnt);atomic_inc (&dm->usecnt);
drivers/infiniband/core/verbs.c +0 -3
@@ -299,7 +299,6 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags, mr->device = pd->device; mr->pd = pd; mr->type =IB_MR_TYPE_DMA ; - mr->uobject =NULL ; mr->need_inval = false; pd->__internal_mr = mr;
@@ -2035,7 +2034,6 @@ struct ib_mr *ib_alloc_mr_user(struct ib_pd *pd, enum ib_mr_type mr_type, mr->device = pd->device; mr->pd = pd; mr->dm =NULL ; - mr->uobject =NULL ;atomic_inc (&pd->usecnt); mr->need_inval = false; mr->res.type =RDMA_RESTRACK_MR ;
@@ -2088,7 +2086,6 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd, mr->device = pd->device; mr->pd = pd; mr->dm =NULL ; - mr->uobject =NULL ;atomic_inc (&pd->usecnt); mr->need_inval = false; mr->res.type =RDMA_RESTRACK_MR ;
include/rdma/ib_verbs.h +2 -1
@@ -1733,7 +1733,6 @@ struct ib_mr {enum ib_mr_type type;bool need_inval;union { -struct ib_uobject *uobject;/* user */ struct list_head qp_entry;/* FR */ };
@@ -2540,6 +2539,7 @@ struct ib_device_ops {/* Object sharing callbacks */ clone_callback (ib_pd); +clone_callback (ib_mr);DECLARE_RDMA_OBJ_SIZE (ib_ah);DECLARE_RDMA_OBJ_SIZE (ib_cq);
@@ -2558,6 +2558,7 @@ static inline int trivial_clone_##ib_type(struct ib_udata *udata, \/* Shared IB HW object support */ trivial_clone_callback (ib_pd); +trivial_clone_callback (ib_mr);struct ib_core_device {/* device must be the first element in structure until,
Signature · 1 line
2.20.1
PATCH 20/25 IB/core: Install clone ib_mr in device ops
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
Install clone_ib_mr in the device ops.
--- drivers/infiniband/core/device.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/core/device.c +1 -0
@@ -2573,6 +2573,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)SET_DEVICE_OP (dev_ops, set_vf_link_state);SET_DEVICE_OP (dev_ops, unmap_fmr);SET_DEVICE_OP (dev_ops, clone_ib_pd); +SET_DEVICE_OP (dev_ops, clone_ib_mr);SET_OBJ_SIZE (dev_ops, ib_ah);SET_OBJ_SIZE (dev_ops, ib_cq);
Signature · 1 line
2.20.1
PATCH 21/25 IB/mlx4: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
No special handling in for object cloning, utilize the trivial implementation.
--- drivers/infiniband/hw/mlx4/main.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/hw/mlx4/main.c +1 -0
@@ -2575,6 +2575,7 @@ static const struct ib_device_ops mlx4_ib_dev_ops = {/* Object sharing callbacks */ .clone_ib_pd = mlx4_ib_clone_pd, + .clone_ib_mr = trivial_clone_ib_mr,INIT_RDMA_OBJ_SIZE (ib_ah, mlx4_ib_ah, ibah), INIT_RDMA_OBJ_SIZE(ib_cq, mlx4_ib_cq, ibcq),
Signature · 1 line
2.20.1
PATCH 22/25 IB/mlx5: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
No special handling in for object cloning, utilize the trivial implementation.
--- drivers/infiniband/hw/mlx5/main.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/hw/mlx5/main.c +1 -0
@@ -6315,6 +6315,7 @@ static const struct ib_device_ops mlx5_ib_dev_ops = {/* Object sharing callbacks */ .clone_ib_pd = mlx5_ib_clone_pd, + .clone_ib_mr = trivial_clone_ib_mr,INIT_RDMA_OBJ_SIZE (ib_ah, mlx5_ib_ah, ibah), INIT_RDMA_OBJ_SIZE(ib_cq, mlx5_ib_cq, ibcq),
Signature · 1 line
2.20.1
PATCH 23/25 RDMA/rxe: Add implementation of clone_pd callback
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
No special handling in for object cloning, utilize the trivial implementation.
--- drivers/infiniband/sw/rxe/rxe_verbs.c | 1 + 1 file changed, 1 insertion(+)
drivers/infiniband/sw/rxe/rxe_verbs.c +1 -0
@@ -1150,6 +1150,7 @@ static const struct ib_device_ops rxe_dev_ops = {/* Object sharing callbacks */ .clone_ib_pd = trivial_clone_ib_pd, + .clone_ib_mr = trivial_clone_ib_mr,INIT_RDMA_OBJ_SIZE (ib_ah, rxe_ah, ibah), INIT_RDMA_OBJ_SIZE(ib_cq, rxe_cq, ibcq),
Signature · 1 line
2.20.1
PATCH 24/25 IB/uverbs: Add clone reference counting to ib_mr
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
To keep track of shared object life time add ref count to ib_mr
--- drivers/infiniband/core/uverbs_cmd.c | 2 ++ include/rdma/ib_verbs.h | 3 +++ 2 files changed, 5 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +2 -0
@@ -768,8 +768,10 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)atomic_inc (&pd->usecnt); mr->res.type =RDMA_RESTRACK_MR ;rdma_restrack_uadd (&mr->res); +atomic_set (&mr->refcnt,1 ); uobj->object = mr; + uobj->refcnt = &mr->refcnt;memset (&resp,0 ,sizeof resp); resp.lkey = mr->lkey;
include/rdma/ib_verbs.h +3 -0
@@ -1742,6 +1742,9 @@ struct ib_mr {* Implementation details of the RDMA core, don't use in drivers: */ struct rdma_restrack_entry res; + +/* number of uobj using this ib_pd */ +atomic_t refcnt; };struct ib_mw {
Signature · 1 line
2.20.1
PATCH 25/25 IB/uverbs: Add MR import verb
In reply to Shamir RabinovitchFrom: Yuval Shaia <[email protected]>
Implementation of new verb which enable to import ib_mr object.
--- drivers/infiniband/core/uverbs_cmd.c | 79 ++++++++++++++++++++++++++++ include/uapi/rdma/ib_user_verbs.h | 1 + 2 files changed, 80 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +79 -0
@@ -3891,6 +3891,15 @@ static void ib_uverbs_clone_ib_pd(union ib_uverbs_import_fr_fd_resp *resp, resp->alloc_pd.pd_handle = handle; } +static void ib_uverbs_clone_ib_mr (union ib_uverbs_import_fr_fd_resp *resp, +struct ib_mr *mr, +__u32 handle) +{ + resp->reg_mr.lkey = mr->lkey; + resp->reg_mr.rkey = mr->rkey; + resp->reg_mr.mr_handle = handle; +} +static int ib_uverbs_import_ib_pd (struct uverbs_attr_bundle *attrs) {union ib_uverbs_import_fr_fd_resp resp = {0 };
@@ -3957,6 +3966,74 @@ static int ib_uverbs_import_ib_pd(struct uverbs_attr_bundle *attrs)return ret; } +static int ib_uverbs_import_ib_mr (struct uverbs_attr_bundle *attrs) +{ +union ib_uverbs_import_fr_fd_resp resp = {0 }; +struct ib_uobject *src_uobj =NULL ; +struct ib_uobject *dst_uobj =NULL ; +struct ib_uverbs_import_fr_fd cmd; +struct ib_uverbs_file *src_file; +struct ib_device *ibdev; +struct ib_mr *mr; +struct file *filep; +int ret; + + ret =uverbs_request (attrs, &cmd,sizeof (cmd)); +if (ret) +return ret; + + ret =ib_uverbs_import_lock (attrs, cmd.fd,UVERBS_OBJECT_MR , + cmd.handle, &src_uobj, &filep, + &src_file); +if (ret) +return ret; + + mr = src_uobj->object; + + dst_uobj =uobj_alloc (UVERBS_OBJECT_MR , attrs, &ibdev); +if (IS_ERR (dst_uobj)) { + ret = -ENOMEM ; + goto uobj; + } + +if (!ibdev->ops.clone_ib_mr) { + ret = -EINVAL ; + goto uobj; + } + + dst_uobj->object = src_uobj->object; + dst_uobj->refcnt = src_uobj->refcnt; + +if (WARN_ON (!atomic_inc_not_zero (src_uobj->refcnt))) { + ret = -EINVAL ; + goto uobj; + } + + ret = ibdev->ops.clone_ib_mr (&attrs->driver_udata, mr); +if (ret) + goto uobj; + +ib_uverbs_clone_ib_mr (&resp, dst_uobj->object, dst_uobj->id); + + ret =uverbs_response (attrs, &resp,sizeof (resp)); +if (ret) + goto uobj; + + ret =uobj_alloc_commit (dst_uobj, attrs); + +ib_uverbs_import_unlock (src_uobj, filep); + +return ret; + +uobj: +ib_uverbs_import_unlock (src_uobj, filep); + +if (!IS_ERR_OR_NULL (dst_uobj)) +uobj_alloc_abort (dst_uobj, attrs); + +return ret; +} +static int ib_uverbs_import_fr_fd (struct uverbs_attr_bundle *attrs) {struct ib_uverbs_import_fr_fd cmd;
@@ -3969,6 +4046,8 @@ static int ib_uverbs_import_fr_fd(struct uverbs_attr_bundle *attrs)switch (cmd.type) {case UVERBS_OBJECT_PD :return ib_uverbs_import_ib_pd (attrs); +case UVERBS_OBJECT_MR : +return ib_uverbs_import_ib_mr (attrs); }return -EINVAL ;
include/uapi/rdma/ib_user_verbs.h +1 -0
@@ -1311,6 +1311,7 @@ struct ib_uverbs_import_fr_fd {union ib_uverbs_import_fr_fd_resp {struct ib_uverbs_alloc_pd_resp alloc_pd; +struct ib_uverbs_reg_mr_resp reg_mr; };#define IB_DEVICE_NAME_MAX 64
Signature · 1 line
2.20.1
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.
Future expansion of this can be done on top of the new infrastructure to any HW object that is suitable for sharing (mainly those that are not saving important data in the user context).
Quoted 11 lines · Shamir Rabinovitch
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 7 lines
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.
Well, why not just share the entire uverbs FD then? Once the PD is shared all security is lost anyhow..
This is not the model that was explained to me last year
Jason
Quoted 21 lines · Jason Gunthorpe
Quoted 11 lines · Shamir Rabinovitch
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 7 lines
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.
Quoted 6 lines
Well, why not just share the entire uverbs FD then? Once the PD is shared all security is lost anyhow..
This is not the model that was explained to me last year
Jason
We do share the whole uvrbs FD (context) with the second process and let that process to instantiate the PD & MR from the shared FD. The instantiation include creating new uobject in the second process context that points to the same ib_x HW objects.
The second process does not own the shared context.
It just use it to get access to the shared ib_x objects and then it mark those & shared FD as shared.
What was the expectation from "import_from_xxx" ?
Quoted 25 lines · Shamir Rabinovitch
Quoted 24 lines · Jason Gunthorpe
Quoted 11 lines · Shamir Rabinovitch
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 7 lines
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.Well, why not just share the entire uverbs FD then? Once the PD is shared all security is lost anyhow..
Quoted 6 lines
Quoted 3 lines
This is not the model that was explained to me last year
Jason
We do share the whole uvrbs FD (context) with the second process and let that process to instantiate the PD & MR from the shared FD.
Then the first (both) process(es) should have access to the MR right?
Quoted 5 lines
The instantiation include creating new uobject in the second process context that points to the same
ib_xHW objects.
The second process does not own the shared context.
It just use it to get access to the sharedib_xobjects and then it mark those & shared FD as shared.
I'm not following this?
Quoted 2 lines
What was the expectation from "
import_from_xxx" ?
... and I don't understand this question.
Ira
Quoted 26 lines · Ira Weiny
Quoted 25 lines · Shamir Rabinovitch
Quoted 24 lines · Jason Gunthorpe
Quoted 11 lines · Shamir Rabinovitch
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 7 lines
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.Well, why not just share the entire uverbs FD then? Once the PD is shared all security is lost anyhow..
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
This is not the model that was explained to me last year
Jason
We do share the whole uvrbs FD (context) with the second process and let that process to instantiate the PD & MR from the shared FD.
Then the first (both) process(es) should have access to the MR right?
Yes.
(Please note that since we maintain refcount then the ib_mr object will be destroyed only when the two will destroy it).
Quoted 8 lines
Quoted 6 lines
The instantiation include creating new uobject in the second process context that points to the same
ib_xHW objects.
The second process does not own the shared context.
It just use it to get access to the sharedib_xobjects and then it mark those & shared FD as shared.I'm not following this?
Shamir, correct me if i'm wrong here:
So there is one ib_mr object and two uobjects that "points" to it.
Quoted 5 lines
Quoted 3 lines
What was the expectation from "
import_from_xxx" ?... and I don't understand this question.
Shamir?
Quoted 3 lines
Ira
Quoted 37 lines · Shamir Rabinovitch
Quoted 21 lines · Jason Gunthorpe
Quoted 11 lines · Shamir Rabinovitch
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Shamir Rabinovitch
Following patch-set introduce the shared object feature.
A shared object feature allows one process to create HW objects (currently PD and MR) so that a second process can import.
That sounds like a major complication, so you'd better also explain the use case very well.
Quoted 7 lines
The main use case was that there is a server that has giant shared memory that is shared across many processes (lots of mtts). Each process needs the same memory registration (lots of mrs that register same memory).
In such scenario, the HCA runs out of mtts.
To solve this problem, an single memory registration is shared across all the process in that server saving hca mtts.Quoted 6 lines
Well, why not just share the entire uverbs FD then? Once the PD is shared all security is lost anyhow..
This is not the model that was explained to me last year
Jason
We do share the whole uvrbs FD (context) with the second process and let that process to instantiate the PD & MR from the shared FD. The instantiation include creating new uobject in the second process context that points to the same
ib_xHW objects.
The second process does not own the shared context.
It just use it to get access to the sharedib_xobjects and then it mark those & shared FD as shared.
Quoted 1 line
What was the expectation from "
import_from_xxx" ?
None of this discussion makes any sense to me, or matches the use model that I thought this was targetting
Jason