On Sun, Sep 11, 2016 at 09:49:15PM -0700, Adit Ranadive wrote: > + > +/** > + * pvrdma_alloc_pd - allocate protection domain > + * @ibdev: the IB device > + * @context: user context > + * @udata: user data > + * > + * @return: the ib_pd protection domain pointer on success, otherwise errno. > + */ > +struct ib_pd *pvrdma_alloc_pd(struct ib_device *ibdev, > + struct ib_ucontext *context, > + struct ib_udata *udata) > +{ > + struct pvrdma_pd *pd; > + struct pvrdma_dev *dev = to_vdev(ibdev); > + union pvrdma_cmd_req req; > + union pvrdma_cmd_resp rsp; > + struct pvrdma_cmd_create_pd *cmd = &req.create_pd; > + struct pvrdma_cmd_create_pd_resp *resp = &rsp.create_pd_resp; > + int ret; > + void *ptr; > + > + /* Check allowed max pds */ > + if (!atomic_add_unless(&dev->num_pds, 1, dev->dsr->caps.max_pd)) > + return ERR_PTR(-EINVAL); > + > + memset(cmd, 0, sizeof(*cmd)); > + cmd->hdr.cmd = PVRDMA_CMD_CREATE_PD; > + cmd->ctx_handle = (context) ? to_vucontext(context)->ctx_handle : 0; > + ret = pvrdma_cmd_post(dev, &req, &rsp); > + if (ret < 0) { > + dev_warn(&dev->pdev->dev, > + "failed to allocate protection domain, error: %d\n", > + ret); > + ptr = ERR_PTR(ret); > + goto err; > + } else if (resp->hdr.ack != PVRDMA_CMD_CREATE_PD_RESP) { > + dev_warn(&dev->pdev->dev, > + "unknown response for allocate protection domain\n"); > + ptr = ERR_PTR(-EFAULT); > + goto err; > + } > + > + pd = kmalloc(sizeof(*pd), GFP_KERNEL); > + if (!pd) { > + ptr = ERR_PTR(-ENOMEM); > + goto err; > + } I know that this was my suggestion but also remember that you raised a (correct) argument that it is preferred to first do other allocation and free them if command fails then the other way around where failure of memory allocation (like here) will force us to do the opposite command (pvrdma_dealloc_pd in this case). So either accept your way (better) or call pvrdma_dealloc_pd when kmalloc fails. > + > + pd->privileged = !context; > + pd->pd_handle = resp->pd_handle; > + pd->pdn = resp->pd_handle; > + > + if (context) { > + if (ib_copy_to_udata(udata, &pd->pdn, sizeof(__u32))) { > + dev_warn(&dev->pdev->dev, > + "failed to copy back protection domain\n"); > + pvrdma_dealloc_pd(&pd->ibpd); > + return ERR_PTR(-EFAULT); > + } > + } > + > + /* u32 pd handle */ > + return &pd->ibpd; > + > +err: > + atomic_dec(&dev->num_pds); > + return ptr; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html