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.
> > >
> > > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > > drivers/infiniband/hw/bnxt_re/ib_verbs.c | 4 ++--
> > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
> > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 3 ++-
> > > drivers/infiniband/hw/hns/hns_roce_qp.c | 17 +++++++++--------
> > > drivers/infiniband/hw/i40iw/i40iw_verbs.c | 2 +-
> > > drivers/infiniband/hw/mlx4/qp.c | 11 ++++++-----
> > > drivers/infiniband/hw/mlx4/srq.c | 10 +++++-----
> > > drivers/infiniband/hw/mlx5/qp.c | 4 ++--
> > > drivers/infiniband/hw/mlx5/srq.c | 8 ++++----
> > > drivers/infiniband/hw/mthca/mthca_provider.c | 10 +++++-----
> > > drivers/infiniband/hw/mthca/mthca_qp.c | 7 ++++---
> > > drivers/infiniband/hw/mthca/mthca_srq.c | 8 ++++----
> > > drivers/infiniband/hw/nes/nes_verbs.c | 7 ++++---
> > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +-
> > > drivers/infiniband/hw/qedr/verbs.c | 4 ++--
> > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c | 2 +-
> > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c | 2 +-
> > > drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
> > > 18 files changed, 56 insertions(+), 50 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > index 54fdd4c..d2d2630 100644
> > > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > @@ -698,7 +698,7 @@ struct ib_ah *bnxt_re_create_ah(struct ib_pd *ib_pd,
> > > ah->qplib_ah.flow_label = grh->flow_label;
> > > ah->qplib_ah.hop_limit = grh->hop_limit;
> > > ah->qplib_ah.sl = rdma_ah_get_sl(ah_attr);
> > > - if (ib_pd->uobject &&
> > > + if (rdma_is_user_pd(ib_pd) &&
> > > !rdma_is_multicast_addr((struct in6_addr *)
> > > grh->dgid.raw) &&
> > > !rdma_link_local_addr((struct in6_addr *)
> >
> > I have no idea why this if is here, but looks like it should be 'if
> > (udata)' ?
> >
> > > @@ -729,7 +729,7 @@ struct ib_ah *bnxt_re_create_ah(struct ib_pd *ib_pd,
> > > }
> > >
> > > /* Write AVID to shared page. */
> > > - if (ib_pd->uobject) {
> > > + if (rdma_is_user_pd(ib_pd)) {
> > > struct ib_ucontext *ib_uctx = ib_pd->uobject->context;
> > > struct bnxt_re_ucontext *uctx;
> > > unsigned long flag;
> >
> > This should be 'if (udata) {.. ib_utx = get_uctx_from_udata(..);..'
> >
> > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > index 00170aa..a6c581a 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;
> > > - 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?
The PD should not be used to infer the user-ness of the QP, that is a
mistake in this driver.
If we want to drop the qp->uobjec then we need a rdma_is_user_qp() and
that would replace this..
> > > @@ -1552,7 +1552,7 @@ int qedr_destroy_srq(struct ib_srq *ibsrq)
> > > in_params.srq_id = srq->srq_id;
> > > dev->ops->rdma_destroy_srq(dev->rdma_ctx, &in_params);
> > >
> > > - if (ibsrq->pd->uobject)
> > > + if (rdma_is_user_pd(ibsrq->pd))
> > > qedr_free_srq_user_params(srq);
> > > else
> > > qedr_free_srq_kernel_params(srq);
> >
> > if ibsrq->uobject
>
> Same question as above. Do you assume that srq will never be shared?
> Any good reason to introduce new check on ib_x uobject rather then just
> use rdma_is_user_pd here?
same reason - the pd userness should not be used to imply the userness
of objects below it...
Jason
