Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On 03/20/2012 05:39 PM, parav.pan...@emulex.com wrote: +struct ib_mr *ocrdma_get_dma_mr(struct ib_pd *ibpd, int acc) +{ + struct ocrdma_mr *mr; + + mr = ocrdma_alloc_lkey(ibpd, acc, 0, OCRDMA_ADDR_CHECK_DISABLE); + if (!mr) + return ERR_PTR(-ENOMEM); ocrdma_alloc_lkey does not return NULL on error. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On Wed, Mar 21, 2012 at 10:42 AM, frank zago fz...@systemfabricworks.com wrote: On 03/20/2012 05:39 PM, parav.pan...@emulex.com wrote: +struct ib_mr *ocrdma_get_dma_mr(struct ib_pd *ibpd, int acc) +{ + struct ocrdma_mr *mr; + + mr = ocrdma_alloc_lkey(ibpd, acc, 0, OCRDMA_ADDR_CHECK_DISABLE); + if (!mr) + return ERR_PTR(-ENOMEM); ocrdma_alloc_lkey does not return NULL on error. Good catch! Even more to the point, ocrdma_alloc_lkey() doesn't return a struct ocrdma_mr* on success. So this function is totally broken. - R. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
(Resent becasue the first one got lost) On 03/20/2012 05:39 PM, parav.pan...@emulex.com wrote: + +int ocrdma_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr, + struct ib_send_wr **bad_wr) +{ + int status = 0; + struct ocrdma_qp *qp = get_ocrdma_qp(ibqp); + struct ocrdma_hdr_wqe *hdr; + unsigned long flags; + + spin_lock_irqsave(qp-q_lock, flags); + if (qp-state != OCRDMA_QPS_RTS qp-state != OCRDMA_QPS_SQD) { + spin_unlock_irqrestore(qp-q_lock, flags); + return -EINVAL; + } There, and in several places in this function, you return an error without setting bad_wr. Frank. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On 03/20/2012 05:39 PM, parav.pan...@emulex.com wrote: + +int ocrdma_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr, + struct ib_send_wr **bad_wr) +{ + int status = 0; + struct ocrdma_qp *qp = get_ocrdma_qp(ibqp); + struct ocrdma_hdr_wqe *hdr; + unsigned long flags; + + spin_lock_irqsave(qp-q_lock, flags); + if (qp-state != OCRDMA_QPS_RTS qp-state != OCRDMA_QPS_SQD) { + spin_unlock_irqrestore(qp-q_lock, flags); + return -EINVAL; + } There, and in several places in this function, you return an error without setting bad_wr. Frank. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
Inline. -Original Message- From: Roland Dreier [mailto:rol...@purestorage.com] Sent: Wednesday, March 21, 2012 11:27 PM To: frank zago Cc: Pandit, Parav; linux-rdma@vger.kernel.org Subject: Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter On Wed, Mar 21, 2012 at 10:42 AM, frank zago fz...@systemfabricworks.com wrote: On 03/20/2012 05:39 PM, parav.pan...@emulex.com wrote: +struct ib_mr *ocrdma_get_dma_mr(struct ib_pd *ibpd, int acc) { + struct ocrdma_mr *mr; + + mr = ocrdma_alloc_lkey(ibpd, acc, 0, + OCRDMA_ADDR_CHECK_DISABLE); + if (!mr) + return ERR_PTR(-ENOMEM); ocrdma_alloc_lkey does not return NULL on error. I'll fix this part. Good catch! Even more to the point, ocrdma_alloc_lkey() doesn't return a struct ocrdma_mr* on success. So this function is totally broken. It does returns ocrdma_mr* on success. Why do you think it doesn't return? Below is the snippet. status = ocrdma_mbx_alloc_lkey(dev, mr-hwmr, pd-id, addr_check); if (status) { kfree(mr); return ERR_PTR(-ENOMEM); } mr-pd = pd; atomic_inc(pd-use_cnt); mr-ibmr.lkey = mr-hwmr.lkey; if (mr-hwmr.remote_wr || mr-hwmr.remote_rd) mr-ibmr.rkey = mr-hwmr.lkey; return mr; - R. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
-Original Message- From: Roland Dreier [mailto:rol...@purestorage.com] Sent: Wednesday, March 21, 2012 10:13 PM To: Pandit, Parav Cc: linux-rdma@vger.kernel.org; net...@vger.kernel.org Subject: Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter +struct ib_pd *ocrdma_alloc_pd(struct ib_device *ibdev, + struct ib_ucontext *context, + struct ib_udata *udata) { + struct ocrdma_dev *dev = get_ocrdma_dev(ibdev); + struct ocrdma_pd *pd; + int status; + + pd = kzalloc(sizeof(*pd), GFP_KERNEL); + if (!pd) + return ERR_PTR(-ENOMEM); + pd-dev = dev; + if (udata context) { + pd-dpp_enabled = (dev-nic_info.dev_family == + OCRDMA_GEN2_FAMILY) ? true : + false; Writing (bool expr) ? true : false is pretty silly, since it's just an obfuscated way of writing bool expr IOW, you can just write pd-dpp_enabled = (dev-nic_info.dev_family == OCRDMA_GEN2_FAMILY); +int ocrdma_dealloc_pd(struct ib_pd *ibpd) { + struct ocrdma_pd *pd = get_ocrdma_pd(ibpd); + struct ocrdma_dev *dev = pd-dev; + int status; + u64 usr_db; + + if (atomic_read(pd-use_cnt)) { + ocrdma_err(%s(%d) pd=0x%x is in use.\n, + __func__, dev-id, pd-id); + status = -EFAULT; + goto dealloc_err; + } all of the use_cnt tracking in this driver seems to duplicate what the rdma midlayer already does... is there any reason we need that in the low-level hardware driver too, or can we just get rid of the various use_cnt members? This use_cnt can be removed from low-level hardware driver. I'll remove it. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
It does returns ocrdma_mr* on success. Why do you think it doesn't return? Below is the snippet. Sorry, you're absoutely right. I was looking at ocrdma_mbx_alloc_lkey(). - R. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html