* [PATCH v1 for-rc 1/8] RDMA/vmw_pvrdma: Clarify QP and CQ is_kernel logic
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-12-14 0:19 ` Bryan Tan
[not found] ` <20171214001937.GA11378-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-14 0:21 ` [PATCH v1 for-rc 2/8] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path Bryan Tan
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:19 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Be more consistent in setting and checking is_kernel
flag for QPs and CQs.
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_cq.c | 9 ++++-----
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++----
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
index 3562c0c..ea8db5e6 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
@@ -132,8 +132,9 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
}
cq->ibcq.cqe = entries;
+ cq->is_kernel = !context;
- if (context) {
+ if (!cq->is_kernel) {
if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
ret = -EFAULT;
goto err_cq;
@@ -148,8 +149,6 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
npages = ib_umem_page_count(cq->umem);
} else {
- cq->is_kernel = true;
-
/* One extra page for shared ring state */
npages = 1 + (entries * sizeof(struct pvrdma_cqe) +
PAGE_SIZE - 1) / PAGE_SIZE;
@@ -202,7 +201,7 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
dev->cq_tbl[cq->cq_handle % dev->dsr->caps.max_cq] = cq;
spin_unlock_irqrestore(&dev->cq_tbl_lock, flags);
- if (context) {
+ if (!cq->is_kernel) {
cq->uar = &(to_vucontext(context)->uar);
/* Copy udata back. */
@@ -219,7 +218,7 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
err_page_dir:
pvrdma_page_dir_cleanup(dev, &cq->pdir);
err_umem:
- if (context)
+ if (!cq->is_kernel)
ib_umem_release(cq->umem);
err_cq:
atomic_dec(&dev->num_cqs);
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
index 10420a1..b932b7e 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
@@ -249,8 +249,9 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
init_waitqueue_head(&qp->wait);
qp->state = IB_QPS_RESET;
+ qp->is_kernel = !(pd->uobject && udata);
- if (pd->uobject && udata) {
+ if (!qp->is_kernel) {
dev_dbg(&dev->pdev->dev,
"create queuepair from user space\n");
@@ -291,8 +292,6 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
qp->npages_recv = 0;
qp->npages = qp->npages_send + qp->npages_recv;
} else {
- qp->is_kernel = true;
-
ret = pvrdma_set_sq_size(to_vdev(pd->device),
&init_attr->cap, qp);
if (ret)
@@ -394,7 +393,7 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
err_pdir:
pvrdma_page_dir_cleanup(dev, &qp->pdir);
err_umem:
- if (pd->uobject && udata) {
+ if (!qp->is_kernel) {
if (qp->rumem)
ib_umem_release(qp->rumem);
if (qp->sumem)
--
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] 17+ messages in thread
* [PATCH v1 for-rc 2/8] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-14 0:19 ` [PATCH v1 for-rc 1/8] RDMA/vmw_pvrdma: Clarify QP and CQ is_kernel logic Bryan Tan
@ 2017-12-14 0:21 ` Bryan Tan
[not found] ` <20171214002123.GA13402-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-14 0:22 ` [PATCH v1 for-rc 3/8] RDMA/vmw_pvrdma: Use more specific sizeof in kcalloc Bryan Tan
` (6 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:21 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 b932b7e..77e7e57 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
@@ -430,6 +430,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] 17+ messages in thread
* [PATCH v1 for-rc 3/8] RDMA/vmw_pvrdma: Use more specific sizeof in kcalloc
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-14 0:19 ` [PATCH v1 for-rc 1/8] RDMA/vmw_pvrdma: Clarify QP and CQ is_kernel logic Bryan Tan
2017-12-14 0:21 ` [PATCH v1 for-rc 2/8] RDMA/vmw_pvrdma: Call ib_umem_release on destroy QP path Bryan Tan
@ 2017-12-14 0:22 ` Bryan Tan
2017-12-14 0:23 ` [PATCH v1 for-rc 4/8] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning Bryan Tan
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:22 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Convert the sizeof(void *) in two kcalloc calls to be more
specific for the arrays that are being allocated.
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_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index 1f4e187..941e324 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -243,13 +243,13 @@ static int pvrdma_register_device(struct pvrdma_dev *dev)
mutex_init(&dev->port_mutex);
spin_lock_init(&dev->desc_lock);
- dev->cq_tbl = kcalloc(dev->dsr->caps.max_cq, sizeof(void *),
+ dev->cq_tbl = kcalloc(dev->dsr->caps.max_cq, sizeof(struct pvrdma_cq *),
GFP_KERNEL);
if (!dev->cq_tbl)
return ret;
spin_lock_init(&dev->cq_tbl_lock);
- dev->qp_tbl = kcalloc(dev->dsr->caps.max_qp, sizeof(void *),
+ dev->qp_tbl = kcalloc(dev->dsr->caps.max_qp, sizeof(struct pvrdma_qp *),
GFP_KERNEL);
if (!dev->qp_tbl)
goto err_cq_free;
--
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] 17+ messages in thread
* [PATCH v1 for-rc 4/8] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (2 preceding siblings ...)
2017-12-14 0:22 ` [PATCH v1 for-rc 3/8] RDMA/vmw_pvrdma: Use more specific sizeof in kcalloc Bryan Tan
@ 2017-12-14 0:23 ` Bryan Tan
2017-12-14 0:24 ` [PATCH v1 for-rc 5/8] RDMA/vmw_pvrdma: Use refcount_t instead of atomic_t Bryan Tan
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:23 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] 17+ messages in thread
* [PATCH v1 for-rc 5/8] RDMA/vmw_pvrdma: Use refcount_t instead of atomic_t
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (3 preceding siblings ...)
2017-12-14 0:23 ` [PATCH v1 for-rc 4/8] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning Bryan Tan
@ 2017-12-14 0:24 ` Bryan Tan
[not found] ` <20171214002402.GA16082-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-12-14 0:25 ` [PATCH v1 for-rc 6/8] RDMA/vmw_pvrdma: Remove usage of BIT() from UAPI header Bryan Tan
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:24 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
refcount_t is the preferred type for refcounts. Change the
QP and CQ refcnt fields to use refcount_t.
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.h | 4 ++--
drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c | 6 +++---
drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 15 ++++++---------
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 6 +++---
4 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 63bc2ef..07d287e 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
@@ -93,7 +93,7 @@ struct pvrdma_cq {
struct pvrdma_page_dir pdir;
u32 cq_handle;
bool is_kernel;
- atomic_t refcnt;
+ refcount_t refcnt;
wait_queue_head_t wait;
};
@@ -196,7 +196,7 @@ struct pvrdma_qp {
u8 state;
bool is_kernel;
struct mutex mutex; /* QP state mutex. */
- atomic_t refcnt;
+ refcount_t refcnt;
wait_queue_head_t wait;
};
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
index ea8db5e6..9dba949 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
@@ -177,7 +177,7 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
else
pvrdma_page_dir_insert_umem(&cq->pdir, cq->umem, 0);
- atomic_set(&cq->refcnt, 1);
+ refcount_set(&cq->refcnt, 1);
init_waitqueue_head(&cq->wait);
spin_lock_init(&cq->cq_lock);
@@ -229,8 +229,8 @@ 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 (!refcount_dec_and_test(&cq->refcnt))
+ wait_event(cq->wait, !refcount_read(&cq->refcnt));
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 941e324..5cff9fa 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -333,7 +333,7 @@ static void pvrdma_qp_event(struct pvrdma_dev *dev, u32 qpn, int type)
spin_lock_irqsave(&dev->qp_tbl_lock, flags);
qp = dev->qp_tbl[qpn % dev->dsr->caps.max_qp];
if (qp)
- atomic_inc(&qp->refcnt);
+ refcount_inc(&qp->refcnt);
spin_unlock_irqrestore(&dev->qp_tbl_lock, flags);
if (qp && qp->ibqp.event_handler) {
@@ -346,8 +346,7 @@ 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)
+ if (refcount_dec_and_test(&qp->refcnt))
wake_up(&qp->wait);
}
}
@@ -360,7 +359,7 @@ static void pvrdma_cq_event(struct pvrdma_dev *dev, u32 cqn, int type)
spin_lock_irqsave(&dev->cq_tbl_lock, flags);
cq = dev->cq_tbl[cqn % dev->dsr->caps.max_cq];
if (cq)
- atomic_inc(&cq->refcnt);
+ refcount_inc(&cq->refcnt);
spin_unlock_irqrestore(&dev->cq_tbl_lock, flags);
if (cq && cq->ibcq.event_handler) {
@@ -373,8 +372,7 @@ 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)
+ if (refcount_dec_and_test(&cq->refcnt))
wake_up(&cq->wait);
}
}
@@ -533,14 +531,13 @@ static irqreturn_t pvrdma_intrx_handler(int irq, void *dev_id)
spin_lock_irqsave(&dev->cq_tbl_lock, flags);
cq = dev->cq_tbl[cqne->info % dev->dsr->caps.max_cq];
if (cq)
- atomic_inc(&cq->refcnt);
+ refcount_inc(&cq->refcnt);
spin_unlock_irqrestore(&dev->cq_tbl_lock, flags);
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))
+ if (refcount_dec_and_test(&cq->refcnt))
wake_up(&cq->wait);
}
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 77e7e57..9745cb1 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
@@ -245,7 +245,7 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
spin_lock_init(&qp->sq.lock);
spin_lock_init(&qp->rq.lock);
mutex_init(&qp->mutex);
- atomic_set(&qp->refcnt, 1);
+ refcount_set(&qp->refcnt, 1);
init_waitqueue_head(&qp->wait);
qp->state = IB_QPS_RESET;
@@ -427,8 +427,8 @@ 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 (!refcount_dec_and_test(&qp->refcnt))
+ wait_event(qp->wait, !refcount_read(&qp->refcnt));
if (!qp->is_kernel) {
if (qp->rumem)
--
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] 17+ messages in thread
* [PATCH v1 for-rc 6/8] RDMA/vmw_pvrdma: Remove usage of BIT() from UAPI header
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (4 preceding siblings ...)
2017-12-14 0:24 ` [PATCH v1 for-rc 5/8] RDMA/vmw_pvrdma: Use refcount_t instead of atomic_t Bryan Tan
@ 2017-12-14 0:25 ` Bryan Tan
2017-12-14 0:25 ` [PATCH v1 for-rc 7/8] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file Bryan Tan
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:25 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
BIT() should not be used in the UAPI header. Remove it.
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
include/uapi/rdma/vmw_pvrdma-abi.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
index aaa352f..8d37af5 100644
--- a/include/uapi/rdma/vmw_pvrdma-abi.h
+++ b/include/uapi/rdma/vmw_pvrdma-abi.h
@@ -52,12 +52,12 @@
#define PVRDMA_UVERBS_ABI_VERSION 3 /* ABI Version. */
#define PVRDMA_UAR_HANDLE_MASK 0x00FFFFFF /* Bottom 24 bits. */
#define PVRDMA_UAR_QP_OFFSET 0 /* QP doorbell. */
-#define PVRDMA_UAR_QP_SEND BIT(30) /* Send bit. */
-#define PVRDMA_UAR_QP_RECV BIT(31) /* Recv bit. */
+#define PVRDMA_UAR_QP_SEND (1 << 30) /* Send bit. */
+#define PVRDMA_UAR_QP_RECV (1 << 31) /* Recv bit. */
#define PVRDMA_UAR_CQ_OFFSET 4 /* CQ doorbell. */
-#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_CQ_ARM_SOL (1 << 29) /* Arm solicited bit. */
+#define PVRDMA_UAR_CQ_ARM (1 << 30) /* Arm bit. */
+#define PVRDMA_UAR_CQ_POLL (1 << 31) /* Poll 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] 17+ messages in thread
* [PATCH v1 for-rc 7/8] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (5 preceding siblings ...)
2017-12-14 0:25 ` [PATCH v1 for-rc 6/8] RDMA/vmw_pvrdma: Remove usage of BIT() from UAPI header Bryan Tan
@ 2017-12-14 0:25 ` Bryan Tan
2017-12-14 0:26 ` [PATCH v1 for-rc 8/8] RDMA/vmw_pvrdma: Use completion instead of wait queue Bryan Tan
2017-12-18 18:25 ` [PATCH v1 for-rc 0/8] vmw_pvrdma fixes for 4.15 Jason Gunthorpe
8 siblings, 0 replies; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:25 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Add the two UAR SRQ macros that are required by the userlevel
library.
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 8d37af5..02ca0d0 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 (1 << 29) /* Arm solicited bit. */
#define PVRDMA_UAR_CQ_ARM (1 << 30) /* Arm bit. */
#define PVRDMA_UAR_CQ_POLL (1 << 31) /* Poll bit. */
+#define PVRDMA_UAR_SRQ_OFFSET 8 /* SRQ doorbell. */
+#define PVRDMA_UAR_SRQ_RECV (1 << 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] 17+ messages in thread
* [PATCH v1 for-rc 8/8] RDMA/vmw_pvrdma: Use completion instead of wait queue
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (6 preceding siblings ...)
2017-12-14 0:25 ` [PATCH v1 for-rc 7/8] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file Bryan Tan
@ 2017-12-14 0:26 ` Bryan Tan
2017-12-18 18:25 ` [PATCH v1 for-rc 0/8] vmw_pvrdma fixes for 4.15 Jason Gunthorpe
8 siblings, 0 replies; 17+ messages in thread
From: Bryan Tan @ 2017-12-14 0:26 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.
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 | 8 ++++----
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 ++++---
drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c | 7 ++++---
5 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 07d287e..44cb1cf 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;
refcount_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. */
refcount_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 9dba949..faa9478 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
@@ -178,7 +178,7 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
pvrdma_page_dir_insert_umem(&cq->pdir, cq->umem, 0);
refcount_set(&cq->refcnt, 1);
- init_waitqueue_head(&cq->wait);
+ init_completion(&cq->free);
spin_lock_init(&cq->cq_lock);
memset(cmd, 0, sizeof(*cmd));
@@ -229,8 +229,9 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
static void pvrdma_free_cq(struct pvrdma_dev *dev, struct pvrdma_cq *cq)
{
- if (!refcount_dec_and_test(&cq->refcnt))
- wait_event(cq->wait, !refcount_read(&cq->refcnt));
+ if (refcount_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 5cff9fa..939ac2f 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -347,7 +347,7 @@ static void pvrdma_qp_event(struct pvrdma_dev *dev, u32 qpn, int type)
}
if (qp) {
if (refcount_dec_and_test(&qp->refcnt))
- wake_up(&qp->wait);
+ complete(&qp->free);
}
}
@@ -373,7 +373,7 @@ static void pvrdma_cq_event(struct pvrdma_dev *dev, u32 cqn, int type)
}
if (cq) {
if (refcount_dec_and_test(&cq->refcnt))
- wake_up(&cq->wait);
+ complete(&cq->free);
}
}
@@ -402,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);
}
}
@@ -538,7 +538,7 @@ static irqreturn_t pvrdma_intrx_handler(int irq, void *dev_id)
cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);
if (cq) {
if (refcount_dec_and_test(&cq->refcnt))
- wake_up(&cq->wait);
+ 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 9745cb1..7bf518b 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);
refcount_set(&qp->refcnt, 1);
- init_waitqueue_head(&qp->wait);
+ init_completion(&qp->free);
qp->state = IB_QPS_RESET;
qp->is_kernel = !(pd->uobject && udata);
@@ -427,8 +427,9 @@ static void pvrdma_free_qp(struct pvrdma_qp *qp)
pvrdma_unlock_cqs(scq, rcq, &scq_flags, &rcq_flags);
- if (!refcount_dec_and_test(&qp->refcnt))
- wait_event(qp->wait, !refcount_read(&qp->refcnt));
+ if (refcount_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] 17+ messages in thread
* Re: [PATCH v1 for-rc 0/8] vmw_pvrdma fixes for 4.15
[not found] ` <20171214001753.GA9780-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
` (7 preceding siblings ...)
2017-12-14 0:26 ` [PATCH v1 for-rc 8/8] RDMA/vmw_pvrdma: Use completion instead of wait queue Bryan Tan
@ 2017-12-18 18:25 ` Jason Gunthorpe
[not found] ` <20171218182511.GH19070-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
8 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2017-12-18 18:25 UTC (permalink / raw)
To: Bryan Tan; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
On Wed, Dec 13, 2017 at 04:17:59PM -0800, Bryan Tan wrote:
>
> Do some cleanup and fixes related to the comments received during SRQ
> support upstreaming that applied to existing code. Also add in two
> missed macros used by the userlevel library for SRQs, a missed
> ib_umem_release, an incorrect usage of the new refcount_t type,
> and switched from use of wait queues to completions.
>
> 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
This series would be fine for -next, the patches look OK to me now.
But there are too many non-rc things to be for-rc.
If you want this in -rc for some reason then you need to send a series
with just those commits, otherwise it will go to -next.
These fix actual bugs and could be for-rc
> 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: Use completion instead of wait queue
These are clearly not for-rc:
> RDMA/vmw_pvrdma: Use more specific sizeof in kcalloc
> RDMA/vmw_pvrdma: Use refcount_t instead of atomic_t
Does this fix a bug or is it just a style change? Can't tell:
> RDMA/vmw_pvrdma: Clarify QP and CQ is_kernel logic
These two are boarder line. Would need a better commit message explaining
why these need to be in -rc:
> RDMA/vmw_pvrdma: Remove usage of BIT() from UAPI header
> RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file
Let me know what you want.
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] 17+ messages in thread