On Sun, Nov 04, 2018 at 02:20:51PM +0200, Shamir Rabinovitch wrote: > On Sun, Nov 04, 2018 at 10:36:32AM +0200, Shamir Rabinovitch wrote: > > On Wed, Oct 31, 2018 at 11:55:15AM -0600, Jason Gunthorpe wrote: > > > On Wed, Oct 31, 2018 at 02:42:42PM +0200, Shamir Rabinovitch wrote: > > > > Now we have the ability to tell from ib_pd if ib_pd was > > > > created by user/kernel verbs. Stop using the ib_pd->uobject > > > > pointer for this. This patch prepare the ib_pb uobject pointer > > > > removal that will happen in another patch. > > > > [...] > > > > +++ 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; > > > > - bool is_user = ibqp->pd->uobject; > > > > + bool is_user = rdma_is_user_pd(ibqp->pd); > > > > int is_timeout = 0; > > > > int ret; > > > > > > ibqp->uboject > > > > This assume that qp will never be shared? I thought we try to > > avoid having ib_x uobject pointer. Do you still prefer this rather then > > just ask rdma_is_user_pd which does not involve ib_qp uobject pointer? > > > > > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > > index a4c62ae..8c659c1 100644 > > > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > > @@ -4096,7 +4096,8 @@ static int hns_roce_v2_destroy_qp(struct ib_qp *ibqp) > > > > struct hns_roce_qp *hr_qp = to_hr_qp(ibqp); > > > > int ret; > > > > > > > > - ret = hns_roce_v2_destroy_qp_common(hr_dev, hr_qp, !!ibqp->pd->uobject); > > > > + ret = hns_roce_v2_destroy_qp_common(hr_dev, hr_qp, rdma_is_user_pd(ibqp->pd)); > > > > > > ibqp->uboject > > > > Same question as above > > > > > [...] > > > > @@ -1612,7 +1612,8 @@ static int _mlx4_ib_destroy_qp(struct ib_qp *qp) > > > > struct mlx4_ib_pd *pd; > > > > > > > > pd = get_pd(mqp); > > > > - destroy_qp_common(dev, mqp, MLX4_IB_QP_SRC, !!pd->ibpd.uobject); > > > > + destroy_qp_common(dev, mqp, MLX4_IB_QP_SRC, > > > > + rdma_is_user_pd(&pd->ibpd)); > > > > } > > > > > > if qp->uobject > > Same question as above > [...] > > > > @@ -202,7 +202,7 @@ struct ib_srq *mlx4_ib_create_srq(struct ib_pd *pd, > > > > return &srq->ibsrq; > > > > > > > > err_wrid: > > > > - if (pd->uobject) > > > > + if (rdma_is_user_pd(pd)) > > > > mlx4_ib_db_unmap_user(to_mucontext(pd->uobject->context), &srq->db); > > > > else > > > > kvfree(srq->wrid); > > > > > > if srq->uobject > > Same question as above > [...] > > > > @@ -211,13 +211,13 @@ struct ib_srq *mlx4_ib_create_srq(struct ib_pd *pd, > > > > mlx4_mtt_cleanup(dev->dev, &srq->mtt); > > > > > > > > err_buf: > > > > - if (pd->uobject) > > > > + if (rdma_is_user_pd(pd)) > > > > ib_umem_release(srq->umem); > > > > else > > > > mlx4_buf_free(dev->dev, buf_size, &srq->buf); > > > > > > if srq->uobject? srq->umem? I took the 'srq->umem' option... > > > > > > > err_db: > > > > - if (!pd->uobject) > > > > + if (!rdma_is_user_pd(pd)) > > > > mlx4_db_free(dev->dev, &srq->db); > > > > > > if srq->uobject > > Same question as above > [...] > > > > @@ -354,7 +354,7 @@ struct ib_srq *mlx5_ib_create_srq(struct ib_pd *pd, > > > > mlx5_core_destroy_srq(dev->mdev, &srq->msrq); > > > > > > > > err_usr_kern_srq: > > > > - if (pd->uobject) > > > > + if (rdma_is_user_pd(pd)) > > > > destroy_srq_user(pd, srq); > > > > else > > > > destroy_srq_kernel(dev, srq); > > > > > > if srq->uobject > > Same question as above > Jason, pushing udata to wherever it was missing was not so bad as I thought so this part is OK. I only left with question around your request to use 'ib_x->uobject'. If it is OK with you I'd rather to switch those which only tell user/kernel to rdma_is_user_pd as was in the previous revision. Thanks
