linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15
@ 2017-12-20 17:47 Bryan Tan
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-12-20 17:47 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Add in two missed macros used by the userlevel library for SRQs,
fix a missed ib_umem_release for QPs, fix an incorrect usage of
the new refcount_t type for SRQs, and fix a potential use after
free.

v1 -> v2 changelog:
- Remove patches that are not for-rc material
- Clarify commit message for the SRQ macros patch

v0 -> v1 changelog:
- Removed use of BIT() in UAPI header
- Make setting/usage of is_kernel consistent between QP/CQ/SRQ
- Use completions instead of wait queues for resource destroy
- Cleaned up commit messages

Bryan Tan (4):
  RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path
  RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning
  RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
  RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy

 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h      |  6 +++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c   |  7 ++++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 17 +++++++----------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c   | 14 +++++++++++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c  |  7 ++++---
 include/uapi/rdma/vmw_pvrdma-abi.h             |  2 ++
 6 files changed, 31 insertions(+), 22 deletions(-)

-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 for-rc 1/4] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-12-20 17:49   ` Bryan Tan
  2017-12-20 17:50   ` [PATCH v2 for-rc 2/4] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning Bryan Tan
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Bryan Tan @ 2017-12-20 17:49 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

The QP cleanup did not previously call ib_umem_release. Fix this.

Fixes: 29c8d9eba550 ("IB: Add vmw_pvrdma driver")
Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
index 10420a1..dceebc6 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
@@ -431,6 +431,13 @@ static void pvrdma_free_qp(struct pvrdma_qp *qp)
 	atomic_dec(&qp->refcnt);
 	wait_event(qp->wait, !atomic_read(&qp->refcnt));
 
+	if (!qp->is_kernel) {
+		if (qp->rumem)
+			ib_umem_release(qp->rumem);
+		if (qp->sumem)
+			ib_umem_release(qp->sumem);
+	}
+
 	pvrdma_page_dir_cleanup(dev, &qp->pdir);
 
 	kfree(qp);
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 for-rc 2/4] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-20 17:49   ` [PATCH v2 for-rc 1/4] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path Bryan Tan
@ 2017-12-20 17:50   ` Bryan Tan
       [not found]     ` <20171220174956.GA20571-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-20 17:50   ` [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file Bryan Tan
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-12-20 17:50 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

refcount_dec generates a warning when the operation
causes the refcount to hit zero. Avoid this by using
refcount_dec_and_test.

Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
index 826ccb8..a2b1a3c 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
@@ -236,8 +236,8 @@ static void pvrdma_free_srq(struct pvrdma_dev *dev, struct pvrdma_srq *srq)
 	dev->srq_tbl[srq->srq_handle] = NULL;
 	spin_unlock_irqrestore(&dev->srq_tbl_lock, flags);
 
-	refcount_dec(&srq->refcnt);
-	wait_event(srq->wait, !refcount_read(&srq->refcnt));
+	if (!refcount_dec_and_test(&srq->refcnt))
+		wait_event(srq->wait, !refcount_read(&srq->refcnt));
 
 	/* There is no support for kernel clients, so this is safe. */
 	ib_umem_release(srq->umem);
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-20 17:49   ` [PATCH v2 for-rc 1/4] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path Bryan Tan
  2017-12-20 17:50   ` [PATCH v2 for-rc 2/4] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning Bryan Tan
@ 2017-12-20 17:50   ` Bryan Tan
       [not found]     ` <20171220175051.GA22751-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-20 17:51   ` [PATCH v2 for-rc 4/4] RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy Bryan Tan
  2017-12-22  0:43   ` [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15 Jason Gunthorpe
  4 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-12-20 17:50 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Support for SRQs were added in the vmw_pvrdma userlevel library
before two necessary macros were added into the kernel ABI header
file. Add the two UAR SRQ macros that are required by the userlevel
library so that the library can rely on the kernel ABI header file
for these SRQ macro definitions.

Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
index aaa352f..4007cac 100644
--- a/include/uapi/rdma/vmw_pvrdma-abi.h
+++ b/include/uapi/rdma/vmw_pvrdma-abi.h
@@ -58,6 +58,8 @@
 #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
 #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
 #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
+#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
+#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
 
 enum pvrdma_wr_opcode {
 	PVRDMA_WR_RDMA_WRITE,
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 for-rc 4/4] RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
                     ` (2 preceding siblings ...)
  2017-12-20 17:50   ` [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file Bryan Tan
@ 2017-12-20 17:51   ` Bryan Tan
  2017-12-22  0:43   ` [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15 Jason Gunthorpe
  4 siblings, 0 replies; 11+ messages in thread
From: Bryan Tan @ 2017-12-20 17:51 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

The use of wait queues in vmw_pvrdma for handling concurrent
access to a resource leaves a possible use after free bug.
Fix this by using completions instead, and also using
atomic_dec_and_test to ensure complete() is called only once.

Fixes: 29c8d9eba550 ("IB: Add vmw_pvrdma driver")
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h      |  6 +++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c   |  7 ++++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 17 +++++++----------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c   |  7 ++++---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c  |  7 ++++---
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 63bc2ef..4f7bd3b6 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
@@ -94,7 +94,7 @@ struct pvrdma_cq {
 	u32 cq_handle;
 	bool is_kernel;
 	atomic_t refcnt;
-	wait_queue_head_t wait;
+	struct completion free;
 };
 
 struct pvrdma_id_table {
@@ -175,7 +175,7 @@ struct pvrdma_srq {
 	u32 srq_handle;
 	int npages;
 	refcount_t refcnt;
-	wait_queue_head_t wait;
+	struct completion free;
 };
 
 struct pvrdma_qp {
@@ -197,7 +197,7 @@ struct pvrdma_qp {
 	bool is_kernel;
 	struct mutex mutex; /* QP state mutex. */
 	atomic_t refcnt;
-	wait_queue_head_t wait;
+	struct completion free;
 };
 
 struct pvrdma_dev {
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
index 3562c0c..e529622 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
@@ -179,7 +179,7 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
 		pvrdma_page_dir_insert_umem(&cq->pdir, cq->umem, 0);
 
 	atomic_set(&cq->refcnt, 1);
-	init_waitqueue_head(&cq->wait);
+	init_completion(&cq->free);
 	spin_lock_init(&cq->cq_lock);
 
 	memset(cmd, 0, sizeof(*cmd));
@@ -230,8 +230,9 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
 
 static void pvrdma_free_cq(struct pvrdma_dev *dev, struct pvrdma_cq *cq)
 {
-	atomic_dec(&cq->refcnt);
-	wait_event(cq->wait, !atomic_read(&cq->refcnt));
+	if (atomic_dec_and_test(&cq->refcnt))
+		complete(&cq->free);
+	wait_for_completion(&cq->free);
 
 	if (!cq->is_kernel)
 		ib_umem_release(cq->umem);
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index 1f4e187..e926818 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -346,9 +346,8 @@ static void pvrdma_qp_event(struct pvrdma_dev *dev, u32 qpn, int type)
 		ibqp->event_handler(&e, ibqp->qp_context);
 	}
 	if (qp) {
-		atomic_dec(&qp->refcnt);
-		if (atomic_read(&qp->refcnt) == 0)
-			wake_up(&qp->wait);
+		if (atomic_dec_and_test(&qp->refcnt))
+			complete(&qp->free);
 	}
 }
 
@@ -373,9 +372,8 @@ static void pvrdma_cq_event(struct pvrdma_dev *dev, u32 cqn, int type)
 		ibcq->event_handler(&e, ibcq->cq_context);
 	}
 	if (cq) {
-		atomic_dec(&cq->refcnt);
-		if (atomic_read(&cq->refcnt) == 0)
-			wake_up(&cq->wait);
+		if (atomic_dec_and_test(&cq->refcnt))
+			complete(&cq->free);
 	}
 }
 
@@ -404,7 +402,7 @@ static void pvrdma_srq_event(struct pvrdma_dev *dev, u32 srqn, int type)
 	}
 	if (srq) {
 		if (refcount_dec_and_test(&srq->refcnt))
-			wake_up(&srq->wait);
+			complete(&srq->free);
 	}
 }
 
@@ -539,9 +537,8 @@ static irqreturn_t pvrdma_intrx_handler(int irq, void *dev_id)
 		if (cq && cq->ibcq.comp_handler)
 			cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
 		if (cq) {
-			atomic_dec(&cq->refcnt);
-			if (atomic_read(&cq->refcnt))
-				wake_up(&cq->wait);
+			if (atomic_dec_and_test(&cq->refcnt))
+				complete(&cq->free);
 		}
 		pvrdma_idx_ring_inc(&ring->cons_head, ring_slots);
 	}
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
index dceebc6..4059308 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
@@ -246,7 +246,7 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
 		spin_lock_init(&qp->rq.lock);
 		mutex_init(&qp->mutex);
 		atomic_set(&qp->refcnt, 1);
-		init_waitqueue_head(&qp->wait);
+		init_completion(&qp->free);
 
 		qp->state = IB_QPS_RESET;
 
@@ -428,8 +428,9 @@ static void pvrdma_free_qp(struct pvrdma_qp *qp)
 
 	pvrdma_unlock_cqs(scq, rcq, &scq_flags, &rcq_flags);
 
-	atomic_dec(&qp->refcnt);
-	wait_event(qp->wait, !atomic_read(&qp->refcnt));
+	if (atomic_dec_and_test(&qp->refcnt))
+		complete(&qp->free);
+	wait_for_completion(&qp->free);
 
 	if (!qp->is_kernel) {
 		if (qp->rumem)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
index a2b1a3c..5acebb1 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c
@@ -149,7 +149,7 @@ struct ib_srq *pvrdma_create_srq(struct ib_pd *pd,
 
 	spin_lock_init(&srq->lock);
 	refcount_set(&srq->refcnt, 1);
-	init_waitqueue_head(&srq->wait);
+	init_completion(&srq->free);
 
 	dev_dbg(&dev->pdev->dev,
 		"create shared receive queue from user space\n");
@@ -236,8 +236,9 @@ static void pvrdma_free_srq(struct pvrdma_dev *dev, struct pvrdma_srq *srq)
 	dev->srq_tbl[srq->srq_handle] = NULL;
 	spin_unlock_irqrestore(&dev->srq_tbl_lock, flags);
 
-	if (!refcount_dec_and_test(&srq->refcnt))
-		wait_event(srq->wait, !refcount_read(&srq->refcnt));
+	if (refcount_dec_and_test(&srq->refcnt))
+		complete(&srq->free);
+	wait_for_completion(&srq->free);
 
 	/* There is no support for kernel clients, so this is safe. */
 	ib_umem_release(srq->umem);
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 2/4] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning
       [not found]     ` <20171220174956.GA20571-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-12-20 19:55       ` Leon Romanovsky
  0 siblings, 0 replies; 11+ messages in thread
From: Leon Romanovsky @ 2017-12-20 19:55 UTC (permalink / raw)
  To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

On Wed, Dec 20, 2017 at 09:50:01AM -0800, Bryan Tan wrote:
> refcount_dec generates a warning when the operation
> causes the refcount to hit zero. Avoid this by using
> refcount_dec_and_test.
>
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
       [not found]     ` <20171220175051.GA22751-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-12-21 12:59       ` Leon Romanovsky
  2017-12-22 16:17       ` Jason Gunthorpe
  2017-12-28  4:48       ` Jason Gunthorpe
  2 siblings, 0 replies; 11+ messages in thread
From: Leon Romanovsky @ 2017-12-21 12:59 UTC (permalink / raw)
  To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
>
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> index aaa352f..4007cac 100644
> --- a/include/uapi/rdma/vmw_pvrdma-abi.h
> +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> @@ -58,6 +58,8 @@
>  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
>  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
>  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
>

Didn't we see that BIT() in UAPI is a wrong thing to do? Why don't you
add normal (1 << 30) ?

>  enum pvrdma_wr_opcode {
>  	PVRDMA_WR_RDMA_WRITE,
> --
> 1.8.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15
       [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
                     ` (3 preceding siblings ...)
  2017-12-20 17:51   ` [PATCH v2 for-rc 4/4] RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy Bryan Tan
@ 2017-12-22  0:43   ` Jason Gunthorpe
  4 siblings, 0 replies; 11+ messages in thread
From: Jason Gunthorpe @ 2017-12-22  0:43 UTC (permalink / raw)
  To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 20, 2017 at 09:47:57AM -0800, Bryan Tan wrote:
> Add in two missed macros used by the userlevel library for SRQs,
> fix a missed ib_umem_release for QPs, fix an incorrect usage of
> the new refcount_t type for SRQs, and fix a potential use after
> free.
> 
> v1 -> v2 changelog:
> - Remove patches that are not for-rc material
> - Clarify commit message for the SRQ macros patch
> 
> v0 -> v1 changelog:
> - Removed use of BIT() in UAPI header
> - Make setting/usage of is_kernel consistent between QP/CQ/SRQ
> - Use completions instead of wait queues for resource destroy
> - Cleaned up commit messages
> 
> Bryan Tan (4):
>   RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path
>   RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning
>   RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy

I applied everything to for-rc except this one:

   RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file

I'll probably take both ABI header changes to for-rc later..

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
       [not found]     ` <20171220175051.GA22751-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-21 12:59       ` Leon Romanovsky
@ 2017-12-22 16:17       ` Jason Gunthorpe
       [not found]         ` <20171222161724.GC30884-uk2M96/98Pc@public.gmane.org>
  2017-12-28  4:48       ` Jason Gunthorpe
  2 siblings, 1 reply; 11+ messages in thread
From: Jason Gunthorpe @ 2017-12-22 16:17 UTC (permalink / raw)
  To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
> 
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> index aaa352f..4007cac 100644
> +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> @@ -58,6 +58,8 @@
>  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
>  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
>  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */

Thinking about this.. These constants are not used in the kernel, so
what are they for, and why are they a 'uapi' header?

I'm guessing this is the 'PCI BAR' ABI and the kernel does't use UAR?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
       [not found]     ` <20171220175051.GA22751-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  2017-12-21 12:59       ` Leon Romanovsky
  2017-12-22 16:17       ` Jason Gunthorpe
@ 2017-12-28  4:48       ` Jason Gunthorpe
  2 siblings, 0 replies; 11+ messages in thread
From: Jason Gunthorpe @ 2017-12-28  4:48 UTC (permalink / raw)
  To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
> 
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)

I decided to put this into for-next, since I really didn't think I
could defend it in -rc, as the kernel doesn't need or use the header
contents.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
       [not found]         ` <20171222161724.GC30884-uk2M96/98Pc@public.gmane.org>
@ 2017-12-28 17:38           ` Adit Ranadive
  0 siblings, 0 replies; 11+ messages in thread
From: Adit Ranadive @ 2017-12-28 17:38 UTC (permalink / raw)
  To: Jason Gunthorpe, Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Fri, Dec 22, 2017 at 09:17:24AM -0700, Jason Gunthorpe wrote:
> On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> > Support for SRQs were added in the vmw_pvrdma userlevel library
> > before two necessary macros were added into the kernel ABI header
> > file. Add the two UAR SRQ macros that are required by the userlevel
> > library so that the library can rely on the kernel ABI header file
> > for these SRQ macro definitions.
> > 
> > Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> > Reviewed-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> >  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> > index aaa352f..4007cac 100644
> > +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> > @@ -58,6 +58,8 @@
> >  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
> >  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
> >  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> > +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> > +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
> 
> Thinking about this.. These constants are not used in the kernel, so
> what are they for, and why are they a 'uapi' header?
> 
> I'm guessing this is the 'PCI BAR' ABI and the kernel does't use UAR?

Hi Jason,

Sorry for the late reply on this thread. We do create a UAR page for the kernel
clients to use. Currently, we don't use the SRQ bits in the kernel and only in 
userspace. But all the other offsets, bits are used when writing to the driver
specific UAR page.

Thanks,
Adit
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-12-28 17:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-20 17:47 [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15 Bryan Tan
     [not found] ` <20171220174748.GA18149-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-20 17:49   ` [PATCH v2 for-rc 1/4] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path Bryan Tan
2017-12-20 17:50   ` [PATCH v2 for-rc 2/4] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning Bryan Tan
     [not found]     ` <20171220174956.GA20571-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-20 19:55       ` Leon Romanovsky
2017-12-20 17:50   ` [PATCH v2 for-rc 3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file Bryan Tan
     [not found]     ` <20171220175051.GA22751-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-21 12:59       ` Leon Romanovsky
2017-12-22 16:17       ` Jason Gunthorpe
     [not found]         ` <20171222161724.GC30884-uk2M96/98Pc@public.gmane.org>
2017-12-28 17:38           ` Adit Ranadive
2017-12-28  4:48       ` Jason Gunthorpe
2017-12-20 17:51   ` [PATCH v2 for-rc 4/4] RDMA/vmw_pvrdma: Avoid use after free due to QP/CQ/SRQ destroy Bryan Tan
2017-12-22  0:43   ` [PATCH v2 for-rc 0/4] vmw_pvrdma fixes for 4.15 Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).