[PATCH] RDMA/siw: publish QP after initialization

[PATCH] RDMA/siw: publish QP after initialization

[Ruoyu Wang]

siw_create_qp() allocates a QP number 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 if the object is
published at allocation time.

Reserve the QPN with an empty XArray entry first. Publish the QP object
only after the kernel-visible QP state is initialized and just before
siw_create_qp() returns it to the caller.

Fixes: f29dd55b0236 ("rdma/siw: queue pair methods")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/infiniband/sw/siw/siw.h       |  1 +
 drivers/infiniband/sw/siw/siw_qp.c    | 26 ++++++++++++++++++--------
 drivers/infiniband/sw/siw/siw_verbs.c | 12 +++++++++++-
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
index f5fd71717b80..ade7c96135c2 100644
--- a/drivers/infiniband/sw/siw/siw.h
+++ b/drivers/infiniband/sw/siw/siw.h
@@ -511,6 +511,7 @@ void siw_send_terminate(struct siw_qp *qp);
 void siw_qp_get_ref(struct ib_qp *qp);
 void siw_qp_put_ref(struct ib_qp *qp);
 int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp);
+int siw_qp_publish(struct siw_qp *qp);
 void siw_free_qp(struct kref *ref);
 
 void siw_init_terminate(struct siw_qp *qp, enum term_elayer layer,
diff --git a/drivers/infiniband/sw/siw/siw_qp.c b/drivers/infiniband/sw/siw/siw_qp.c
index bb780e3904a2..1a9135d9a2a7 100644
--- a/drivers/infiniband/sw/siw/siw_qp.c
+++ b/drivers/infiniband/sw/siw/siw_qp.c
@@ -1281,15 +1281,25 @@ void siw_rq_flush(struct siw_qp *qp)
 
 int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp)
 {
-	int rv = xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, qp, xa_limit_32b,
-			  GFP_KERNEL);
+	qp->sdev = sdev;
 
-	if (!rv) {
-		kref_init(&qp->ref);
-		qp->sdev = sdev;
-		siw_dbg_qp(qp, "new QP\n");
-	}
-	return rv;
+	return xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, NULL,
+			xa_limit_32b, GFP_KERNEL);
+}
+
+int siw_qp_publish(struct siw_qp *qp)
+{
+	void *old;
+
+	kref_init(&qp->ref);
+
+	old = xa_store(&qp->sdev->qp_xa, qp_id(qp), qp, GFP_KERNEL);
+	if (xa_is_err(old))
+		return xa_err(old);
+
+	siw_dbg_qp(qp, "new QP\n");
+
+	return 0;
 }
 
 void siw_free_qp(struct kref *ref)
diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 1e1d262a4ae2..71bc0cc59e3d 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -482,14 +482,24 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
 		goto err_out_xa;
 	}
 	INIT_LIST_HEAD(&qp->devq);
+	init_completion(&qp->qp_free);
+
 	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);
+	rv = siw_qp_publish(qp);
+	if (rv)
+		goto err_out_list;
 
 	return 0;
 
+err_out_list:
+	spin_lock_irqsave(&sdev->lock, flags);
+	list_del(&qp->devq);
+	spin_unlock_irqrestore(&sdev->lock, flags);
+
+	siw_put_tx_cpu(qp->tx_cpu);
 err_out_xa:
 	xa_erase(&sdev->qp_xa, qp_id(qp));
 	if (uctx) {

Re: [PATCH] RDMA/siw: publish QP after initialization

[Bernard Metzler]

On 20.06.2026 17:53, Ruoyu Wang wrote:
> siw_create_qp() allocates a QP number 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 if the object is
> published at allocation time.
> 
> Reserve the QPN with an empty XArray entry first. Publish the QP object
> only after the kernel-visible QP state is initialized and just before
> siw_create_qp() returns it to the caller.
> 
> Fixes: f29dd55b0236 ("rdma/siw: queue pair methods")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>   drivers/infiniband/sw/siw/siw.h       |  1 +
>   drivers/infiniband/sw/siw/siw_qp.c    | 26 ++++++++++++++++++--------
>   drivers/infiniband/sw/siw/siw_verbs.c | 12 +++++++++++-
>   3 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
> index f5fd71717b80..ade7c96135c2 100644
> --- a/drivers/infiniband/sw/siw/siw.h
> +++ b/drivers/infiniband/sw/siw/siw.h
> @@ -511,6 +511,7 @@ void siw_send_terminate(struct siw_qp *qp);
>   void siw_qp_get_ref(struct ib_qp *qp);
>   void siw_qp_put_ref(struct ib_qp *qp);
>   int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp);
> +int siw_qp_publish(struct siw_qp *qp);
>   void siw_free_qp(struct kref *ref);
>   
>   void siw_init_terminate(struct siw_qp *qp, enum term_elayer layer,
> diff --git a/drivers/infiniband/sw/siw/siw_qp.c b/drivers/infiniband/sw/siw/siw_qp.c
> index bb780e3904a2..1a9135d9a2a7 100644
> --- a/drivers/infiniband/sw/siw/siw_qp.c
> +++ b/drivers/infiniband/sw/siw/siw_qp.c
> @@ -1281,15 +1281,25 @@ void siw_rq_flush(struct siw_qp *qp)
>   
>   int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp)
>   {
> -	int rv = xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, qp, xa_limit_32b,
> -			  GFP_KERNEL);
> +	qp->sdev = sdev;
>   
> -	if (!rv) {
> -		kref_init(&qp->ref);
> -		qp->sdev = sdev;
> -		siw_dbg_qp(qp, "new QP\n");
> -	}
> -	return rv;
> +	return xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, NULL,
> +			xa_limit_32b, GFP_KERNEL);
> +}
> +
> +int siw_qp_publish(struct siw_qp *qp)
> +{
> +	void *old;
> +
> +	kref_init(&qp->ref);
> +
> +	old = xa_store(&qp->sdev->qp_xa, qp_id(qp), qp, GFP_KERNEL);
> +	if (xa_is_err(old))
> +		return xa_err(old);
> +
> +	siw_dbg_qp(qp, "new QP\n");
> +
> +	return 0;
>   }
>   
>   void siw_free_qp(struct kref *ref)
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> index 1e1d262a4ae2..71bc0cc59e3d 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -482,14 +482,24 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
>   		goto err_out_xa;
>   	}
>   	INIT_LIST_HEAD(&qp->devq);
> +	init_completion(&qp->qp_free);
> +
>   	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);
> +	rv = siw_qp_publish(qp);

To avoid this transient visibility of a not-yet-initialized
QP - can't we just move siw_qp_add() to the end of the
siw_create_qp() function?


> +	if (rv)
> +		goto err_out_list;
>   
>   	return 0;
>   
> +err_out_list:
> +	spin_lock_irqsave(&sdev->lock, flags);
> +	list_del(&qp->devq);
> +	spin_unlock_irqrestore(&sdev->lock, flags);
> +
> +	siw_put_tx_cpu(qp->tx_cpu);
>   err_out_xa:
>   	xa_erase(&sdev->qp_xa, qp_id(qp));
>   	if (uctx) {

[PATCH v2] RDMA/siw: publish QP after initialization

[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 the siw_qp_add() publication step to the end of siw_create_qp(),
after the kernel-visible QP state is initialized. The QPN must still be
known before copying the siw-specific create response to userspace, so
reserve the QPN first with an empty XArray entry. This lets
siw_create_qp() report the QPN while QPN lookups still return NULL until
the QP is published.

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 v2:
- Move the siw_qp_add() publication step to the end of siw_create_qp().
- Add siw_qp_reserve_qpn() so the udata response can still report qp_num
  before the QP becomes visible to QPN lookups.

 drivers/infiniband/sw/siw/siw.h       |  1 +
 drivers/infiniband/sw/siw/siw_qp.c    | 26 ++++++++++++++++++--------
 drivers/infiniband/sw/siw/siw_verbs.c | 14 ++++++++++++--
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
index f5fd71717b80..f8d28dd7dd86 100644
--- a/drivers/infiniband/sw/siw/siw.h
+++ b/drivers/infiniband/sw/siw/siw.h
@@ -510,6 +510,7 @@ void siw_send_terminate(struct siw_qp *qp);
 
 void siw_qp_get_ref(struct ib_qp *qp);
 void siw_qp_put_ref(struct ib_qp *qp);
+int siw_qp_reserve_qpn(struct siw_device *sdev, struct siw_qp *qp);
 int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp);
 void siw_free_qp(struct kref *ref);
 
diff --git a/drivers/infiniband/sw/siw/siw_qp.c b/drivers/infiniband/sw/siw/siw_qp.c
index bb780e3904a2..7d6224ebfe71 100644
--- a/drivers/infiniband/sw/siw/siw_qp.c
+++ b/drivers/infiniband/sw/siw/siw_qp.c
@@ -1279,17 +1279,27 @@ void siw_rq_flush(struct siw_qp *qp)
 	}
 }
 
+int siw_qp_reserve_qpn(struct siw_device *sdev, struct siw_qp *qp)
+{
+	qp->sdev = sdev;
+
+	return xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, NULL,
+			xa_limit_32b, GFP_KERNEL);
+}
+
 int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp)
 {
-	int rv = xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, qp, xa_limit_32b,
-			  GFP_KERNEL);
+	void *old;
 
-	if (!rv) {
-		kref_init(&qp->ref);
-		qp->sdev = sdev;
-		siw_dbg_qp(qp, "new QP\n");
-	}
-	return rv;
+	kref_init(&qp->ref);
+
+	old = xa_store(&sdev->qp_xa, qp_id(qp), qp, GFP_KERNEL);
+	if (xa_is_err(old))
+		return xa_err(old);
+
+	siw_dbg_qp(qp, "new QP\n");
+
+	return 0;
 }
 
 void siw_free_qp(struct kref *ref)
diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 1e1d262a4ae2..ef9fa9c5bf88 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -369,7 +369,7 @@ 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);
+	rv = siw_qp_reserve_qpn(sdev, qp);
 	if (rv)
 		goto err_atomic;
 
@@ -482,14 +482,24 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
 		goto err_out_xa;
 	}
 	INIT_LIST_HEAD(&qp->devq);
+	init_completion(&qp->qp_free);
+
 	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);
+	rv = siw_qp_add(sdev, qp);
+	if (rv)
+		goto err_out_list;
 
 	return 0;
 
+err_out_list:
+	spin_lock_irqsave(&sdev->lock, flags);
+	list_del(&qp->devq);
+	spin_unlock_irqrestore(&sdev->lock, flags);
+
+	siw_put_tx_cpu(qp->tx_cpu);
 err_out_xa:
 	xa_erase(&sdev->qp_xa, qp_id(qp));
 	if (uctx) {

Re: [PATCH v2] RDMA/siw: publish QP after initialization

[Bernard Metzler]

On 25.06.2026 15:44, 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 the siw_qp_add() publication step to the end of siw_create_qp(),
> after the kernel-visible QP state is initialized. The QPN must still be
> known before copying the siw-specific create response to userspace, so
> reserve the QPN first with an empty XArray entry. This lets
> siw_create_qp() report the QPN while QPN lookups still return NULL until
> the QP is published.
> 
Hi Ruoyu,

I am sorry I obviously wasn't clear on the idea:

Why can't we just move the xa_alloc() thing to the bottom
of create_qp()? Your complaint was that the QP is already visible
during initialization, which might be problematic. So, if you
move that down - just before adding the qp to the siw device.
It should be safe.

Why do we fuzz around with reserving an xa entry before
actually having a qp at it, if we can do the xa entry
as a last step after qp initialization.

Thanks,
Bernard
> 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 v2:
> - Move the siw_qp_add() publication step to the end of siw_create_qp().
> - Add siw_qp_reserve_qpn() so the udata response can still report qp_num
>    before the QP becomes visible to QPN lookups.
> 
>   drivers/infiniband/sw/siw/siw.h       |  1 +
>   drivers/infiniband/sw/siw/siw_qp.c    | 26 ++++++++++++++++++--------
>   drivers/infiniband/sw/siw/siw_verbs.c | 14 ++++++++++++--
>   3 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
> index f5fd71717b80..f8d28dd7dd86 100644
> --- a/drivers/infiniband/sw/siw/siw.h
> +++ b/drivers/infiniband/sw/siw/siw.h
> @@ -510,6 +510,7 @@ void siw_send_terminate(struct siw_qp *qp);
>   
>   void siw_qp_get_ref(struct ib_qp *qp);
>   void siw_qp_put_ref(struct ib_qp *qp);
> +int siw_qp_reserve_qpn(struct siw_device *sdev, struct siw_qp *qp);
>   int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp);
>   void siw_free_qp(struct kref *ref);
>   
> diff --git a/drivers/infiniband/sw/siw/siw_qp.c b/drivers/infiniband/sw/siw/siw_qp.c
> index bb780e3904a2..7d6224ebfe71 100644
> --- a/drivers/infiniband/sw/siw/siw_qp.c
> +++ b/drivers/infiniband/sw/siw/siw_qp.c
> @@ -1279,17 +1279,27 @@ void siw_rq_flush(struct siw_qp *qp)
>   	}
>   }
>   
> +int siw_qp_reserve_qpn(struct siw_device *sdev, struct siw_qp *qp)
> +{
> +	qp->sdev = sdev;
> +
> +	return xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, NULL,
> +			xa_limit_32b, GFP_KERNEL);
> +}
> +
>   int siw_qp_add(struct siw_device *sdev, struct siw_qp *qp)
>   {
> -	int rv = xa_alloc(&sdev->qp_xa, &qp->base_qp.qp_num, qp, xa_limit_32b,
> -			  GFP_KERNEL);
> +	void *old;
>   
> -	if (!rv) {
> -		kref_init(&qp->ref);
> -		qp->sdev = sdev;
> -		siw_dbg_qp(qp, "new QP\n");
> -	}
> -	return rv;
> +	kref_init(&qp->ref);
> +
> +	old = xa_store(&sdev->qp_xa, qp_id(qp), qp, GFP_KERNEL);
> +	if (xa_is_err(old))
> +		return xa_err(old);
> +
> +	siw_dbg_qp(qp, "new QP\n");
> +
> +	return 0;
>   }
>   
>   void siw_free_qp(struct kref *ref)
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> index 1e1d262a4ae2..ef9fa9c5bf88 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -369,7 +369,7 @@ 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);
> +	rv = siw_qp_reserve_qpn(sdev, qp);
>   	if (rv)
>   		goto err_atomic;
>   
> @@ -482,14 +482,24 @@ int siw_create_qp(struct ib_qp *ibqp, struct ib_qp_init_attr *attrs,
>   		goto err_out_xa;
>   	}
>   	INIT_LIST_HEAD(&qp->devq);
> +	init_completion(&qp->qp_free);
> +
>   	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);
> +	rv = siw_qp_add(sdev, qp);
> +	if (rv)
> +		goto err_out_list;
>   
>   	return 0;
>   
> +err_out_list:
> +	spin_lock_irqsave(&sdev->lock, flags);
> +	list_del(&qp->devq);
> +	spin_unlock_irqrestore(&sdev->lock, flags);
> +
> +	siw_put_tx_cpu(qp->tx_cpu);
>   err_out_xa:
>   	xa_erase(&sdev->qp_xa, qp_id(qp));
>   	if (uctx) {