RDMA/siw: publish QP after initialization
Subsystems (2)
PATCH v4 1/1 RDMA/siw: publish QP after initialization
siw_create_qp() currently calls siw_qp_add() before the queues, CQ pointers, state, completion, and device list entry are ready. A QPN lookup can therefore reach a QP that is still being constructed.
Move siw_qp_add() to the end of siw_create_qp(), after QP initialization and before adding the QP to the siw device list.
---
Changes in v4:
- Move if (udata->outlen < sizeof(uresp)) into the second if (udata) clause, right before ib_copy_to_udata() (Bernard Metzler).
- Move INIT_LIST_HEAD(&qp->devq) to just before spin_lock_irqsave(), close to list_add_tail() where it logically belongs (Bernard Metzler).
Changes in v3:
- Move siw_qp_add()/xa_alloc() to the end of siw_create_qp().
- Drop the QPN reservation helper from v2.
drivers/infiniband/sw/siw/siw_verbs.c | 57 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 22 deletions(-)
drivers/infiniband/sw/siw/siw_verbs.c +24 -20
@@ -316,6 +316,7 @@struct siw_ucontext *uctx = rdma_udata_to_drv_context(udata, struct siw_ucontext, base_ucontext); +struct siw_uresp_create_qp uresp = {};unsigned long flags;int num_sqe, num_rqe, rv =0 ;size_t length;
@@ -369,11 +370,6 @@spin_lock_init (&qp->rq_lock);spin_lock_init (&qp->orq_lock); - rv =siw_qp_add (sdev, qp); -if (rv) - goto err_atomic; - -/* All queue indices are derived from modulo operations * on a free running 'get' (consumer) and 'put' (producer) * unsigned counter. Having queue sizes at power of two
@@ -391,14 +387,14 @@if (qp->sendq ==NULL ) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; }if (attrs->sq_sig_type !=IB_SIGNAL_REQ_WR ) {if (attrs->sq_sig_type ==IB_SIGNAL_ALL_WR ) qp->attrs.flags |=SIW_SIGNAL_ALL_WR ;else { rv = -EINVAL ; - goto err_out_xa; + goto err_out; } } qp->pd = pd;
@@ -424,7 +420,7 @@if (qp->recvq ==NULL ) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } qp->attrs.rq_size = num_rqe; }
@@ -439,11 +435,8 @@ qp->attrs.state =SIW_QP_STATE_IDLE ;if (udata) { -struct siw_uresp_create_qp uresp = {}; - uresp.num_sqe = num_sqe; uresp.num_rqe = num_rqe; - uresp.qp_id =qp_id (qp);if (qp->sendq) { length = num_sqe *sizeof (struct siw_sqe );
@@ -452,7 +445,7 @@ length, &uresp.sq_key);if (!qp->sq_entry) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } }
@@ -464,9 +457,23 @@if (!qp->rq_entry) { uresp.sq_key =SIW_INVAL_UOBJ_KEY ; rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } } + } + qp->tx_cpu =siw_get_tx_cpu (sdev); +if (qp->tx_cpu <0 ) { + rv = -EINVAL ; + goto err_out; + } +init_completion (&qp->qp_free); + + rv =siw_qp_add (sdev, qp); +if (rv) + goto err_out_tx; + +if (udata) { + uresp.qp_id =qp_id (qp);if (udata->outlen <sizeof (uresp)) { rv = -EINVAL ;
@@ -476,22 +483,19 @@if (rv) goto err_out_xa; } - qp->tx_cpu =siw_get_tx_cpu (sdev); -if (qp->tx_cpu <0 ) { - rv = -EINVAL ; - goto err_out_xa; - } +INIT_LIST_HEAD (&qp->devq);spin_lock_irqsave (&sdev->lock, flags);list_add_tail (&qp->devq, &sdev->qp_list);spin_unlock_irqrestore (&sdev->lock, flags); -init_completion (&qp->qp_free); -return 0 ; err_out_xa:xa_erase (&sdev->qp_xa,qp_id (qp)); +err_out_tx: +siw_put_tx_cpu (qp->tx_cpu); +err_out:if (uctx) {rdma_user_mmap_entry_remove (qp->sq_entry);rdma_user_mmap_entry_remove (qp->rq_entry);
Signature · 1 line
2.51.0
Quoted 136 lines · Ruoyu Wang
siw_create_qp()currently callssiw_qp_add()before the queues, CQ pointers, state, completion, and device list entry are ready. A QPN lookup can therefore reach a QP that is still being constructed.Move
siw_qp_add()to the end ofsiw_create_qp(), after QP initialization and before adding the QP to the siw device list.Fixes f29dd55b0236 ("rdma/siw: queue pair methods")Suggested-by Bernard Metzler <[email protected]>Signed-off-by Ruoyu Wang <[email protected]>---
Changes in v4:
- Moveif (udata->outlen < sizeof(uresp))into the secondif (udata)clause, right beforeib_copy_to_udata()(Bernard Metzler).
- MoveINIT_LIST_HEAD(&qp->devq)to just beforespin_lock_irqsave(), close tolist_add_tail()where it logically belongs (Bernard Metzler).Changes in v3:
- Move siw_qp_add()/xa_alloc() to the end ofsiw_create_qp().
- Drop the QPN reservation helper from v2.drivers/infiniband/sw/siw/siw_verbs.c | 57 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 22 deletions(-)drivers/infiniband/sw/siw/siw_verbs.c +21
-20@@ -316,6 +316,7 @@struct siw_ucontext *uctx = rdma_udata_to_drv_context(udata, struct siw_ucontext, base_ucontext); +struct siw_uresp_create_qp uresp = {};unsigned long flags;int num_sqe, num_rqe, rv =0 ;size_t length;@@ -369,11 +370,6 @@spin_lock_init (&qp->rq_lock);spin_lock_init (&qp->orq_lock); - rv =siw_qp_add (sdev, qp); -if (rv) - goto err_atomic; - -/* All queue indices are derived from modulo operations * on a free running 'get' (consumer) and 'put' (producer) * unsigned counter. Having queue sizes at power of two @@ -391,14 +387,14 @@if (qp->sendq ==NULL ) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; }if (attrs->sq_sig_type !=IB_SIGNAL_REQ_WR ) {if (attrs->sq_sig_type ==IB_SIGNAL_ALL_WR ) qp->attrs.flags |=SIW_SIGNAL_ALL_WR ;else { rv = -EINVAL ; - goto err_out_xa; + goto err_out; } } qp->pd = pd;@@ -424,7 +420,7 @@if (qp->recvq ==NULL ) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } qp->attrs.rq_size = num_rqe; }@@ -439,11 +435,8 @@ qp->attrs.state =SIW_QP_STATE_IDLE ;if (udata) { -struct siw_uresp_create_qp uresp = {}; - uresp.num_sqe = num_sqe; uresp.num_rqe = num_rqe; - uresp.qp_id =qp_id (qp);if (qp->sendq) { length = num_sqe *sizeof (struct siw_sqe );@@ -452,7 +445,7 @@ length, &uresp.sq_key);if (!qp->sq_entry) { rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } }@@ -464,9 +457,23 @@if (!qp->rq_entry) { uresp.sq_key =SIW_INVAL_UOBJ_KEY ; rv = -ENOMEM ; - goto err_out_xa; + goto err_out; } } + } + qp->tx_cpu =siw_get_tx_cpu (sdev); +if (qp->tx_cpu <0 ) { + rv = -EINVAL ; + goto err_out; + } +init_completion (&qp->qp_free); + + rv =siw_qp_add (sdev, qp); +if (rv) + goto err_out_tx; + +if (udata) { + uresp.qp_id =qp_id (qp);if (udata->outlen <sizeof (uresp)) { rv = -EINVAL ;@@ -476,22 +483,19 @@if (rv) goto err_out_xa; } - qp->tx_cpu =siw_get_tx_cpu (sdev); -if (qp->tx_cpu <0 ) { - rv = -EINVAL ; - goto err_out_xa; - } +INIT_LIST_HEAD (&qp->devq);spin_lock_irqsave (&sdev->lock, flags);list_add_tail (&qp->devq, &sdev->qp_list);spin_unlock_irqrestore (&sdev->lock, flags); -init_completion (&qp->qp_free); -
drivers/infiniband/sw/siw/siw_verbs.c line 501
Quoted 10 lines
drivers/infiniband/sw/siw/siw_verbs.c +3
-0return 0 ; err_out_xa:xa_erase (&sdev->qp_xa,qp_id (qp)); +err_out_tx: +siw_put_tx_cpu (qp->tx_cpu); +err_out:if (uctx) {rdma_user_mmap_entry_remove (qp->sq_entry);rdma_user_mmap_entry_remove (qp->rq_entry);
Thanks Ruoyu, that looks good.
Quoted 18 lines · Ruoyu Wang
siw_create_qp()currently callssiw_qp_add()before the queues, CQ pointers, state, completion, and device list entry are ready. A QPN lookup can therefore reach a QP that is still being constructed.Move
siw_qp_add()to the end ofsiw_create_qp(), after QP initialization and before adding the QP to the siw device list.Fixes f29dd55b0236 ("rdma/siw: queue pair methods")Suggested-by Bernard Metzler <[email protected]>Signed-off-by Ruoyu Wang <[email protected]>Acked-by Bernard Metzler <[email protected]>---
Changes in v4:
- Moveif (udata->outlen < sizeof(uresp))into the secondif (udata)clause, right beforeib_copy_to_udata()(Bernard Metzler).
- MoveINIT_LIST_HEAD(&qp->devq)to just beforespin_lock_irqsave(), close tolist_add_tail()where it logically belongs (Bernard Metzler).
Quoted 6 lines
Changes in v3:
- Move siw_qp_add()/xa_alloc() to the end ofsiw_create_qp().
- Drop the QPN reservation helper from v2.drivers/infiniband/sw/siw/siw_verbs.c | 57 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 22 deletions(-)
Applied to for-rc, thanks
Jason