| Message ID | 20171208190010.GA31023@bryantan-devbox.prom.eng.vmware.com.prom.eng.vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Dec 08, 2017 at 11:00:17AM -0800, Bryan Tan wrote: > Be more consistent in checking is_kernel flag for QPs. Consist with what? (asking because expecting also pvrdma_create_cq to be fixed). > > Testing Done: ibv_rc_pingpong, rping, perftests. These are all userspace tests, right? > > Reviewed-by: Adit Ranadive <aditr@vmware.com> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > Signed-off-by: Bryan Tan <bryantan@vmware.com> > --- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > 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) Regardless of the comments: Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > -- > 1.8.5.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 13, 2017 at 11:02:46AM +0200, Yuval Shaia wrote: > On Fri, Dec 08, 2017 at 11:00:17AM -0800, Bryan Tan wrote: > > Be more consistent in checking is_kernel flag for QPs. > > Consist with what? > (asking because expecting also pvrdma_create_cq to be fixed). I had updated create SRQ's is_kernel in the same way. Thanks for pointing out that I had missed it in create CQ. > > > > Testing Done: ibv_rc_pingpong, rping, perftests. > > These are all userspace tests, right? Yes. That was accidentally carried over from our how we format our commits internally. I'll remove the "Testing Done" line here and in the next commit message. > > > > Reviewed-by: Adit Ranadive <aditr@vmware.com> > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > Signed-off-by: Bryan Tan <bryantan@vmware.com> > > --- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > 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) > > Regardless of the comments: > > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > > > -- > > 1.8.5.6 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=MjCVwEZlX7H-yXFOZ8b3IdkUDz9tjnFu-2RVvpFiKHw&m=M9b99UdgNMHlndgR9oY0KSFjeYht9EpUHM4iqxKTcUE&s=d8rmC5ZUNj2gLH8KOcrCgMmMg6484BYoXivYSjKEha0&e= -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 13, 2017 at 11:08:26AM -0800, Bryan Tan wrote: > On Wed, Dec 13, 2017 at 11:02:46AM +0200, Yuval Shaia wrote: > > On Fri, Dec 08, 2017 at 11:00:17AM -0800, Bryan Tan wrote: > > > Be more consistent in checking is_kernel flag for QPs. > > > > Consist with what? > > (asking because expecting also pvrdma_create_cq to be fixed). > > I had updated create SRQ's is_kernel in the same way. Thanks for > pointing out that I had missed it in create CQ. > > > > > > > Testing Done: ibv_rc_pingpong, rping, perftests. > > > > These are all userspace tests, right? > > Yes. That was accidentally carried over from our how we > format our commits internally. I'll remove the "Testing Done" > line here and in the next commit message. But...since you exposed this info my question remains, you changed a flow of a branch that checks if verb was requested by kernel or userspace but you tested it only with userspace tools. > > > > > > > Reviewed-by: Adit Ranadive <aditr@vmware.com> > > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > > Signed-off-by: Bryan Tan <bryantan@vmware.com> > > > --- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > 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) > > > > Regardless of the comments: > > > > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > > > > > -- > > > 1.8.5.6 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=MjCVwEZlX7H-yXFOZ8b3IdkUDz9tjnFu-2RVvpFiKHw&m=M9b99UdgNMHlndgR9oY0KSFjeYht9EpUHM4iqxKTcUE&s=d8rmC5ZUNj2gLH8KOcrCgMmMg6484BYoXivYSjKEha0&e= -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 13, 2017 at 09:20:09PM +0200, Yuval Shaia wrote: > On Wed, Dec 13, 2017 at 11:08:26AM -0800, Bryan Tan wrote: > > On Wed, Dec 13, 2017 at 11:02:46AM +0200, Yuval Shaia wrote: > > > On Fri, Dec 08, 2017 at 11:00:17AM -0800, Bryan Tan wrote: > > > > Be more consistent in checking is_kernel flag for QPs. > > > > > > Consist with what? > > > (asking because expecting also pvrdma_create_cq to be fixed). > > > > I had updated create SRQ's is_kernel in the same way. Thanks for > > pointing out that I had missed it in create CQ. > > > > > > > > > > Testing Done: ibv_rc_pingpong, rping, perftests. > > > > > > These are all userspace tests, right? > > > > Yes. That was accidentally carried over from our how we > > format our commits internally. I'll remove the "Testing Done" > > line here and in the next commit message. > > But...since you exposed this info my question remains, you changed a flow > of a branch that checks if verb was requested by kernel or userspace but > you tested it only with userspace tools. Given that it was an if-else and the nature of the change I felt it sufficient to test with userspace applications. A kernel QP is also created for CM, which is used in rping. I just tested the patch series with krping though, and no issues there. > > > > > > > > > > Reviewed-by: Adit Ranadive <aditr@vmware.com> > > > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > > > Signed-off-by: Bryan Tan <bryantan@vmware.com> > > > > --- > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 7 +++---- > > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > > > 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) > > > > > > Regardless of the comments: > > > > > > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > > > > > > > -- > > > > 1.8.5.6 > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > > > the body of a message to majordomo@vger.kernel.org > > > > More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=MjCVwEZlX7H-yXFOZ8b3IdkUDz9tjnFu-2RVvpFiKHw&m=M9b99UdgNMHlndgR9oY0KSFjeYht9EpUHM4iqxKTcUE&s=d8rmC5ZUNj2gLH8KOcrCgMmMg6484BYoXivYSjKEha0&e= -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)