On Thu, Aug 25, 2016 at 07:16:32 -0700, Yuval Shaia wrote:
> On Wed, Aug 03, 2016 at 04:27:39PM -0700, Adit Ranadive wrote:
> > This patch adds support for creating and destroying memory regions.
> > The PVRDMA device supports User MRs, DMA MRs (no Remote
> Read/Write
> > support), Fast Register MRs.
> >
> > Changes v2->v3:
> > - Removed boolean in pvrdma_cmd_post.
> >
> > Reviewed-by: Jorgen Hansen <jhansen@xxxxxxxxxx>
> > Reviewed-by: George Zhang <georgezhang@xxxxxxxxxx>
> > Reviewed-by: Aditya Sarwade <asarwade@xxxxxxxxxx>
> > Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx>
> > Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/pvrdma/pvrdma_mr.c | 331
> > +++++++++++++++++++++++++++++++
> > 1 file changed, 331 insertions(+)
> > create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_mr.c
> >
...
> > +/**
> > + * pvrdma_get_dma_mr - get a DMA memory region
> > + * @pd: protection domain
> > + * @acc: access flags
> > + *
> > + * @return: ib_mr pointer on success, otherwise returns an errno.
> > + */
> > +struct ib_mr *pvrdma_get_dma_mr(struct ib_pd *pd, int acc) {
> > + struct pvrdma_dev *dev = to_vdev(pd->device);
> > + struct pvrdma_user_mr *mr;
> > + union pvrdma_cmd_req req;
> > + union pvrdma_cmd_resp rsp;
> > + struct pvrdma_cmd_create_mr *cmd = &req.create_mr;
> > + struct pvrdma_cmd_create_mr_resp *resp = &rsp.create_mr_resp;
> > + int ret;
> > +
> > + if (acc != IB_ACCESS_LOCAL_WRITE) {
>
> Since those access flags are bits i suggest to use bit operations.
> if (!(acc & IB_ACCESS_LOCAL_WRITE))
Ok.
> > + dev_warn(&dev->pdev->dev, "unsupported dma mr access
> flags\n");
> > + return ERR_PTR(-EOPNOTSUPP);
> > + }
> > +
> > + mr = kzalloc(sizeof(*mr), GFP_KERNEL);
> > + if (!mr)
> > + return ERR_PTR(-ENOMEM);
>
> Suggesting to do this allocation after command succeeds. This way you will
> not have to deal with free-ing it when cmd fails.
Hmm, I kept it as it is in upcoming v4 mainly because if the mr alloc failed I would
have to issue another device command to destroy the MR. I would rather free the
MR here than issue a device command to destroy it.
Thanks,
Adit
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
