On Wed, Oct 31, 2018 at 02:42:41PM +0200, Shamir Rabinovitch wrote:
> Next patch will replace usage of pd->uobject with rdma_is_user_pd
> function. As suggested by Jason, some of the code was miss using
> the pd->uobject pointer in places it should have not been used and
> better alternative exists. Fix this just before we do the replace.
>
> Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
> drivers/infiniband/hw/i40iw/i40iw_verbs.c | 42 ++++++++++++++----------------
> drivers/infiniband/hw/mlx4/qp.c | 6 ++---
> drivers/infiniband/hw/mlx5/qp.c | 4 +--
> 4 files changed, 26 insertions(+), 28 deletions(-)
You need to cc driver patches like this to the driver maintainers..
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index ca05810..00170aa 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -3926,7 +3926,7 @@ int hns_roce_v1_destroy_qp(struct ib_qp *ibqp)
> struct hns_roce_qp_work *qp_work;
> struct hns_roce_v1_priv *priv;
> struct hns_roce_cq *send_cq, *recv_cq;
> - int is_user = !!ibqp->pd->uobject;
> + bool is_user = ibqp->pd->uobject;
This should be written as ibqp->uobject
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index 1028758..9d3178d 100644
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -673,28 +673,26 @@ static struct ib_qp *i40iw_create_qp(struct ib_pd *ibpd,
> goto error;
> }
> iwqp->ctx_info.qp_compl_ctx = req.user_compl_ctx;
> - if (ibpd->uobject && ibpd->uobject->context) {
> - iwqp->user_mode = 1;
> - ucontext = to_ucontext(ibpd->uobject->context);
> -
> - if (req.user_wqe_buffers) {
> - struct i40iw_pbl *iwpbl;
> -
> - spin_lock_irqsave(
> - &ucontext->qp_reg_mem_list_lock, flags);
> - iwpbl = i40iw_get_pbl(
> - (unsigned long)req.user_wqe_buffers,
> - &ucontext->qp_reg_mem_list);
> - spin_unlock_irqrestore(
> - &ucontext->qp_reg_mem_list_lock, flags);
> -
> - if (!iwpbl) {
> - err_code = -ENODATA;
> - i40iw_pr_err("no pbl info\n");
> - goto error;
> - }
> - memcpy(&iwqp->iwpbl, iwpbl, sizeof(iwqp->iwpbl));
> + iwqp->user_mode = 1;
> + ucontext = to_ucontext(ibpd->uobject->context);
It is a bit odd, but I think it is OK to require that udata implies
ibpd->uobject, and the next series that changes this to get the
ucontext from the udata will tidy that assumption..
> diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> index 0711ca1..c783539 100644
> +++ b/drivers/infiniband/hw/mlx4/qp.c
> @@ -1189,7 +1189,7 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
> if (qp->mlx4_ib_qp_type == MLX4_IB_QPT_PROXY_GSI)
> free_proxy_bufs(pd->device, qp);
> err_wrid:
> - if (pd->uobject) {
> + if (qp->umem) {
Why not use 'if (udata)' here, and also above?
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index 6841c0f..87e1675 100644
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -1876,7 +1876,7 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
> qp->flags |= MLX5_IB_QP_CVLAN_STRIPPING;
> }
>
> - if (pd && pd->uobject) {
> + if (udata) {
> if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
> mlx5_ib_dbg(dev, "copy failed\n");
> return -EFAULT;
Like here..
Jason
