Convert XRC to use xarray
Subsystems (2)
v2 cover Convert XRC to use xarray
From: Leon Romanovsky <[email protected]>
Changelog:
v2: Rewrote mlx5 part to avoid ib_xrcd allocation.
v1: https://lore.kernel.org/lkml/[email protected]
Changed ib_dealloc_xrcd_user() do not iterate over tgt list, because it is expected to be empty.
v0: https://lore.kernel.org/lkml/[email protected]
Two small patches to simplify and improve XRC logic.
Thanks
Leon Romanovsky (1):
RDMA/mlx5: Get XRCD number directly for the internal use
Maor Gottlieb (2):
RDMA/core: Clean ib_alloc_xrcd() and reuse it to allocate XRC domain RDMA/core: Optimize XRC target lookup
drivers/infiniband/core/uverbs_cmd.c | 12 ++--- drivers/infiniband/core/verbs.c | 81 ++++++++++++++-------------- drivers/infiniband/hw/mlx5/main.c | 39 +++++--------- drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +- drivers/infiniband/hw/mlx5/qp.c | 10 ++-- drivers/infiniband/hw/mlx5/srq.c | 4 +- include/rdma/ib_verbs.h | 23 ++------ 7 files changed, 70 insertions(+), 103 deletions(-)
--
2.26.2
PATCH v2 1/3 RDMA/mlx5: Get XRCD number directly for the internal use
From: Leon Romanovsky <[email protected]>
The mlx5_ib creates XRC domain and uses for creating internal SRQ. However all that is needed is XRCD number and not full blown ib_xrcd objects.
Update the code to get and store the number only.
--- drivers/infiniband/hw/mlx5/main.c | 39 ++++++++++------------------ drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +-- drivers/infiniband/hw/mlx5/qp.c | 10 +++---- drivers/infiniband/hw/mlx5/srq.c | 4 +-- 4 files changed, 23 insertions(+), 34 deletions(-)
drivers/infiniband/hw/mlx5/main.c +14 -25
@@ -5016,6 +5016,9 @@ static int create_dev_resources(struct mlx5_ib_resources *devr) dev = container_of(devr, struct mlx5_ib_dev, devr); ibdev = &dev->ib_dev; +if (!MLX5_CAP_GEN (dev->mdev, xrc)) +return -EOPNOTSUPP ; +mutex_init (&devr->mutex); devr->p0 =rdma_zalloc_drv_obj (ibdev, ib_pd);
@@ -5043,34 +5046,19 @@ static int create_dev_resources(struct mlx5_ib_resources *devr)if (ret) goto err_create_cq; - devr->x0 =mlx5_ib_alloc_xrcd (&dev->ib_dev,NULL ); -if (IS_ERR (devr->x0)) { - ret =PTR_ERR (devr->x0); + ret =mlx5_cmd_xrcd_alloc (dev->mdev, &devr->xrcdn0,0 ); +if (ret) goto error2; - } - devr->x0->device = &dev->ib_dev; - devr->x0->inode =NULL ; -atomic_set (&devr->x0->usecnt,0 ); -mutex_init (&devr->x0->tgt_qp_mutex); -INIT_LIST_HEAD (&devr->x0->tgt_qp_list); - devr->x1 =mlx5_ib_alloc_xrcd (&dev->ib_dev,NULL ); -if (IS_ERR (devr->x1)) { - ret =PTR_ERR (devr->x1); + ret =mlx5_cmd_xrcd_alloc (dev->mdev, &devr->xrcdn1,0 ); +if (ret) goto error3; - } - devr->x1->device = &dev->ib_dev; - devr->x1->inode =NULL ; -atomic_set (&devr->x1->usecnt,0 ); -mutex_init (&devr->x1->tgt_qp_mutex); -INIT_LIST_HEAD (&devr->x1->tgt_qp_list);memset (&attr,0 ,sizeof (attr)); attr.attr.max_sge =1 ; attr.attr.max_wr =1 ; attr.srq_type =IB_SRQT_XRC ; attr.ext.cq = devr->c0; - attr.ext.xrc.xrcd = devr->x0; devr->s0 =rdma_zalloc_drv_obj (ibdev, ib_srq);if (!devr->s0) {
@@ -5081,13 +5069,11 @@ static int create_dev_resources(struct mlx5_ib_resources *devr) devr->s0->device = &dev->ib_dev; devr->s0->pd = devr->p0; devr->s0->srq_type =IB_SRQT_XRC ; - devr->s0->ext.xrc.xrcd = devr->x0; devr->s0->ext.cq = devr->c0; ret =mlx5_ib_create_srq (devr->s0, &attr,NULL );if (ret) goto err_create; -atomic_inc (&devr->s0->ext.xrc.xrcd->usecnt);atomic_inc (&devr->s0->ext.cq->usecnt);atomic_inc (&devr->p0->usecnt);atomic_set (&devr->s0->usecnt,0 );
@@ -5129,9 +5115,9 @@ static int create_dev_resources(struct mlx5_ib_resources *devr) err_create:kfree (devr->s0); error4: -mlx5_ib_dealloc_xrcd (devr->x1,NULL ); +mlx5_cmd_xrcd_dealloc (dev->mdev, devr->xrcdn1,0 ); error3: -mlx5_ib_dealloc_xrcd (devr->x0,NULL ); +mlx5_cmd_xrcd_dealloc (dev->mdev, devr->xrcdn0,0 ); error2:mlx5_ib_destroy_cq (devr->c0,NULL ); err_create_cq:
@@ -5145,14 +5131,17 @@ static int create_dev_resources(struct mlx5_ib_resources *devr)static void destroy_dev_resources (struct mlx5_ib_resources *devr) { +struct mlx5_ib_dev *dev;int port; + dev = container_of(devr, struct mlx5_ib_dev, devr); +mlx5_ib_destroy_srq (devr->s1,NULL );kfree (devr->s1);mlx5_ib_destroy_srq (devr->s0,NULL );kfree (devr->s0); -mlx5_ib_dealloc_xrcd (devr->x0,NULL ); -mlx5_ib_dealloc_xrcd (devr->x1,NULL ); +mlx5_cmd_xrcd_dealloc (dev->mdev, devr->xrcdn1,0 ); +mlx5_cmd_xrcd_dealloc (dev->mdev, devr->xrcdn0,0 );mlx5_ib_destroy_cq (devr->c0,NULL );kfree (devr->c0);mlx5_ib_dealloc_pd (devr->p0,NULL );
drivers/infiniband/hw/mlx5/mlx5_ib.h +2 -2
@@ -730,8 +730,8 @@ struct mlx5_ib_port_resources {struct mlx5_ib_resources {struct ib_cq *c0; -struct ib_xrcd *x0; -struct ib_xrcd *x1; +u32 xrcdn0; +u32 xrcdn1;struct ib_pd *p0;struct ib_srq *s0;struct ib_srq *s1;
drivers/infiniband/hw/mlx5/qp.c +5 -5
@@ -2035,15 +2035,15 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,switch (init_attr->qp_type) {case IB_QPT_XRC_INI :MLX5_SET (qpc, qpc, cqn_rcv,to_mcq (devr->c0)->mcq.cqn); -MLX5_SET (qpc, qpc, xrcd,to_mxrcd (devr->x1)->xrcdn); +MLX5_SET (qpc, qpc, xrcd, devr->xrcdn1);MLX5_SET (qpc, qpc, srqn_rmpn_xrqn,to_msrq (devr->s0)->msrq.srqn);break ;default :if (init_attr->srq) { -MLX5_SET (qpc, qpc, xrcd,to_mxrcd (devr->x0)->xrcdn); +MLX5_SET (qpc, qpc, xrcd, devr->xrcdn0);MLX5_SET (qpc, qpc, srqn_rmpn_xrqn,to_msrq (init_attr->srq)->msrq.srqn); }else { -MLX5_SET (qpc, qpc, xrcd,to_mxrcd (devr->x1)->xrcdn); +MLX5_SET (qpc, qpc, xrcd, devr->xrcdn1);MLX5_SET (qpc, qpc, srqn_rmpn_xrqn,to_msrq (devr->s1)->msrq.srqn); } }
@@ -2183,11 +2183,11 @@ static int create_kernel_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,MLX5_SET (qpc, qpc, no_sq,1 );if (attr->srq) { -MLX5_SET (qpc, qpc, xrcd,to_mxrcd (devr->x0)->xrcdn); +MLX5_SET (qpc, qpc, xrcd, devr->xrcdn0);MLX5_SET (qpc, qpc, srqn_rmpn_xrqn,to_msrq (attr->srq)->msrq.srqn); }else { -MLX5_SET (qpc, qpc, xrcd,to_mxrcd (devr->x1)->xrcdn); +MLX5_SET (qpc, qpc, xrcd, devr->xrcdn1);MLX5_SET (qpc, qpc, srqn_rmpn_xrqn,to_msrq (devr->s1)->msrq.srqn); }
drivers/infiniband/hw/mlx5/srq.c +2 -2
@@ -274,10 +274,10 @@ int mlx5_ib_create_srq(struct ib_srq *ib_srq,if (srq->wq_sig) in.flags |=MLX5_SRQ_FLAG_WQ_SIG ; -if (init_attr->srq_type ==IB_SRQT_XRC ) +if (init_attr->srq_type ==IB_SRQT_XRC && init_attr->ext.xrc.xrcd) in.xrcd =to_mxrcd (init_attr->ext.xrc.xrcd)->xrcdn;else - in.xrcd =to_mxrcd (dev->devr.x0)->xrcdn; + in.xrcd = dev->devr.xrcdn0;if (init_attr->srq_type ==IB_SRQT_TM ) { in.tm_log_list_size =
Signature · 1 line
2.26.2
PATCH v2 2/3 RDMA/core: Clean ib_alloc_xrcd() and reuse it to allocate XRC domain
In reply to Leon RomanovskyFrom: Maor Gottlieb <[email protected]>
ib_alloc_xrcd already does the required initialization, so move the uverbs to call it and save code duplication, while cleaning the function argument lists of that function.
--- drivers/infiniband/core/uverbs_cmd.c | 12 +++--------- drivers/infiniband/core/verbs.c | 24 ++++++++++++++++++------ include/rdma/ib_verbs.h | 18 +++--------------- 3 files changed, 24 insertions(+), 30 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +3 -9
@@ -614,17 +614,11 @@ static int ib_uverbs_open_xrcd(struct uverbs_attr_bundle *attrs) }if (!xrcd) { - xrcd = ib_dev->ops.alloc_xrcd (ib_dev, &attrs->driver_udata); + xrcd =ib_alloc_xrcd_user (ib_dev, inode, &attrs->driver_udata);if (IS_ERR (xrcd)) { ret =PTR_ERR (xrcd); goto err; } - - xrcd->inode = inode; - xrcd->device = ib_dev; -atomic_set (&xrcd->usecnt,0 ); -mutex_init (&xrcd->tgt_qp_mutex); -INIT_LIST_HEAD (&xrcd->tgt_qp_list); new_xrcd =1 ; }
@@ -663,7 +657,7 @@ static int ib_uverbs_open_xrcd(struct uverbs_attr_bundle *attrs) } err_dealloc_xrcd: -ib_dealloc_xrcd (xrcd,uverbs_get_cleared_udata (attrs)); +ib_dealloc_xrcd_user (xrcd,uverbs_get_cleared_udata (attrs)); err:uobj_alloc_abort (&obj->uobject, attrs);
@@ -701,7 +695,7 @@ int ib_uverbs_dealloc_xrcd(struct ib_uobject *uobject, struct ib_xrcd *xrcd,if (inode && !atomic_dec_and_test (&xrcd->usecnt))return 0 ; - ret =ib_dealloc_xrcd (xrcd, &attrs->driver_udata); + ret =ib_dealloc_xrcd_user (xrcd, &attrs->driver_udata);if (ib_is_destroy_retryable (ret, why, uobject)) {atomic_inc (&xrcd->usecnt);
drivers/infiniband/core/verbs.c +18 -6
@@ -2289,17 +2289,24 @@ int ib_detach_mcast(struct ib_qp *qp, union ib_gid *gid, u16 lid) }EXPORT_SYMBOL (ib_detach_mcast); -struct ib_xrcd *__ib_alloc_xrcd (struct ib_device *device,const char *caller) +/** +* ib_alloc_xrcd_user - Allocates an XRC domain. +* @device: The device on which to allocate the XRC domain. +* @inode: inode to connect XRCD +* @udata: Valid user data or NULL for kernel object +*/ +struct ib_xrcd *ib_alloc_xrcd_user (struct ib_device *device, +struct inode *inode,struct ib_udata *udata) {struct ib_xrcd *xrcd;if (!device->ops.alloc_xrcd)return ERR_PTR (-EOPNOTSUPP ); - xrcd = device->ops.alloc_xrcd (device,NULL ); + xrcd = device->ops.alloc_xrcd (device, udata);if (!IS_ERR (xrcd)) { xrcd->device = device; - xrcd->inode =NULL ; + xrcd->inode = inode;atomic_set (&xrcd->usecnt,0 );mutex_init (&xrcd->tgt_qp_mutex);INIT_LIST_HEAD (&xrcd->tgt_qp_list);
@@ -2307,9 +2314,14 @@ struct ib_xrcd *__ib_alloc_xrcd(struct ib_device *device, const char *caller)return xrcd; } -EXPORT_SYMBOL (__ib_alloc_xrcd); +EXPORT_SYMBOL (ib_alloc_xrcd_user); -int ib_dealloc_xrcd (struct ib_xrcd *xrcd,struct ib_udata *udata) +/** +* ib_dealloc_xrcd_user - Deallocates an XRC domain. +* @xrcd: The XRC domain to deallocate. +* @udata: Valid user data or NULL for kernel object +*/ +int ib_dealloc_xrcd_user (struct ib_xrcd *xrcd,struct ib_udata *udata) {struct ib_qp *qp;int ret;
@@ -2327,7 +2339,7 @@ int ib_dealloc_xrcd(struct ib_xrcd *xrcd, struct ib_udata *udata)return xrcd->device->ops.dealloc_xrcd (xrcd, udata); } -EXPORT_SYMBOL (ib_dealloc_xrcd); +EXPORT_SYMBOL (ib_dealloc_xrcd_user);/** * ib_create_wq - Creates a WQ associated with the specified protection
include/rdma/ib_verbs.h +3 -15
@@ -4332,21 +4332,9 @@ int ib_attach_mcast(struct ib_qp *qp, union ib_gid *gid, u16 lid);*/ int ib_detach_mcast (struct ib_qp *qp,union ib_gid *gid,u16 lid); -/** -* ib_alloc_xrcd - Allocates an XRC domain. -* @device: The device on which to allocate the XRC domain. -* @caller: Module name for kernel consumers -*/ -struct ib_xrcd *__ib_alloc_xrcd (struct ib_device *device,const char *caller); -#define ib_alloc_xrcd (device) \ - __ib_alloc_xrcd((device), KBUILD_MODNAME) - -/** -* ib_dealloc_xrcd - Deallocates an XRC domain. -* @xrcd: The XRC domain to deallocate. -* @udata: Valid user data or NULL for kernel object -*/ -int ib_dealloc_xrcd (struct ib_xrcd *xrcd,struct ib_udata *udata); +struct ib_xrcd *ib_alloc_xrcd_user (struct ib_device *device, +struct inode *inode,struct ib_udata *udata); +int ib_dealloc_xrcd_user (struct ib_xrcd *xrcd,struct ib_udata *udata);static inline int ib_check_mr_access (int flags) {
Signature · 1 line
2.26.2
PATCH v2 3/3 RDMA/core: Optimize XRC target lookup
In reply to Leon RomanovskyFrom: Maor Gottlieb <[email protected]>
Replace the mutex with read write semaphore and use xarray instead of linked list for XRC target QPs. This will give faster XRC target lookup. In addition, when QP is closed, don't insert it back to the xarray if the destroy command failed.
--- drivers/infiniband/core/verbs.c | 57 ++++++++++++--------------------- include/rdma/ib_verbs.h | 5 ++- 2 files changed, 23 insertions(+), 39 deletions(-)
drivers/infiniband/core/verbs.c +21 -36
@@ -1090,13 +1090,6 @@ static void __ib_shared_qp_event_handler(struct ib_event *event, void *context)spin_unlock_irqrestore (&qp->device->qp_open_list_lock, flags); } -static void __ib_insert_xrcd_qp (struct ib_xrcd *xrcd,struct ib_qp *qp) -{ -mutex_lock (&xrcd->tgt_qp_mutex); -list_add (&qp->xrcd_list, &xrcd->tgt_qp_list); -mutex_unlock (&xrcd->tgt_qp_mutex); -} -static struct ib_qp *__ib_open_qp (struct ib_qp *real_qp,void (*event_handler)(struct ib_event *,void *),void *qp_context)
@@ -1139,16 +1132,15 @@ struct ib_qp *ib_open_qp(struct ib_xrcd *xrcd,if (qp_open_attr->qp_type !=IB_QPT_XRC_TGT )return ERR_PTR (-EINVAL ); - qp =ERR_PTR (-EINVAL ); -mutex_lock (&xrcd->tgt_qp_mutex); -list_for_each_entry (real_qp, &xrcd->tgt_qp_list, xrcd_list) { -if (real_qp->qp_num == qp_open_attr->qp_num) { - qp =__ib_open_qp (real_qp, qp_open_attr->event_handler, - qp_open_attr->qp_context); -break ; - } +down_read (&xrcd->tgt_qps_rwsem); + real_qp =xa_load (&xrcd->tgt_qps, qp_open_attr->qp_num); +if (!real_qp) { +up_read (&xrcd->tgt_qps_rwsem); +return ERR_PTR (-EINVAL ); } -mutex_unlock (&xrcd->tgt_qp_mutex); + qp =__ib_open_qp (real_qp, qp_open_attr->event_handler, + qp_open_attr->qp_context); +up_read (&xrcd->tgt_qps_rwsem);return qp; }EXPORT_SYMBOL (ib_open_qp);
@@ -1157,6 +1149,7 @@ static struct ib_qp *create_xrc_qp_user(struct ib_qp *qp, struct ib_qp_init_attr *qp_init_attr) {struct ib_qp *real_qp = qp; +int err; qp->event_handler = __ib_shared_qp_event_handler; qp->qp_context = qp;
@@ -1172,7 +1165,12 @@ static struct ib_qp *create_xrc_qp_user(struct ib_qp *qp,if (IS_ERR (qp))return qp; -__ib_insert_xrcd_qp (qp_init_attr->xrcd, real_qp); + err =xa_err (xa_store (&qp_init_attr->xrcd->tgt_qps, real_qp->qp_num, + real_qp,GFP_KERNEL )); +if (err) { +ib_close_qp (qp); +return ERR_PTR (err); + }return qp; }
@@ -1888,21 +1886,18 @@ static int __ib_destroy_shared_qp(struct ib_qp *qp) real_qp = qp->real_qp; xrcd = real_qp->xrcd; - -mutex_lock (&xrcd->tgt_qp_mutex); +down_write (&xrcd->tgt_qps_rwsem);ib_close_qp (qp);if (atomic_read (&real_qp->usecnt) ==0 ) -list_del (&real_qp->xrcd_list); +xa_erase (&xrcd->tgt_qps, real_qp->qp_num);else real_qp =NULL ; -mutex_unlock (&xrcd->tgt_qp_mutex); +up_write (&xrcd->tgt_qps_rwsem);if (real_qp) { ret =ib_destroy_qp (real_qp);if (!ret)atomic_dec (&xrcd->usecnt); -else -__ib_insert_xrcd_qp (xrcd, real_qp); }return 0 ;
@@ -2308,8 +2303,8 @@ struct ib_xrcd *ib_alloc_xrcd_user(struct ib_device *device, xrcd->device = device; xrcd->inode = inode;atomic_set (&xrcd->usecnt,0 ); -mutex_init (&xrcd->tgt_qp_mutex); -INIT_LIST_HEAD (&xrcd->tgt_qp_list); +init_rwsem (&xrcd->tgt_qps_rwsem); +xa_init (&xrcd->tgt_qps); }return xrcd;
@@ -2323,20 +2318,10 @@ EXPORT_SYMBOL(ib_alloc_xrcd_user);*/ int ib_dealloc_xrcd_user (struct ib_xrcd *xrcd,struct ib_udata *udata) { -struct ib_qp *qp; -int ret; -if (atomic_read (&xrcd->usecnt))return -EBUSY ; -while (!list_empty (&xrcd->tgt_qp_list)) { - qp = list_entry(xrcd->tgt_qp_list.next, struct ib_qp, xrcd_list); - ret =ib_destroy_qp (qp); -if (ret) -return ret; - } -mutex_destroy (&xrcd->tgt_qp_mutex); - +WARN_ON (!xa_empty (&xrcd->tgt_qps));return xrcd->device->ops.dealloc_xrcd (xrcd, udata); }EXPORT_SYMBOL (ib_dealloc_xrcd_user);
include/rdma/ib_verbs.h +2 -3
@@ -1568,9 +1568,8 @@ struct ib_xrcd {struct ib_device *device;atomic_t usecnt;/* count all exposed resources */ struct inode *inode; - -struct mutex tgt_qp_mutex; -struct list_head tgt_qp_list; +struct rw_semaphore tgt_qps_rwsem; +struct xarray tgt_qps; };struct ib_ah {
Signature · 1 line
2.26.2
Quoted 10 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v2: Rewrote mlx5 part to avoidib_xrcdallocation.
v1: https://lore.kernel.org/lkml/[email protected]
Changedib_dealloc_xrcd_user()do not iterate over tgt list, because it is expected to be empty.
v0: https://lore.kernel.org/lkml/[email protected]
Two small patches to simplify and improve XRC logic.
Quoted 8 lines
Thanks
Leon Romanovsky (1):
RDMA/mlx5: Get XRCD number directly for the internal useMaor Gottlieb (2):
RDMA/core: Cleanib_alloc_xrcd()and reuse it to allocate XRC domain RDMA/core: Optimize XRC target lookup
Applied to for-next, thanks
Jason