* [PATCH v3] RDMA/siw: publish QP after initialization
@ 2026-06-27 14:40 Ruoyu Wang
2026-06-29 15:32 ` Bernard Metzler
0 siblings, 1 reply; 2+ messages in thread
From: Ruoyu Wang @ 2026-06-27 14:40 UTC (permalink / raw)
To: Bernard Metzler
Cc: Jason Gunthorpe, Leon Romanovsky, linux-rdma, linux-kernel,
Ruoyu Wang
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.
Fixes: f29dd55b0236 ("rdma/siw: queue pair methods")
Suggested-by: Bernard Metzler <bernard.metzler@linux.dev>
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
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 | 45 +++++++++++++++------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 1e1d262a4ae2..ee3e5529d6f4 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -316,6 +316,7 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
if (qp->recvq == NULL) {
rv = -ENOMEM;
- goto err_out_xa;
+ goto err_out;
}
qp->attrs.rq_size = num_rqe;
}
@@ -439,11 +435,8 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
length, &uresp.sq_key);
if (!qp->sq_entry) {
rv = -ENOMEM;
- goto err_out_xa;
+ goto err_out;
}
}
@@ -464,34 +457,46 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
if (!qp->rq_entry) {
uresp.sq_key = SIW_INVAL_UOBJ_KEY;
rv = -ENOMEM;
- goto err_out_xa;
+ goto err_out;
}
}
if (udata->outlen < sizeof(uresp)) {
rv = -EINVAL;
- goto err_out_xa;
+ goto err_out;
}
- rv = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
- 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;
+ goto err_out;
}
INIT_LIST_HEAD(&qp->devq);
+ 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);
+
+ rv = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
+ if (rv)
+ goto err_out_xa;
+ }
+
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);
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] RDMA/siw: publish QP after initialization
2026-06-27 14:40 [PATCH v3] RDMA/siw: publish QP after initialization Ruoyu Wang
@ 2026-06-29 15:32 ` Bernard Metzler
0 siblings, 0 replies; 2+ messages in thread
From: Bernard Metzler @ 2026-06-29 15:32 UTC (permalink / raw)
To: Ruoyu Wang; +Cc: Jason Gunthorpe, Leon Romanovsky, linux-rdma, linux-kernel
On 27.06.2026 16:40, Ruoyu Wang wrote:
> 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.
>
> Fixes: f29dd55b0236 ("rdma/siw: queue pair methods")
> Suggested-by: Bernard Metzler <bernard.metzler@linux.dev>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
> 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 | 45 +++++++++++++++------------
> 1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> index 1e1d262a4ae2..ee3e5529d6f4 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -316,6 +316,7 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
> 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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
> 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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
>
> 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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
>
> if (qp->recvq == NULL) {
> rv = -ENOMEM;
> - goto err_out_xa;
> + goto err_out;
> }
> qp->attrs.rq_size = num_rqe;
> }
> @@ -439,11 +435,8 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
> 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 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
> length, &uresp.sq_key);
> if (!qp->sq_entry) {
> rv = -ENOMEM;
> - goto err_out_xa;
> + goto err_out;
> }
> }
>
> @@ -464,34 +457,46 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
> if (!qp->rq_entry) {
> uresp.sq_key = SIW_INVAL_UOBJ_KEY;
> rv = -ENOMEM;
> - goto err_out_xa;
> + goto err_out;
> }
> }
>
move below check as well into your new 'if (udata) {'
clause below, right before doing the
ib_copy_to_udata() thing. It logically belongs there.
> if (udata->outlen < sizeof(uresp)) {
> rv = -EINVAL;
> - goto err_out_xa;
> + goto err_out;
> }
> - rv = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> - 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;
> + goto err_out;
> }
> INIT_LIST_HEAD(&qp->devq);
Put above line close to the list_add_tail() below,
that's where it logically belongs to.
Looks good otherwise!
Thanks,
Bernard.
> + 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);
> +
> + rv = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> + if (rv)
> + goto err_out_xa;
> + }
> +
> 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);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-29 15:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-27 14:40 [PATCH v3] RDMA/siw: publish QP after initialization Ruoyu Wang
2026-06-29 15:32 ` Bernard Metzler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox