ib_core allocation patches

From: Leon Romanovsky <[email protected]>

The RWQ table is used for RSS uverbs and not in used for the kernel consumers, delete ib_create_rwq_ind_table() routine that is not called at all.

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/verbs.c | 39 ---------------------------------
 include/rdma/ib_verbs.h         |  3 ---
 2 files changed, 42 deletions(-)

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
  drivers/infiniband/core/device.c           |  1 +
  drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
  drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
  drivers/infiniband/core/verbs.c            | 25 +------------
  drivers/infiniband/hw/mlx4/main.c          |  4 +-
  drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
  drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
  drivers/infiniband/hw/mlx5/main.c          |  3 ++
  drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
  drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
  include/rdma/ib_verbs.h                    | 11 +++---
  11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
  	SET_OBJ_SIZE(dev_ops, ib_cq);
  	SET_OBJ_SIZE(dev_ops, ib_mw);
  	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
  	SET_OBJ_SIZE(dev_ops, ib_srq);
  	SET_OBJ_SIZE(dev_ops, ib_ucontext);
  	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  {
  	struct ib_uverbs_ex_create_rwq_ind_table cmd;
  	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
  	int err;
  	struct ib_rwq_ind_table_init_attr init_attr = {};
  	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
  	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
  	int i, j, num_read_wqs;
  	u32 num_wq_handles;
  	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  		goto put_wqs;
  	}
 
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
  		goto err_uobj;
  	}
 
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
  	rwq_ind_tbl->ind_tbl = wqs;
  	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
  	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  	rwq_ind_tbl->device = ib_dev;
  	atomic_set(&rwq_ind_tbl->usecnt, 0);
 
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
  	for (i = 0; i < num_wq_handles; i++)
  		atomic_inc(&wqs[i]->usecnt);
 

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  	return 0;
 
  err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
  err_uobj:
  	uobj_alloc_abort(uobj, attrs);
  put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
  			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
  			ib_uverbs_ex_destroy_rwq_ind_table,
  			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
 
  	DECLARE_UVERBS_OBJECT(
  		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +3 -1

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
  {
  	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
  	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+

drivers/infiniband/core/uverbs_std_types.c +1 -0

+	kfree(rwq_ind_tbl);
  	kfree(ind_tbl);
  	return ret;
  }

drivers/infiniband/core/verbs.c +1 -24

index 65c9118a931c..4210f0842bc6 100644
@@ -1703,7 +1703,7 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
  					  &old_sgid_attr_alt_av);
  		if (ret)
  			goto out_av;
-
+//
  		/*
  		 * Today the core code can only handle alternate paths and APM
  		 * for IB. Ban them in roce mode.

@@ -2412,29 +2412,6 @@ int ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,
  }
  EXPORT_SYMBOL(ib_modify_wq);
 
-/*
- * ib_destroy_rwq_ind_table - Destroys the specified Indirection Table.
- * @wq_ind_table: The Indirection Table to destroy.
-*/
-int ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *rwq_ind_table)
-{
-	int err, i;
-	u32 table_size = (1 << rwq_ind_table->log_ind_tbl_size);
-	struct ib_wq **ind_tbl = rwq_ind_table->ind_tbl;
-
-	if (atomic_read(&rwq_ind_table->usecnt))
-		return -EBUSY;
-
-	err = rwq_ind_table->device->ops.destroy_rwq_ind_table(rwq_ind_table);
-	if (!err) {
-		for (i = 0; i < table_size; i++)
-			atomic_dec(&ind_tbl[i]->usecnt);
-	}
-
-	return err;
-}
-EXPORT_SYMBOL(ib_destroy_rwq_ind_table);
-
  int ib_check_mr_status(struct ib_mr *mr, u32 check_mask,
  		       struct ib_mr_status *mr_status)
  {

drivers/infiniband/hw/mlx4/main.c +3 -1

index 0ad584a3b8d6..88a3a9a88bee 100644
@@ -2585,9 +2585,11 @@ static const struct ib_device_ops mlx4_ib_dev_ops = {
  static const struct ib_device_ops mlx4_ib_dev_wq_ops = {
  	.create_rwq_ind_table = mlx4_ib_create_rwq_ind_table,
  	.create_wq = mlx4_ib_create_wq,
-	.destroy_rwq_ind_table = mlx4_ib_destroy_rwq_ind_table,
  	.destroy_wq = mlx4_ib_destroy_wq,
  	.modify_wq = mlx4_ib_modify_wq,
+
+	INIT_RDMA_OBJ_SIZE(ib_rwq_ind_table, mlx4_ib_rwq_ind_table,
+			   ib_rwq_ind_tbl),
  };
 
  static const struct ib_device_ops mlx4_ib_dev_mw_ops = {

drivers/infiniband/hw/mlx4/mlx4_ib.h +7 -5

index 5fbe766aef6b..2730a0f65f47 100644
@@ -366,6 +366,10 @@ struct mlx4_ib_ah {
  	union mlx4_ext_av       av;
  };
 
+struct mlx4_ib_rwq_ind_table {
+	struct ib_rwq_ind_table ib_rwq_ind_tbl;
+};
+
  /****************************************/
  /* alias guid support */
  /****************************************/

@@ -893,11 +897,9 @@ void mlx4_ib_destroy_wq(struct ib_wq *wq, struct ib_udata *udata);
  int mlx4_ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,
  		      u32 wq_attr_mask, struct ib_udata *udata);
 
-struct ib_rwq_ind_table
-*mlx4_ib_create_rwq_ind_table(struct ib_device *device,
-			      struct ib_rwq_ind_table_init_attr *init_attr,
-			      struct ib_udata *udata);
-int mlx4_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *wq_ind_table);
+int mlx4_ib_create_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_tbl,
+				 struct ib_rwq_ind_table_init_attr *init_attr,
+				 u32 *ind_tbl_num, struct ib_udata *udata);
  int mlx4_ib_umem_calc_optimal_mtt_size(struct ib_umem *umem, u64 start_va,
  				       int *num_of_mtts);
 

drivers/infiniband/hw/mlx4/qp.c +11 -29

index cf51e3cbd969..5795bfc1a512 100644
@@ -4340,34 +4340,32 @@ void mlx4_ib_destroy_wq(struct ib_wq *ibwq, struct ib_udata *udata)
  	kfree(qp);
  }
 
-struct ib_rwq_ind_table
-*mlx4_ib_create_rwq_ind_table(struct ib_device *device,
-			      struct ib_rwq_ind_table_init_attr *init_attr,
-			      struct ib_udata *udata)
+int mlx4_ib_create_rwq_ind_table(struct ib_rwq_ind_table *rwq_ind_table,
+				 struct ib_rwq_ind_table_init_attr *init_attr,
+				 u32 *ind_tbl_num, struct ib_udata *udata)
  {
-	struct ib_rwq_ind_table *rwq_ind_table;
  	struct mlx4_ib_create_rwq_ind_tbl_resp resp = {};
  	unsigned int ind_tbl_size = 1 << init_attr->log_ind_tbl_size;
+	struct ib_device *device = rwq_ind_table->device;
  	unsigned int base_wqn;
  	size_t min_resp_len;
-	int i;
-	int err;
+	int i, err = 0;
 
  	if (udata->inlen > 0 &&
  	    !ib_is_udata_cleared(udata, 0,
  				 udata->inlen))
-		return ERR_PTR(-EOPNOTSUPP);
+		return -EOPNOTSUPP;
 
  	min_resp_len = offsetof(typeof(resp), reserved) + sizeof(resp.reserved);
  	if (udata->outlen && udata->outlen < min_resp_len)
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
  	if (ind_tbl_size >
  	    device->attrs.rss_caps.max_rwq_indirection_table_size) {
  		pr_debug("log_ind_tbl_size = %d is bigger than supported = %d\n",
  			 ind_tbl_size,
  			 device->attrs.rss_caps.max_rwq_indirection_table_size);
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
  	}
 
  	base_wqn = init_attr->ind_tbl[0]->wq_num;

@@ -4375,39 +4373,23 @@ struct ib_rwq_ind_table
  	if (base_wqn % ind_tbl_size) {
  		pr_debug("WQN=0x%x isn't aligned with indirection table size\n",
  			 base_wqn);
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
  	}
 
  	for (i = 1; i < ind_tbl_size; i++) {
  		if (++base_wqn != init_attr->ind_tbl[i]->wq_num) {
  			pr_debug("indirection table's WQNs aren't consecutive\n");
-			return ERR_PTR(-EINVAL);
+			return -EINVAL;
  		}
  	}
 
-	rwq_ind_table = kzalloc(sizeof(*rwq_ind_table), GFP_KERNEL);
-	if (!rwq_ind_table)
-		return ERR_PTR(-ENOMEM);
-
  	if (udata->outlen) {
  		resp.response_length = offsetof(typeof(resp), response_length) +
  					sizeof(resp.response_length);
  		err = ib_copy_to_udata(udata, &resp, resp.response_length);
-		if (err)
-			goto err;
  	}
 
-	return rwq_ind_table;
-
-err:
-	kfree(rwq_ind_table);
-	return ERR_PTR(err);
-}
-
-int mlx4_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_tbl)
-{
-	kfree(ib_rwq_ind_tbl);
-	return 0;
+	return err;
  }
 
  struct mlx4_ib_drain_cqe {

drivers/infiniband/hw/mlx5/main.c +3 -0

index a6d5c35a2845..682ba1b6f34a 100644
@@ -6844,6 +6844,9 @@ static const struct ib_device_ops mlx5_ib_dev_common_roce_ops = {
  	.destroy_wq = mlx5_ib_destroy_wq,
  	.get_netdev = mlx5_ib_get_netdev,
  	.modify_wq = mlx5_ib_modify_wq,
+
+	INIT_RDMA_OBJ_SIZE(ib_rwq_ind_table, mlx5_ib_rwq_ind_table,
+			   ib_rwq_ind_tbl),
  };
 
  static int mlx5_ib_stage_common_roce_init(struct mlx5_ib_dev *dev)

drivers/infiniband/hw/mlx5/mlx5_ib.h +4 -4

index 726386fe4440..c4a285e1950d 100644
@@ -1272,10 +1272,10 @@ struct ib_wq *mlx5_ib_create_wq(struct ib_pd *pd,
  void mlx5_ib_destroy_wq(struct ib_wq *wq, struct ib_udata *udata);
  int mlx5_ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,
  		      u32 wq_attr_mask, struct ib_udata *udata);
-struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
-						      struct ib_rwq_ind_table_init_attr *init_attr,
-						      struct ib_udata *udata);
-int mlx5_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *wq_ind_table);
+int mlx5_ib_create_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_table,
+				 struct ib_rwq_ind_table_init_attr *init_attr,
+				 u32 *ind_tbl_num, struct ib_udata *udata);
+void mlx5_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *wq_ind_table);
  struct ib_dm *mlx5_ib_alloc_dm(struct ib_device *ibdev,
  			       struct ib_ucontext *context,
  			       struct ib_dm_alloc_attr *attr,

drivers/infiniband/hw/mlx5/qp.c +17 -26

index 0ae73f4e28a3..3124c80169fb 100644
@@ -5063,12 +5063,13 @@ void mlx5_ib_destroy_wq(struct ib_wq *wq, struct ib_udata *udata)
  	kfree(rwq);
  }
 
-struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
-						      struct ib_rwq_ind_table_init_attr *init_attr,
-						      struct ib_udata *udata)
+int mlx5_ib_create_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_table,
+				 struct ib_rwq_ind_table_init_attr *init_attr,
+				 u32 *ind_tbl_num, struct ib_udata *udata)
  {
-	struct mlx5_ib_dev *dev = to_mdev(device);
-	struct mlx5_ib_rwq_ind_table *rwq_ind_tbl;
+	struct mlx5_ib_rwq_ind_table *rwq_ind_tbl =
+		to_mrwq_ind_table(ib_rwq_ind_table);
+	struct mlx5_ib_dev *dev = to_mdev(ib_rwq_ind_table->device);
  	int sz = 1 << init_attr->log_ind_tbl_size;
  	struct mlx5_ib_create_rwq_ind_tbl_resp resp = {};
  	size_t min_resp_len;

@@ -5081,30 +5082,24 @@ struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
  	if (udata->inlen > 0 &&
  	    !ib_is_udata_cleared(udata, 0,
  				 udata->inlen))
-		return ERR_PTR(-EOPNOTSUPP);
+		return -EOPNOTSUPP;
 
  	if (init_attr->log_ind_tbl_size >
  	    MLX5_CAP_GEN(dev->mdev, log_max_rqt_size)) {
  		mlx5_ib_dbg(dev, "log_ind_tbl_size = %d is bigger than supported = %d\n",
  			    init_attr->log_ind_tbl_size,
  			    MLX5_CAP_GEN(dev->mdev, log_max_rqt_size));
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
  	}
 
  	min_resp_len = offsetof(typeof(resp), reserved) + sizeof(resp.reserved);
  	if (udata->outlen && udata->outlen < min_resp_len)
-		return ERR_PTR(-EINVAL);
-
-	rwq_ind_tbl = kzalloc(sizeof(*rwq_ind_tbl), GFP_KERNEL);
-	if (!rwq_ind_tbl)
-		return ERR_PTR(-ENOMEM);
+		return -EINVAL;
 
  	inlen = MLX5_ST_SZ_BYTES(create_rqt_in) + sizeof(u32) * sz;
  	in = kvzalloc(inlen, GFP_KERNEL);
-	if (!in) {
-		err = -ENOMEM;
-		goto err;
-	}
+	if (!in)
+		return -ENOMEM;
 
  	rqtc = MLX5_ADDR_OF(create_rqt_in, in, rqt_context);
 

@@ -5118,12 +5113,10 @@ struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
  	MLX5_SET(create_rqt_in, in, uid, rwq_ind_tbl->uid);
 
  	err = mlx5_core_create_rqt(dev->mdev, in, inlen, &rwq_ind_tbl->rqtn);
-	kvfree(in);
-
  	if (err)
  		goto err;
 
-	rwq_ind_tbl->ib_rwq_ind_tbl.ind_tbl_num = rwq_ind_tbl->rqtn;
+	*ind_tbl_num = rwq_ind_tbl->rqtn;
  	if (udata->outlen) {
  		resp.response_length = offsetof(typeof(resp), response_length) +
  					sizeof(resp.response_length);

@@ -5132,24 +5125,22 @@ struct ib_rwq_ind_table *mlx5_ib_create_rwq_ind_table(struct ib_device *device,
  			goto err_copy;
  	}
 
-	return &rwq_ind_tbl->ib_rwq_ind_tbl;
+	kvfree(in);
+	return 0;
 
  err_copy:
  	mlx5_cmd_destroy_rqt(dev->mdev, rwq_ind_tbl->rqtn, rwq_ind_tbl->uid);
  err:
-	kfree(rwq_ind_tbl);
-	return ERR_PTR(err);
+	kvfree(in);
+	return err;
  }
 
-int mlx5_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_tbl)
+void mlx5_ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *ib_rwq_ind_tbl)
  {
  	struct mlx5_ib_rwq_ind_table *rwq_ind_tbl = to_mrwq_ind_table(ib_rwq_ind_tbl);
  	struct mlx5_ib_dev *dev = to_mdev(ib_rwq_ind_tbl->device);
 
  	mlx5_cmd_destroy_rqt(dev->mdev, rwq_ind_tbl->rqtn, rwq_ind_tbl->uid);
-
-	kfree(rwq_ind_tbl);
-	return 0;
  }
 
  int mlx5_ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,

include/rdma/ib_verbs.h +5 -6

index 6e2cb69fe90b..ed948946e443 100644
@@ -2526,11 +2526,10 @@ struct ib_device_ops {
  	void (*destroy_wq)(struct ib_wq *wq, struct ib_udata *udata);
  	int (*modify_wq)(struct ib_wq *wq, struct ib_wq_attr *attr,
  			 u32 wq_attr_mask, struct ib_udata *udata);
-	struct ib_rwq_ind_table *(*create_rwq_ind_table)(
-		struct ib_device *device,
-		struct ib_rwq_ind_table_init_attr *init_attr,
-		struct ib_udata *udata);
-	int (*destroy_rwq_ind_table)(struct ib_rwq_ind_table *wq_ind_table);
+	int (*create_rwq_ind_table)(struct ib_rwq_ind_table *ib_rwq_ind_table,
+				    struct ib_rwq_ind_table_init_attr *init_attr,
+				    u32 *ind_tbl_num, struct ib_udata *udata);
+	void (*destroy_rwq_ind_table)(struct ib_rwq_ind_table *wq_ind_table);
  	struct ib_dm *(*alloc_dm)(struct ib_device *device,
  				  struct ib_ucontext *context,
  				  struct ib_dm_alloc_attr *attr,

@@ -2653,6 +2652,7 @@ struct ib_device_ops {
  	DECLARE_RDMA_OBJ_SIZE(ib_cq);
  	DECLARE_RDMA_OBJ_SIZE(ib_mw);
  	DECLARE_RDMA_OBJ_SIZE(ib_pd);
+	DECLARE_RDMA_OBJ_SIZE(ib_rwq_ind_table);
  	DECLARE_RDMA_OBJ_SIZE(ib_srq);
  	DECLARE_RDMA_OBJ_SIZE(ib_ucontext);
  	DECLARE_RDMA_OBJ_SIZE(ib_xrcd);

@@ -4430,7 +4430,6 @@ struct ib_wq *ib_create_wq(struct ib_pd *pd,
  int ib_destroy_wq(struct ib_wq *wq, struct ib_udata *udata);
  int ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *attr,
  		 u32 wq_attr_mask);
-int ib_destroy_rwq_ind_table(struct ib_rwq_ind_table *wq_ind_table);
 
  int ib_map_mr_sg(struct ib_mr *mr, struct scatterlist *sg, int sg_nents,
  		 unsigned int *sg_offset, unsigned int page_size);

From: Leon Romanovsky <[email protected]>

Move allocation and destruction of memory windows under ib_core responsibility and clean drivers to ensure that no updates to MW ib_core structures are done in driver layer.

Signed-off-by Leon Romanovsky <[email protected]>

---
  drivers/infiniband/core/device.c            |  1 +
  drivers/infiniband/core/uverbs.h            |  2 +-
  drivers/infiniband/core/uverbs_cmd.c        | 26 ++++++++-----
  drivers/infiniband/core/uverbs_main.c       | 10 ++---
  drivers/infiniband/core/uverbs_std_types.c  |  3 +-
  drivers/infiniband/hw/cxgb4/iw_cxgb4.h      |  5 +--
  drivers/infiniband/hw/cxgb4/mem.c           | 35 ++++++-----------
  drivers/infiniband/hw/cxgb4/provider.c      |  4 +-
  drivers/infiniband/hw/hns/hns_roce_device.h |  5 +--
  drivers/infiniband/hw/hns/hns_roce_main.c   |  2 +
  drivers/infiniband/hw/hns/hns_roce_mr.c     | 31 +++++----------
  drivers/infiniband/hw/mlx4/main.c           |  2 +
  drivers/infiniband/hw/mlx4/mlx4_ib.h        |  5 +--
  drivers/infiniband/hw/mlx4/mr.c             | 32 +++++-----------
  drivers/infiniband/hw/mlx5/main.c           |  2 +
  drivers/infiniband/hw/mlx5/mlx5_ib.h        |  5 +--
  drivers/infiniband/hw/mlx5/mr.c             | 42 ++++++++-------------
  include/rdma/ib_verbs.h                     |  6 +--
  18 files changed, 91 insertions(+), 127 deletions(-)

drivers/infiniband/core/device.c +1 -0

index 088559d72d57..257e8a667977 100644
@@ -2684,6 +2684,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
  	SET_OBJ_SIZE(dev_ops, ib_ah);
  	SET_OBJ_SIZE(dev_ops, ib_counters);
  	SET_OBJ_SIZE(dev_ops, ib_cq);
+	SET_OBJ_SIZE(dev_ops, ib_mw);
  	SET_OBJ_SIZE(dev_ops, ib_pd);
  	SET_OBJ_SIZE(dev_ops, ib_srq);
  	SET_OBJ_SIZE(dev_ops, ib_ucontext);

drivers/infiniband/core/uverbs.h +1 -1

index 53a10479958b..072bfe4e1b5b 100644
@@ -242,7 +242,7 @@ int ib_uverbs_dealloc_xrcd(struct ib_uobject *uobject, struct ib_xrcd *xrcd,
  			   enum rdma_remove_reason why,
  			   struct uverbs_attr_bundle *attrs);
 
-int uverbs_dealloc_mw(struct ib_mw *mw);
+void uverbs_dealloc_mw(struct ib_mw *mw);
  void ib_uverbs_detach_umcast(struct ib_qp *qp,
  			     struct ib_uqp_object *uobj);
 

drivers/infiniband/core/uverbs_cmd.c +16 -10

index 68c9a0210220..a5301f3d3059 100644
@@ -890,7 +890,7 @@ static int ib_uverbs_dereg_mr(struct uverbs_attr_bundle *attrs)
  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_uverbs_alloc_mw_resp resp = {};
  	struct ib_uobject             *uobj;
  	struct ib_pd                  *pd;
  	struct ib_mw                  *mw;

@@ -916,21 +916,24 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)
  		goto err_put;
  	}
 
-	mw = pd->device->ops.alloc_mw(pd, cmd.mw_type, &attrs->driver_udata);
-	if (IS_ERR(mw)) {
-		ret = PTR_ERR(mw);
+	mw = rdma_zalloc_drv_obj(ib_dev, ib_mw);
+	if (!mw) {
+		ret = -ENOMEM;
  		goto err_put;
  	}
 
-	mw->device  = pd->device;
-	mw->pd      = pd;
+	mw->device = ib_dev;
+	mw->pd = pd;
  	mw->uobject = uobj;
-	atomic_inc(&pd->usecnt);
-
  	uobj->object = mw;
+	mw->type = cmd.mw_type;
 
-	memset(&resp, 0, sizeof(resp));
-	resp.rkey      = mw->rkey;
+	ret = pd->device->ops.alloc_mw(mw, &mw->rkey, &attrs->driver_udata);
+	if (ret)
+		goto err_alloc;
+
+	atomic_inc(&pd->usecnt);
+	resp.rkey = mw->rkey;
  	resp.mw_handle = uobj->id;
 
  	ret = uverbs_response(attrs, &resp, sizeof(resp));

@@ -943,6 +946,9 @@ static int ib_uverbs_alloc_mw(struct uverbs_attr_bundle *attrs)
 
  err_copy:
  	uverbs_dealloc_mw(mw);
+	mw = NULL;
+err_alloc:
+	kfree(mw);
  err_put:
  	uobj_put_obj_read(pd);
  err_free:

drivers/infiniband/core/uverbs_main.c +4 -6

index 69e4755cc04b..706c972ea3a1 100644
@@ -102,15 +102,13 @@ struct ib_ucontext *ib_uverbs_get_ucontext_file(struct ib_uverbs_file *ufile)
  }
  EXPORT_SYMBOL(ib_uverbs_get_ucontext_file);
 
-int uverbs_dealloc_mw(struct ib_mw *mw)
+void uverbs_dealloc_mw(struct ib_mw *mw)
  {
  	struct ib_pd *pd = mw->pd;
-	int ret;
 
-	ret = mw->device->ops.dealloc_mw(mw);
-	if (!ret)
-		atomic_dec(&pd->usecnt);
-	return ret;
+	mw->device->ops.dealloc_mw(mw);
+	atomic_dec(&pd->usecnt);
+	kfree(mw);
  }
 
  static void ib_uverbs_release_dev(struct device *device)

drivers/infiniband/core/uverbs_std_types.c +2 -1

index 08c39cfb1bd9..e4994cc4cc51 100644
@@ -72,7 +72,8 @@ static int uverbs_free_mw(struct ib_uobject *uobject,
  			  enum rdma_remove_reason why,
  			  struct uverbs_attr_bundle *attrs)
  {
-	return uverbs_dealloc_mw((struct ib_mw *)uobject->object);
+	uverbs_dealloc_mw((struct ib_mw *)uobject->object);
+	return 0;
  }
 
  static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,

drivers/infiniband/hw/cxgb4/iw_cxgb4.h +2 -3

index 27da0705c88a..5545bcd0043e 100644
@@ -983,10 +983,9 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
  			    u32 max_num_sg, struct ib_udata *udata);
  int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
  		   unsigned int *sg_offset);
-int c4iw_dealloc_mw(struct ib_mw *mw);
+void c4iw_dealloc_mw(struct ib_mw *mw);
  void c4iw_dealloc(struct uld_ctx *ctx);
-struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			    struct ib_udata *udata);
+int c4iw_alloc_mw(struct ib_mw *mw, u32 *rkey, struct ib_udata *udata);
  struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start,
  					   u64 length, u64 virt, int acc,
  					   struct ib_udata *udata);

drivers/infiniband/hw/cxgb4/mem.c +12 -23

index 1e4f4e525598..edb62d9cf404 100644
@@ -611,30 +611,23 @@ struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
  	return ERR_PTR(err);
  }
 
-struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			    struct ib_udata *udata)
+int c4iw_alloc_mw(struct ib_mw *ibmw, u32 *rkey, struct ib_udata *udata)
  {
+	struct c4iw_mw *mhp = to_c4iw_mw(ibmw);
  	struct c4iw_dev *rhp;
  	struct c4iw_pd *php;
-	struct c4iw_mw *mhp;
  	u32 mmid;
  	u32 stag = 0;
  	int ret;
 
-	if (type != IB_MW_TYPE_1)
-		return ERR_PTR(-EINVAL);
+	if (ibmw->type != IB_MW_TYPE_1)
+		return -EINVAL;
 
-	php = to_c4iw_pd(pd);
+	php = to_c4iw_pd(ibmw->pd);
  	rhp = php->rhp;
-	mhp = kzalloc(sizeof(*mhp), GFP_KERNEL);
-	if (!mhp)
-		return ERR_PTR(-ENOMEM);
-
  	mhp->wr_waitp = c4iw_alloc_wr_wait(GFP_KERNEL);
-	if (!mhp->wr_waitp) {
-		ret = -ENOMEM;
-		goto free_mhp;
-	}
+	if (!mhp->wr_waitp)
+		return -ENOMEM;
 
  	mhp->dereg_skb = alloc_skb(SGE_MAX_WR_LEN, GFP_KERNEL);
  	if (!mhp->dereg_skb) {

@@ -645,18 +638,19 @@ struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
  	ret = allocate_window(&rhp->rdev, &stag, php->pdid, mhp->wr_waitp);
  	if (ret)
  		goto free_skb;
+
  	mhp->rhp = rhp;
  	mhp->attr.pdid = php->pdid;
  	mhp->attr.type = FW_RI_STAG_MW;
  	mhp->attr.stag = stag;
  	mmid = (stag) >> 8;
-	mhp->ibmw.rkey = stag;
+	*rkey = stag;
  	if (xa_insert_irq(&rhp->mrs, mmid, mhp, GFP_KERNEL)) {
  		ret = -ENOMEM;
  		goto dealloc_win;
  	}
  	pr_debug("mmid 0x%x mhp %p stag 0x%x\n", mmid, mhp, stag);
-	return &(mhp->ibmw);
+	return 0;
 
  dealloc_win:
  	deallocate_window(&rhp->rdev, mhp->attr.stag, mhp->dereg_skb,

@@ -665,12 +659,10 @@ struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
  	kfree_skb(mhp->dereg_skb);
  free_wr_wait:
  	c4iw_put_wr_wait(mhp->wr_waitp);
-free_mhp:
-	kfree(mhp);
-	return ERR_PTR(ret);
+	return ret;
  }
 
-int c4iw_dealloc_mw(struct ib_mw *mw)
+void c4iw_dealloc_mw(struct ib_mw *mw)
  {
  	struct c4iw_dev *rhp;
  	struct c4iw_mw *mhp;

@@ -684,9 +676,6 @@ int c4iw_dealloc_mw(struct ib_mw *mw)
  			  mhp->wr_waitp);
  	kfree_skb(mhp->dereg_skb);
  	c4iw_put_wr_wait(mhp->wr_waitp);
-	pr_debug("ib_mw %p mmid 0x%x ptr %p\n", mw, mmid, mhp);
-	kfree(mhp);
-	return 0;
  }
 
  struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,

drivers/infiniband/hw/cxgb4/provider.c +3 -1

index 1d3ff59e4060..7ec74e2fa885 100644
@@ -508,8 +508,10 @@ static const struct ib_device_ops c4iw_dev_ops = {
  	.query_qp = c4iw_ib_query_qp,
  	.reg_user_mr = c4iw_reg_user_mr,
  	.req_notify_cq = c4iw_arm_cq,
-	INIT_RDMA_OBJ_SIZE(ib_pd, c4iw_pd, ibpd),
+
  	INIT_RDMA_OBJ_SIZE(ib_cq, c4iw_cq, ibcq),
+	INIT_RDMA_OBJ_SIZE(ib_mw, c4iw_mw, ibmw),
+	INIT_RDMA_OBJ_SIZE(ib_pd, c4iw_pd, ibpd),
  	INIT_RDMA_OBJ_SIZE(ib_srq, c4iw_srq, ibsrq),
  	INIT_RDMA_OBJ_SIZE(ib_ucontext, c4iw_ucontext, ibucontext),
  };

drivers/infiniband/hw/hns/hns_roce_device.h +2 -3

index b890ff63f24d..e745e5c41770 100644
@@ -1201,9 +1201,8 @@ int hns_roce_hw_destroy_mpt(struct hns_roce_dev *hr_dev,
  			    unsigned long mpt_index);
  unsigned long key_to_hw_index(u32 key);
 
-struct ib_mw *hns_roce_alloc_mw(struct ib_pd *pd, enum ib_mw_type,
-				struct ib_udata *udata);
-int hns_roce_dealloc_mw(struct ib_mw *ibmw);
+int hns_roce_alloc_mw(struct ib_mw *pd, u32 *rkey, struct ib_udata *udata);
+void hns_roce_dealloc_mw(struct ib_mw *ibmw);
 
  void hns_roce_buf_free(struct hns_roce_dev *hr_dev, struct hns_roce_buf *buf);
  int hns_roce_buf_alloc(struct hns_roce_dev *hr_dev, u32 size, u32 max_direct,

drivers/infiniband/hw/hns/hns_roce_main.c +2 -0

index 5907cfd878a6..45f7353bd348 100644
@@ -454,6 +454,8 @@ static const struct ib_device_ops hns_roce_dev_mr_ops = {
  static const struct ib_device_ops hns_roce_dev_mw_ops = {
  	.alloc_mw = hns_roce_alloc_mw,
  	.dealloc_mw = hns_roce_dealloc_mw,
+
+	INIT_RDMA_OBJ_SIZE(ib_mw, hns_roce_mw, ibmw),
  };
 
  static const struct ib_device_ops hns_roce_dev_frmr_ops = {

drivers/infiniband/hw/hns/hns_roce_mr.c +9 -22

index 0e71ebee9e52..4af2797c3b95 100644
@@ -589,28 +589,22 @@ static int hns_roce_mw_enable(struct hns_roce_dev *hr_dev,
  	return ret;
  }
 
-struct ib_mw *hns_roce_alloc_mw(struct ib_pd *ib_pd, enum ib_mw_type type,
-				struct ib_udata *udata)
+int hns_roce_alloc_mw(struct ib_mw *ibmw, u32 *rkey, struct ib_udata *udata)
  {
-	struct hns_roce_dev *hr_dev = to_hr_dev(ib_pd->device);
-	struct hns_roce_mw *mw;
+	struct hns_roce_dev *hr_dev = to_hr_dev(ibmw->device);
+	struct hns_roce_mw *mw = to_hr_mw(ibmw);
  	unsigned long index = 0;
  	int ret;
 
-	mw = kmalloc(sizeof(*mw), GFP_KERNEL);
-	if (!mw)
-		return ERR_PTR(-ENOMEM);
-
  	/* Allocate a key for mw from bitmap */
  	ret = hns_roce_bitmap_alloc(&hr_dev->mr_table.mtpt_bitmap, &index);
  	if (ret)
-		goto err_bitmap;
+		return ret;
 
  	mw->rkey = hw_index_to_key(index);
 
-	mw->ibmw.rkey = mw->rkey;
-	mw->ibmw.type = type;
-	mw->pdn = to_hr_pd(ib_pd)->pdn;
+	*rkey = mw->rkey;
+	mw->pdn = to_hr_pd(ibmw->pd)->pdn;
  	mw->pbl_hop_num = hr_dev->caps.pbl_hop_num;
  	mw->pbl_ba_pg_sz = hr_dev->caps.pbl_ba_pg_sz;
  	mw->pbl_buf_pg_sz = hr_dev->caps.pbl_buf_pg_sz;

@@ -619,26 +613,19 @@ struct ib_mw *hns_roce_alloc_mw(struct ib_pd *ib_pd, enum ib_mw_type type,
  	if (ret)
  		goto err_mw;
 
-	return &mw->ibmw;
+	return 0;
 
  err_mw:
  	hns_roce_mw_free(hr_dev, mw);
-
-err_bitmap:
-	kfree(mw);
-
-	return ERR_PTR(ret);
+	return ret;
  }
 
-int hns_roce_dealloc_mw(struct ib_mw *ibmw)
+void hns_roce_dealloc_mw(struct ib_mw *ibmw)
  {
  	struct hns_roce_dev *hr_dev = to_hr_dev(ibmw->device);
  	struct hns_roce_mw *mw = to_hr_mw(ibmw);
 
  	hns_roce_mw_free(hr_dev, mw);
-	kfree(mw);
-
-	return 0;
  }
 
  static int mtr_map_region(struct hns_roce_dev *hr_dev, struct hns_roce_mtr *mtr,

drivers/infiniband/hw/mlx4/main.c +2 -0

index 816d28854a8e..6471f47bd365 100644
@@ -2602,6 +2602,8 @@ static const struct ib_device_ops mlx4_ib_dev_wq_ops = {
  static const struct ib_device_ops mlx4_ib_dev_mw_ops = {
  	.alloc_mw = mlx4_ib_alloc_mw,
  	.dealloc_mw = mlx4_ib_dealloc_mw,
+
+	INIT_RDMA_OBJ_SIZE(ib_mw, mlx4_ib_mw, ibmw),
  };
 
  static const struct ib_device_ops mlx4_ib_dev_xrc_ops = {

drivers/infiniband/hw/mlx4/mlx4_ib.h +2 -3

index 6f4ea1067095..5fbe766aef6b 100644
@@ -725,9 +725,8 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
  				  u64 virt_addr, int access_flags,
  				  struct ib_udata *udata);
  int mlx4_ib_dereg_mr(struct ib_mr *mr, struct ib_udata *udata);
-struct ib_mw *mlx4_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			       struct ib_udata *udata);
-int mlx4_ib_dealloc_mw(struct ib_mw *mw);
+int mlx4_ib_alloc_mw(struct ib_mw *mw, u32 *rkey, struct ib_udata *udata);
+void mlx4_ib_dealloc_mw(struct ib_mw *mw);
  struct ib_mr *mlx4_ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
  			       u32 max_num_sg, struct ib_udata *udata);
  int mlx4_ib_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,

drivers/infiniband/hw/mlx4/mr.c +10 -22

index d7c78f841d2f..82d7d8dcc9d4 100644
@@ -610,47 +610,35 @@ int mlx4_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
  	return 0;
  }
 
-struct ib_mw *mlx4_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			       struct ib_udata *udata)
+int mlx4_ib_alloc_mw(struct ib_mw *ibmw, u32 *rkey, struct ib_udata *udata)
  {
-	struct mlx4_ib_dev *dev = to_mdev(pd->device);
-	struct mlx4_ib_mw *mw;
+	struct mlx4_ib_dev *dev = to_mdev(ibmw->device);
+	struct mlx4_ib_mw *mw = to_mmw(ibmw);
  	int err;
 
-	mw = kmalloc(sizeof(*mw), GFP_KERNEL);
-	if (!mw)
-		return ERR_PTR(-ENOMEM);
-
-	err = mlx4_mw_alloc(dev->dev, to_mpd(pd)->pdn,
-			    to_mlx4_type(type), &mw->mmw);
+	err = mlx4_mw_alloc(dev->dev, to_mpd(ibmw->pd)->pdn,
+			    to_mlx4_type(ibmw->type), &mw->mmw);
  	if (err)
-		goto err_free;
+		return err;
 
  	err = mlx4_mw_enable(dev->dev, &mw->mmw);
  	if (err)
  		goto err_mw;
 
-	mw->ibmw.rkey = mw->mmw.key;
+	*rkey = mw->mmw.key;
 
-	return &mw->ibmw;
+	return 0;
 
  err_mw:
  	mlx4_mw_free(dev->dev, &mw->mmw);
-
-err_free:
-	kfree(mw);
-
-	return ERR_PTR(err);
+	return err;
  }
 
-int mlx4_ib_dealloc_mw(struct ib_mw *ibmw)
+void mlx4_ib_dealloc_mw(struct ib_mw *ibmw)
  {
  	struct mlx4_ib_mw *mw = to_mmw(ibmw);
 
  	mlx4_mw_free(to_mdev(ibmw->device)->dev, &mw->mmw);
-	kfree(mw);
-
-	return 0;
  }
 
  struct ib_mr *mlx4_ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,

drivers/infiniband/hw/mlx5/main.c +2 -0

index ca35394a181a..8b0b1f95af2f 100644
@@ -6669,6 +6669,8 @@ static const struct ib_device_ops mlx5_ib_dev_sriov_ops = {
  static const struct ib_device_ops mlx5_ib_dev_mw_ops = {
  	.alloc_mw = mlx5_ib_alloc_mw,
  	.dealloc_mw = mlx5_ib_dealloc_mw,
+
+	INIT_RDMA_OBJ_SIZE(ib_mw, mlx5_ib_mw, ibmw),
  };
 
  static const struct ib_device_ops mlx5_ib_dev_xrc_ops = {

drivers/infiniband/hw/mlx5/mlx5_ib.h +2 -3

index 26545e88709d..d84f56517caa 100644
@@ -1195,9 +1195,8 @@ int mlx5_ib_advise_mr(struct ib_pd *pd,
  		      struct ib_sge *sg_list,
  		      u32 num_sge,
  		      struct uverbs_attr_bundle *attrs);
-struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			       struct ib_udata *udata);
-int mlx5_ib_dealloc_mw(struct ib_mw *mw);
+int mlx5_ib_alloc_mw(struct ib_mw *mw, u32 *rkey, struct ib_udata *udata);
+void mlx5_ib_dealloc_mw(struct ib_mw *mw);
  int mlx5_ib_update_xlt(struct mlx5_ib_mr *mr, u64 idx, int npages,
  		       int page_shift, int flags);
  struct mlx5_ib_mr *mlx5_ib_alloc_implicit_mr(struct mlx5_ib_pd *pd,

drivers/infiniband/hw/mlx5/mr.c +15 -21

index 44683073be0c..58a3d62937a1 100644
@@ -1973,12 +1973,11 @@ struct ib_mr *mlx5_ib_alloc_mr_integrity(struct ib_pd *pd,
  				  max_num_meta_sg);
  }
 
-struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
-			       struct ib_udata *udata)
+int mlx5_ib_alloc_mw(struct ib_mw *ibmw, u32 *rkey, struct ib_udata *udata)
  {
-	struct mlx5_ib_dev *dev = to_mdev(pd->device);
+	struct mlx5_ib_dev *dev = to_mdev(ibmw->device);
  	int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
-	struct mlx5_ib_mw *mw = NULL;
+	struct mlx5_ib_mw *mw = to_mmw(ibmw);
  	u32 *in = NULL;
  	void *mkc;
  	int ndescs;

@@ -1991,21 +1990,20 @@ struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
 
  	err = ib_copy_from_udata(&req, udata, min(udata->inlen, sizeof(req)));
  	if (err)
-		return ERR_PTR(err);
+		return err;
 
  	if (req.comp_mask || req.reserved1 || req.reserved2)
-		return ERR_PTR(-EOPNOTSUPP);
+		return -EOPNOTSUPP;
 
  	if (udata->inlen > sizeof(req) &&
  	    !ib_is_udata_cleared(udata, sizeof(req),
  				 udata->inlen - sizeof(req)))
-		return ERR_PTR(-EOPNOTSUPP);
+		return -EOPNOTSUPP;
 
  	ndescs = req.num_klms ? roundup(req.num_klms, 4) : roundup(1, 4);
 
-	mw = kzalloc(sizeof(*mw), GFP_KERNEL);
  	in = kzalloc(inlen, GFP_KERNEL);
-	if (!mw || !in) {
+	if (!in) {
  		err = -ENOMEM;
  		goto free;
  	}

@@ -2014,11 +2012,11 @@ struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
 
  	MLX5_SET(mkc, mkc, free, 1);
  	MLX5_SET(mkc, mkc, translations_octword_size, ndescs);
-	MLX5_SET(mkc, mkc, pd, to_mpd(pd)->pdn);
+	MLX5_SET(mkc, mkc, pd, to_mpd(ibmw->pd)->pdn);
  	MLX5_SET(mkc, mkc, umr_en, 1);
  	MLX5_SET(mkc, mkc, lr, 1);
  	MLX5_SET(mkc, mkc, access_mode_1_0, MLX5_MKC_ACCESS_MODE_KLMS);
-	MLX5_SET(mkc, mkc, en_rinval, !!((type == IB_MW_TYPE_2)));
+	MLX5_SET(mkc, mkc, en_rinval, !!((ibmw->type == IB_MW_TYPE_2)));
  	MLX5_SET(mkc, mkc, qpn, 0xffffff);
 
  	err = mlx5_ib_create_mkey(dev, &mw->mmkey, in, inlen);

@@ -2026,17 +2024,15 @@ struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
  		goto free;
 
  	mw->mmkey.type = MLX5_MKEY_MW;
-	mw->ibmw.rkey = mw->mmkey.key;
+	*rkey = mw->mmkey.key;
  	mw->ndescs = ndescs;
 
  	resp.response_length = min(offsetof(typeof(resp), response_length) +
  				   sizeof(resp.response_length), udata->outlen);
  	if (resp.response_length) {
  		err = ib_copy_to_udata(udata, &resp, resp.response_length);
-		if (err) {
-			mlx5_core_destroy_mkey(dev->mdev, &mw->mmkey);
-			goto free;
-		}
+		if (err)
+			goto free_mkey;
  	}
 
  	if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING)) {

@@ -2048,21 +2044,19 @@ struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
  	}
 
  	kfree(in);
-	return &mw->ibmw;
+	return 0;
 
  free_mkey:
  	mlx5_core_destroy_mkey(dev->mdev, &mw->mmkey);
  free:
-	kfree(mw);
  	kfree(in);
-	return ERR_PTR(err);
+	return err;
  }
 
-int mlx5_ib_dealloc_mw(struct ib_mw *mw)
+void mlx5_ib_dealloc_mw(struct ib_mw *mw)
  {
  	struct mlx5_ib_dev *dev = to_mdev(mw->device);
  	struct mlx5_ib_mw *mmw = to_mmw(mw);
-	int err;
 
  	if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING)) {
  		xa_erase(&dev->odp_mkeys, mlx5_base_mkey(mmw->mmkey.key));

drivers/infiniband/hw/mlx5/mr.c +0 -0

  }
 
  int mlx5_ib_check_mr_status(struct ib_mr *ibmr, u32 check_mask,

include/rdma/ib_verbs.h +3 -3

index 3aa20ce23556..7160662899bb 100644
@@ -2490,9 +2490,8 @@ struct ib_device_ops {
  			 unsigned int *sg_offset);
  	int (*check_mr_status)(struct ib_mr *mr, u32 check_mask,
  			       struct ib_mr_status *mr_status);
-	struct ib_mw *(*alloc_mw)(struct ib_pd *pd, enum ib_mw_type type,
-				  struct ib_udata *udata);
-	int (*dealloc_mw)(struct ib_mw *mw);
+	int (*alloc_mw)(struct ib_mw *mw, u32 *rkey, struct ib_udata *udata);
+	void (*dealloc_mw)(struct ib_mw *mw);
  	int (*attach_mcast)(struct ib_qp *qp, union ib_gid *gid, u16 lid);
  	int (*detach_mcast)(struct ib_qp *qp, union ib_gid *gid, u16 lid);
  	struct ib_xrcd *(*alloc_xrcd)(struct ib_device *device,

@@ -2653,6 +2652,7 @@ struct ib_device_ops {
  	DECLARE_RDMA_OBJ_SIZE(ib_ah);
  	DECLARE_RDMA_OBJ_SIZE(ib_counters);
  	DECLARE_RDMA_OBJ_SIZE(ib_cq);
+	DECLARE_RDMA_OBJ_SIZE(ib_mw);
  	DECLARE_RDMA_OBJ_SIZE(ib_pd);
  	DECLARE_RDMA_OBJ_SIZE(ib_srq);
  	DECLARE_RDMA_OBJ_SIZE(ib_ucontext);

Quoted 124 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
  drivers/infiniband/core/device.c           |  1 +
  drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
  drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
  drivers/infiniband/core/verbs.c            | 25 +------------
  drivers/infiniband/hw/mlx4/main.c          |  4 +-
  drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
  drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
  drivers/infiniband/hw/mlx5/main.c          |  3 ++
  drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
  drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
  include/rdma/ib_verbs.h                    | 11 +++---
  11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
  	SET_OBJ_SIZE(dev_ops, ib_cq);
  	SET_OBJ_SIZE(dev_ops, ib_mw);
  	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
  	SET_OBJ_SIZE(dev_ops, ib_srq);
  	SET_OBJ_SIZE(dev_ops, ib_ucontext);
  	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  {
  	struct ib_uverbs_ex_create_rwq_ind_table cmd;
  	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
  	int err;
  	struct ib_rwq_ind_table_init_attr init_attr = {};
  	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
  	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
  	int i, j, num_read_wqs;
  	u32 num_wq_handles;
  	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  		goto put_wqs;
  	}
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
  		goto err_uobj;
  	}
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
  	rwq_ind_tbl->ind_tbl = wqs;
  	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
  	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  	rwq_ind_tbl->device = ib_dev;
  	atomic_set(&rwq_ind_tbl->usecnt, 0);
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
  	for (i = 0; i < num_wq_handles; i++)
  		atomic_inc(&wqs[i]->usecnt);

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
  	return 0;
  err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
  err_uobj:
  	uobj_alloc_abort(uobj, attrs);
  put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
  			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
  			ib_uverbs_ex_destroy_rwq_ind_table,
  			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
  	DECLARE_UVERBS_OBJECT(
  		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +5 -2

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
  {
  	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
  	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+
+	if (atomic_read(&rwq_ind_tbl->usecnt))
+		ret = -EBUSY;
-	ret = ib_destroy_rwq_ind_table(rwq_ind_tbl);
  	if (ib_is_destroy_retryable(ret, why, uobject))

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 96

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +3 -0

  		return ret;
+	for (i = 0; i < table_size; i++)
+		atomic_dec(&ind_tbl[i]->usecnt);
+

Upon destroy we first expect to destroy the object that is pointing to other objetcs and only then descraese their ref count, this was the orignal logic here. (See also ib_destroy_qp_user(), ib_destroy_wq() and others).

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 98

Quoted 3 lines

drivers/infiniband/core/uverbs_std_types.c +2 -0

+	if (rwq_ind_tbl->device->ops.destroy_rwq_ind_table)
+		rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

This doesn't really make sense, if no destroy function was set, the allocation should be prevented as well.

Quoted 8 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move allocation and destruction of memory windows under ib_core responsibility and clean drivers to ensure that no updates to MW ib_core structures are done in driver layer.

Signed-off-by Leon Romanovsky <[email protected]>

---

Quoted 12 lines

drivers/infiniband/hw/mlx5/mr.c +1 -3

+void mlx5_ib_dealloc_mw(struct ib_mw *mw)
  {
  	struct mlx5_ib_dev *dev = to_mdev(mw->device);
  	struct mlx5_ib_mw *mmw = to_mmw(mw);
-	int err;
  	if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING)) {
  		xa_erase(&dev->odp_mkeys, mlx5_base_mkey(mmw->mmkey.key));

@@ -2073,11 +2067,7 @@ int mlx5_ib_dealloc_mw(struct ib_mw *mw)
  		synchronize_srcu(&dev->odp_srcu);
  	}
-	err = mlx5_core_destroy_mkey(dev->mdev, &mmw->mmkey);
-	if (err)

drivers/infiniband/hw/mlx5/mr.c int mlx5_ib_dealloc_mw(struct ib_mw *mw) · line 2070

Quoted 4 lines

drivers/infiniband/hw/mlx5/mr.c +1 -3

-		return err;
-	kfree(mmw);
-	return 0;
+	mlx5_core_destroy_mkey(dev->mdev, &mmw->mmkey);

Are we fully sure that this can never be triggered by user space to fail as of the property of the MW that can be binded with bypassing kernel ? the new code just ignored the err.

Quoted 125 lines · Yishai Hadas

Quoted 124 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
   drivers/infiniband/core/device.c           |  1 +
   drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
   drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
   drivers/infiniband/core/verbs.c            | 25 +------------
   drivers/infiniband/hw/mlx4/main.c          |  4 +-
   drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
   drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
   drivers/infiniband/hw/mlx5/main.c          |  3 ++
   drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
   drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
   include/rdma/ib_verbs.h                    | 11 +++---
   11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
   	SET_OBJ_SIZE(dev_ops, ib_cq);
   	SET_OBJ_SIZE(dev_ops, ib_mw);
   	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
   	SET_OBJ_SIZE(dev_ops, ib_srq);
   	SET_OBJ_SIZE(dev_ops, ib_ucontext);
   	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   {
   	struct ib_uverbs_ex_create_rwq_ind_table cmd;
   	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
   	int err;
   	struct ib_rwq_ind_table_init_attr init_attr = {};
   	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
   	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
   	int i, j, num_read_wqs;
   	u32 num_wq_handles;
   	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   		goto put_wqs;
   	}
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
   		goto err_uobj;
   	}
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
   	rwq_ind_tbl->ind_tbl = wqs;
   	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
   	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   	rwq_ind_tbl->device = ib_dev;
   	atomic_set(&rwq_ind_tbl->usecnt, 0);
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
   	for (i = 0; i < num_wq_handles; i++)
   		atomic_inc(&wqs[i]->usecnt);

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   	return 0;
   err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
   err_uobj:
   	uobj_alloc_abort(uobj, attrs);
   put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
   			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
   			ib_uverbs_ex_destroy_rwq_ind_table,
   			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
   	DECLARE_UVERBS_OBJECT(
   		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +5 -2

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
   {
   	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
   	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+
+	if (atomic_read(&rwq_ind_tbl->usecnt))
+		ret = -EBUSY;
-	ret = ib_destroy_rwq_ind_table(rwq_ind_tbl);
   	if (ib_is_destroy_retryable(ret, why, uobject))

Quoted 8 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 96

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +3 -0

   		return ret;
+	for (i = 0; i < table_size; i++)
+		atomic_dec(&ind_tbl[i]->usecnt);
+

Upon destroy we first expect to destroy the object that is pointing to other objetcs and only then descraese their ref count, this was the orignal logic here. (See also ib_destroy_qp_user(), ib_destroy_wq() and others).

It is a bug, proper logic is to allocate HW object, elevate reference counting, decrease reference counter and deallocate HW object.

Everything else should be fixed too.

Quoted 7 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 98

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +2 -0

+	if (rwq_ind_tbl->device->ops.destroy_rwq_ind_table)
+		rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

This doesn't really make sense, if no destroy function was set, the allocation should be prevented as well.

Should we delete mlx4 RWQ implementation too?
After this refactoring, mlx4 won't have .destroy_rwq_ind_table.

Quoted 135 lines · Leon Romanovsky

Quoted 125 lines · Yishai Hadas

Quoted 124 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
   drivers/infiniband/core/device.c           |  1 +
   drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
   drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
   drivers/infiniband/core/verbs.c            | 25 +------------
   drivers/infiniband/hw/mlx4/main.c          |  4 +-
   drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
   drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
   drivers/infiniband/hw/mlx5/main.c          |  3 ++
   drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
   drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
   include/rdma/ib_verbs.h                    | 11 +++---
   11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
   	SET_OBJ_SIZE(dev_ops, ib_cq);
   	SET_OBJ_SIZE(dev_ops, ib_mw);
   	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
   	SET_OBJ_SIZE(dev_ops, ib_srq);
   	SET_OBJ_SIZE(dev_ops, ib_ucontext);
   	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   {
   	struct ib_uverbs_ex_create_rwq_ind_table cmd;
   	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
   	int err;
   	struct ib_rwq_ind_table_init_attr init_attr = {};
   	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
   	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
   	int i, j, num_read_wqs;
   	u32 num_wq_handles;
   	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   		goto put_wqs;
   	}
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
   		goto err_uobj;
   	}
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
   	rwq_ind_tbl->ind_tbl = wqs;
   	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
   	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   	rwq_ind_tbl->device = ib_dev;
   	atomic_set(&rwq_ind_tbl->usecnt, 0);
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
   	for (i = 0; i < num_wq_handles; i++)
   		atomic_inc(&wqs[i]->usecnt);

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
   	return 0;
   err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
   err_uobj:
   	uobj_alloc_abort(uobj, attrs);
   put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
   			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
   			ib_uverbs_ex_destroy_rwq_ind_table,
   			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
   	DECLARE_UVERBS_OBJECT(
   		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +5 -2

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
   {
   	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
   	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+
+	if (atomic_read(&rwq_ind_tbl->usecnt))
+		ret = -EBUSY;
-	ret = ib_destroy_rwq_ind_table(rwq_ind_tbl);
   	if (ib_is_destroy_retryable(ret, why, uobject))

Quoted 8 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 96

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +3 -0

   		return ret;
+	for (i = 0; i < table_size; i++)
+		atomic_dec(&ind_tbl[i]->usecnt);
+

Upon destroy we first expect to destroy the object that is pointing to other objetcs and only then descraese their ref count, this was the orignal logic here. (See also ib_destroy_qp_user(), ib_destroy_wq() and others).

Quoted 3 lines

It is a bug, proper logic is to allocate HW object, elevate reference counting, decrease reference counter and deallocate HW object.

I don't agree, only upon a succesfull destroy the we can go a ahead and safely decraese ref count from the pointed objects. This pevents any race from parallel desctrcution of the pointed object and consider any potenaial failure on the original object before decaresing ref counts as of in the QP case.

Quoted 2 lines

Everything else should be fixed too.

I beleive that this patch should be fixed and not all other places. By the way, see your previous patch from this series regarding MW that follows same logic, first dealloc_mw() and only then does atomic_dec() on the pointed PD.

Quoted 9 lines

Quoted 8 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 98

Quoted 5 lines

drivers/infiniband/core/uverbs_std_types.c +2 -0

+	if (rwq_ind_tbl->device->ops.destroy_rwq_ind_table)
+		rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

This doesn't really make sense, if no destroy function was set, the allocation should be prevented as well.

Quoted 3 lines

Should we delete mlx4 RWQ implementation too?
After this refactoring, mlx4 won't have .destroy_rwq_ind_table.

We can expect from symetric reasons to have always alloc/destroy from drivers point of view. The IB layer will not check whether the destroy function exist but will always call it, the driver in its turn may do nothing.

Yishai

Quoted 140 lines · Yishai Hadas

Quoted 138 lines · Leon Romanovsky

Quoted 125 lines · Yishai Hadas

Quoted 124 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
    drivers/infiniband/core/device.c           |  1 +
    drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
    drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
    drivers/infiniband/core/verbs.c            | 25 +------------
    drivers/infiniband/hw/mlx4/main.c          |  4 +-
    drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
    drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
    drivers/infiniband/hw/mlx5/main.c          |  3 ++
    drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
    drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
    include/rdma/ib_verbs.h                    | 11 +++---
    11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
    	SET_OBJ_SIZE(dev_ops, ib_cq);
    	SET_OBJ_SIZE(dev_ops, ib_mw);
    	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
    	SET_OBJ_SIZE(dev_ops, ib_srq);
    	SET_OBJ_SIZE(dev_ops, ib_ucontext);
    	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    {
    	struct ib_uverbs_ex_create_rwq_ind_table cmd;
    	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
    	int err;
    	struct ib_rwq_ind_table_init_attr init_attr = {};
    	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
    	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
    	int i, j, num_read_wqs;
    	u32 num_wq_handles;
    	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    		goto put_wqs;
    	}
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
    		goto err_uobj;
    	}
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
    	rwq_ind_tbl->ind_tbl = wqs;
    	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
    	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    	rwq_ind_tbl->device = ib_dev;
    	atomic_set(&rwq_ind_tbl->usecnt, 0);
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
    	for (i = 0; i < num_wq_handles; i++)
    		atomic_inc(&wqs[i]->usecnt);

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    	return 0;
    err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
    err_uobj:
    	uobj_alloc_abort(uobj, attrs);
    put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
    			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
    			ib_uverbs_ex_destroy_rwq_ind_table,
    			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
    	DECLARE_UVERBS_OBJECT(
    		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +5 -2

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
    {
    	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
    	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+
+	if (atomic_read(&rwq_ind_tbl->usecnt))
+		ret = -EBUSY;
-	ret = ib_destroy_rwq_ind_table(rwq_ind_tbl);
    	if (ib_is_destroy_retryable(ret, why, uobject))

Quoted 8 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 96

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +3 -0

    		return ret;
+	for (i = 0; i < table_size; i++)
+		atomic_dec(&ind_tbl[i]->usecnt);
+

Upon destroy we first expect to destroy the object that is pointing to other objetcs and only then descraese their ref count, this was the orignal logic here. (See also ib_destroy_qp_user(), ib_destroy_wq() and others).

It is a bug, proper logic is to allocate HW object, elevate reference counting, decrease reference counter and deallocate HW object.

Quoted 5 lines

I don't agree, only upon a succesfull destroy the we can go a ahead and safely decraese ref count from the pointed objects. This pevents any race from parallel desctrcution of the pointed object and consider any potenaial failure on the original object before decaresing ref counts as of in the QP case.

There is no such thing successful destroy, from user perspective ALL valid destroys should success.

Users don't know how to recover from
failure at this stage and they have one of two options: reboot the server or leave memory/HW leak.

There are not supposed to be parallel destructions if core code locked everything properly and one of the threads decreased reference counter to zero.

Can you please be more specific and give the scenario of your potential flow?

Quoted 7 lines

Quoted 2 lines

Everything else should be fixed too.

I beleive that this patch should be fixed and not all other places. By the way, see your previous patch from this series regarding MW that follows same logic, first dealloc_mw() and only then does atomic_dec() on the pointed PD.

Of course, it was intended, it is completely different case and flow. PD holds MW and this is why the reference counter is decreased only after MW was destroyed.

Quoted 14 lines

Quoted 13 lines

Quoted 9 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 98

Quoted 6 lines

drivers/infiniband/core/uverbs_std_types.c +2 -0

+	if (rwq_ind_tbl->device->ops.destroy_rwq_ind_table)
+		rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

This doesn't really make sense, if no destroy function was set, the allocation should be prevented as well.

Should we delete mlx4 RWQ implementation too?
After this refactoring, mlx4 won't have .destroy_rwq_ind_table.

Quoted 4 lines

We can expect from symetric reasons to have always alloc/destroy from drivers point of view. The IB layer will not check whether the destroy function exist but will always call it, the driver in its turn may do nothing.

Maybe, I don't like empty functions in the drivers, it is silly.

Quoted 147 lines · Leon Romanovsky

Quoted 145 lines · Yishai Hadas

Quoted 135 lines · Leon Romanovsky

Quoted 125 lines · Yishai Hadas

Quoted 124 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Move struct ib_rwq_ind_table allocation to ib_core.

Signed-off-by Leon Romanovsky <[email protected]>

---
    drivers/infiniband/core/device.c           |  1 +
    drivers/infiniband/core/uverbs_cmd.c       | 37 ++++++++++++-------
    drivers/infiniband/core/uverbs_std_types.c | 13 ++++++-
    drivers/infiniband/core/verbs.c            | 25 +------------
    drivers/infiniband/hw/mlx4/main.c          |  4 +-
    drivers/infiniband/hw/mlx4/mlx4_ib.h       | 12 +++---
    drivers/infiniband/hw/mlx4/qp.c            | 40 ++++++--------------
    drivers/infiniband/hw/mlx5/main.c          |  3 ++
    drivers/infiniband/hw/mlx5/mlx5_ib.h       |  8 ++--
    drivers/infiniband/hw/mlx5/qp.c            | 43 +++++++++-------------
    include/rdma/ib_verbs.h                    | 11 +++---
    11 files changed, 86 insertions(+), 111 deletions(-)

drivers/infiniband/core/device.c +1 -0

index b15fa3fa09ac..85d921c8e2b5 100644
@@ -2686,6 +2686,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
    	SET_OBJ_SIZE(dev_ops, ib_cq);
    	SET_OBJ_SIZE(dev_ops, ib_mw);
    	SET_OBJ_SIZE(dev_ops, ib_pd);
+	SET_OBJ_SIZE(dev_ops, ib_rwq_ind_table);
    	SET_OBJ_SIZE(dev_ops, ib_srq);
    	SET_OBJ_SIZE(dev_ops, ib_ucontext);
    	SET_OBJ_SIZE(dev_ops, ib_xrcd);

drivers/infiniband/core/uverbs_cmd.c +23 -14

index a5301f3d3059..a83d11d1e3ee 100644
@@ -3090,13 +3090,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    {
    	struct ib_uverbs_ex_create_rwq_ind_table cmd;
    	struct ib_uverbs_ex_create_rwq_ind_table_resp  resp = {};
-	struct ib_uobject		  *uobj;
+	struct ib_uobject *uobj;
    	int err;
    	struct ib_rwq_ind_table_init_attr init_attr = {};
    	struct ib_rwq_ind_table *rwq_ind_tbl;
-	struct ib_wq	**wqs = NULL;
+	struct ib_wq **wqs = NULL;
    	u32 *wqs_handles = NULL;
-	struct ib_wq	*wq = NULL;
+	struct ib_wq *wq = NULL;
    	int i, j, num_read_wqs;
    	u32 num_wq_handles;
    	struct uverbs_req_iter iter;

@@ -3151,17 +3151,15 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    		goto put_wqs;
    	}
-	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
-	init_attr.ind_tbl = wqs;
-
-	rwq_ind_tbl = ib_dev->ops.create_rwq_ind_table(ib_dev, &init_attr,
-						       &attrs->driver_udata);
-
-	if (IS_ERR(rwq_ind_tbl)) {
-		err = PTR_ERR(rwq_ind_tbl);
+	rwq_ind_tbl = rdma_zalloc_drv_obj(ib_dev, ib_rwq_ind_table);
+	if (!rwq_ind_tbl) {
+		err = -ENOMEM;
    		goto err_uobj;
    	}
+	init_attr.log_ind_tbl_size = cmd.log_ind_tbl_size;
+	init_attr.ind_tbl = wqs;
+
    	rwq_ind_tbl->ind_tbl = wqs;
    	rwq_ind_tbl->log_ind_tbl_size = init_attr.log_ind_tbl_size;
    	rwq_ind_tbl->uobject = uobj;

@@ -3169,6 +3167,12 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    	rwq_ind_tbl->device = ib_dev;
    	atomic_set(&rwq_ind_tbl->usecnt, 0);
+	err = ib_dev->ops.create_rwq_ind_table(rwq_ind_tbl, &init_attr,
+					       &rwq_ind_tbl->ind_tbl_num,
+					       &attrs->driver_udata);
+	if (err)
+		goto err_create;
+
    	for (i = 0; i < num_wq_handles; i++)
    		atomic_inc(&wqs[i]->usecnt);

@@ -3190,7 +3194,13 @@ static int ib_uverbs_ex_create_rwq_ind_table(struct uverbs_attr_bundle *attrs)
    	return 0;
    err_copy:
-	ib_destroy_rwq_ind_table(rwq_ind_tbl);
+	for (i = 0; i < num_wq_handles; i++)
+		atomic_dec(&wqs[i]->usecnt);
+
+	if (ib_dev->ops.destroy_rwq_ind_table)
+		ib_dev->ops.destroy_rwq_ind_table(rwq_ind_tbl);
+err_create:
+	kfree(rwq_ind_tbl);
    err_uobj:
    	uobj_alloc_abort(uobj, attrs);
    put_wqs:

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
    			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
    			ib_uverbs_ex_destroy_rwq_ind_table,
    			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),
    	DECLARE_UVERBS_OBJECT(
    		UVERBS_OBJECT_WQ,

drivers/infiniband/core/uverbs_std_types.c +5 -2

index e4994cc4cc51..cd82a5a80bdf 100644
@@ -82,12 +82,21 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
    {
    	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
    	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
-	int ret;
+	u32 table_size = (1 << rwq_ind_tbl->log_ind_tbl_size);
+	int ret = 0, i;
+
+	if (atomic_read(&rwq_ind_tbl->usecnt))
+		ret = -EBUSY;
-	ret = ib_destroy_rwq_ind_table(rwq_ind_tbl);
    	if (ib_is_destroy_retryable(ret, why, uobject))

Quoted 8 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 96

Quoted 4 lines

drivers/infiniband/core/uverbs_std_types.c +3 -0

    		return ret;
+	for (i = 0; i < table_size; i++)
+		atomic_dec(&ind_tbl[i]->usecnt);
+

Upon destroy we first expect to destroy the object that is pointing to other objetcs and only then descraese their ref count, this was the orignal logic here. (See also ib_destroy_qp_user(), ib_destroy_wq() and others).

Quoted 3 lines

It is a bug, proper logic is to allocate HW object, elevate reference counting, decrease reference counter and deallocate HW object.

I don't agree, only upon a succesfull destroy the we can go a ahead and safely decraese ref count from the pointed objects. This pevents any race from parallel desctrcution of the pointed object and consider any potenaial failure on the original object before decaresing ref counts as of in the QP case.

Quoted 2 lines

There is no such thing successful destroy, from user perspective ALL valid destroys should success.

This is exactly the ref count purpsoe to prevent calling destoy on an object that may be pointed by other one, otherwise it will fail in the driver / FW.

Quoted 9 lines

Users don't know how to recover from
failure at this stage and they have one of two options: reboot the server or leave memory/HW leak.

There are not supposed to be parallel destructions if core code locked everything properly and one of the threads decreased reference counter to zero.

Right, this should be achived by a proer ref counting odering as done today.

Quoted 4 lines

Can you please be more specific and give the scenario of your potential flow?

The above clearly explain the motivation for current ref count usage.

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

Everything else should be fixed too.

I beleive that this patch should be fixed and not all other places. By the way, see your previous patch from this series regarding MW that follows same logic, first dealloc_mw() and only then does atomic_dec() on the pointed PD.

Quoted 2 lines

Of course, it was intended, it is completely different case and flow. PD holds MW and this is why the reference counter is decreased only after MW was destroyed.

PD doesn't hold MW, MW points to a PD upon its creation as of QP, MR and othres.

Quoted 20 lines

Quoted 19 lines

Quoted 5 lines

Quoted 5 lines

Quoted 9 lines

Quoted 5 lines

drivers/infiniband/core/uverbs_std_types.c static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, · line 98

Quoted 2 lines

drivers/infiniband/core/uverbs_std_types.c +2 -0

+	if (rwq_ind_tbl->device->ops.destroy_rwq_ind_table)
+		rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

This doesn't really make sense, if no destroy function was set, the allocation should be prevented as well.

Should we delete mlx4 RWQ implementation too?
After this refactoring, mlx4 won't have .destroy_rwq_ind_table.

We can expect from symetric reasons to have always alloc/destroy from drivers point of view. The IB layer will not check whether the destroy function exist but will always call it, the driver in its turn may do nothing.

Quoted 2 lines

Maybe, I don't like empty functions in the drivers, it is silly.

We can consier that point, let's get other people feedback as well.

Yishai

drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4031

Quoted 7 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_cmd.c +1 -2

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
 			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
 			ib_uverbs_ex_destroy_rwq_ind_table,
 			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),

Removing these is kind of troublesome.. This misses the one for ioctl:

        UAPI_DEF_CHAIN_OBJ_TREE_NAMED(
                UVERBS_OBJECT_RWQ_IND_TBL,
                UAPI_DEF_OBJ_NEEDS_FN(destroy_rwq_ind_table)),

drivers/infiniband/core/verbs.c static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr, · line 1706

Quoted 10 lines

drivers/infiniband/core/verbs.c +2 -1

index 65c9118a931c..4210f0842bc6 100644
+++ b/drivers/infiniband/core/verbs.c

@@ -1703,7 +1703,7 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
 					  &old_sgid_attr_alt_av);
 		if (ret)
 			goto out_av;
-
+//

??

Jason

Quoted 11 lines · Jason Gunthorpe

drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4031

Quoted 7 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_cmd.c +1 -2

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
 			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
 			ib_uverbs_ex_destroy_rwq_ind_table,
 			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),

Removing these is kind of troublesome.. This misses the one for ioctl:

Quoted 3 lines

        UAPI_DEF_CHAIN_OBJ_TREE_NAMED(
                UVERBS_OBJECT_RWQ_IND_TBL,
                UAPI_DEF_OBJ_NEEDS_FN(destroy_rwq_ind_table)),

I will remove, but it seems that we have some gap here, I would expect any sort of compilation error for mlx4.

Quoted 12 lines · Leon Romanovsky

Quoted 11 lines · Jason Gunthorpe

drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4031

Quoted 7 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_cmd.c +1 -2

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
 			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
 			ib_uverbs_ex_destroy_rwq_ind_table,
 			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),

Removing these is kind of troublesome.. This misses the one for ioctl:

Quoted 6 lines

Quoted 3 lines

        UAPI_DEF_CHAIN_OBJ_TREE_NAMED(
                UVERBS_OBJECT_RWQ_IND_TBL,
                UAPI_DEF_OBJ_NEEDS_FN(destroy_rwq_ind_table)),

I will remove, but it seems that we have some gap here, I would expect any sort of compilation error for mlx4.

Why would there be a compilation error?

And it should not be removed, it needs to be reworked to point to some other function I suppose.

Jason

Quoted 17 lines · Jason Gunthorpe

Quoted 16 lines · Leon Romanovsky

Quoted 14 lines · Jason Gunthorpe

drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4031

Quoted 7 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_cmd.c +1 -2

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
 			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
 			ib_uverbs_ex_destroy_rwq_ind_table,
 			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),

Removing these is kind of troublesome.. This misses the one for ioctl:

        UAPI_DEF_CHAIN_OBJ_TREE_NAMED(
                UVERBS_OBJECT_RWQ_IND_TBL,
                UAPI_DEF_OBJ_NEEDS_FN(destroy_rwq_ind_table)),

Quoted 4 lines

Quoted 2 lines

I will remove, but it seems that we have some gap here, I would expect any sort of compilation error for mlx4.

Why would there be a compilation error?

I would expect BUILD_BUG_ON_ZERO() is thrown if ibdev_fn == NULL

Quoted 2 lines

And it should not be removed, it needs to be reworked to point to some other function I suppose.

Why?

Quoted 18 lines · Leon Romanovsky

Quoted 17 lines · Jason Gunthorpe

Quoted 16 lines · Leon Romanovsky

Quoted 14 lines · Jason Gunthorpe

drivers/infiniband/core/uverbs_cmd.c const struct uapi_definition uverbs_def_write_intf[] = { · line 4031

Quoted 7 lines · Leon Romanovsky

drivers/infiniband/core/uverbs_cmd.c +1 -2

@@ -4018,8 +4028,7 @@ const struct uapi_definition uverbs_def_write_intf[] = {
 			IB_USER_VERBS_EX_CMD_DESTROY_RWQ_IND_TBL,
 			ib_uverbs_ex_destroy_rwq_ind_table,
 			UAPI_DEF_WRITE_I(
-				struct ib_uverbs_ex_destroy_rwq_ind_table),
-			UAPI_DEF_METHOD_NEEDS_FN(destroy_rwq_ind_table))),
+				struct ib_uverbs_ex_destroy_rwq_ind_table))),

Removing these is kind of troublesome.. This misses the one for ioctl:

        UAPI_DEF_CHAIN_OBJ_TREE_NAMED(
                UVERBS_OBJECT_RWQ_IND_TBL,
                UAPI_DEF_OBJ_NEEDS_FN(destroy_rwq_ind_table)),

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

I will remove, but it seems that we have some gap here, I would expect any sort of compilation error for mlx4.

Why would there be a compilation error?

I would expect BUILD_BUG_ON_ZERO() is thrown if ibdev_fn == NULL

??