On Sun, Oct 07, 2018 at 10:23:08AM +0300, 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 | 6 +++---
> drivers/infiniband/hw/mlx4/qp.c | 17 +++++++++--------
> drivers/infiniband/hw/mlx4/srq.c | 10 +++++-----
> drivers/infiniband/hw/mlx5/qp.c | 8 ++++----
> 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, 63 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index bc2b9e038439..13932f8310ce 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 *)
> @@ -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;
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 081aa91fc162..7a5cc05198f5 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;
> + int is_user = rdma_is_user_pd(&ibqp->pd);
should be bool
> int is_timeout = 0;
> int ret;
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> index 0d7568ee0d64..d17ffcf02194 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> @@ -3963,7 +3963,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));
> if (ret) {
> dev_err(hr_dev->dev, "Destroy qp failed(%d)\n", ret);
> return ret;
> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
> index efb7e961ca65..e157f55d19ce 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> @@ -537,7 +537,8 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> else
> hr_qp->sq_signal_bits = cpu_to_le32(IB_SIGNAL_REQ_WR);
>
> - ret = hns_roce_set_rq_size(hr_dev, &init_attr->cap, !!ib_pd->uobject,
> + ret = hns_roce_set_rq_size(hr_dev, &init_attr->cap,
> + rdma_is_user_pd(ib_pd),
> !!init_attr->srq, hr_qp);
> if (ret) {
> dev_err(dev, "hns_roce_set_rq_size failed\n");
> @@ -574,7 +575,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> init_attr->cap.max_recv_sge];
> }
>
> - if (ib_pd->uobject) {
> + if (rdma_is_user_pd(ib_pd)) {
> if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
> dev_err(dev, "ib_copy_from_udata error for create qp\n");
> ret = -EFAULT;
> @@ -759,7 +760,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> else
> hr_qp->doorbell_qpn = cpu_to_le64(hr_qp->qpn);
>
> - if (ib_pd->uobject && (udata->outlen >= sizeof(resp)) &&
> + if (rdma_is_user_pd(ib_pd) && (udata->outlen >= sizeof(resp)) &&
> (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB)) {
>
> /* indicate kernel supports rq record db */
> @@ -786,7 +787,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> hns_roce_release_range_qp(hr_dev, qpn, 1);
>
> err_wrid:
> - if (ib_pd->uobject) {
> + if (rdma_is_user_pd(ib_pd)) {
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) &&
> (udata->outlen >= sizeof(resp)) &&
> hns_roce_qp_has_rq(init_attr))
> @@ -799,7 +800,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> }
>
> err_sq_dbmap:
> - if (ib_pd->uobject)
> + if (rdma_is_user_pd(ib_pd))
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_SQ_RECORD_DB) &&
> (udata->inlen >= sizeof(ucmd)) &&
> (udata->outlen >= sizeof(resp)) &&
> @@ -812,13 +813,13 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> hns_roce_mtt_cleanup(hr_dev, &hr_qp->mtt);
>
> err_buf:
> - if (ib_pd->uobject)
> + if (rdma_is_user_pd(ib_pd))
> ib_umem_release(hr_qp->umem);
> else
> hns_roce_buf_free(hr_dev, hr_qp->buff_size, &hr_qp->hr_buf);
>
> err_db:
> - if (!ib_pd->uobject && hns_roce_qp_has_rq(init_attr) &&
> + if (!rdma_is_user_pd(ib_pd) && hns_roce_qp_has_rq(init_attr) &&
> (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB))
> hns_roce_free_db(hr_dev, &hr_qp->rdb);
>
> @@ -864,7 +865,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd,
> }
> case IB_QPT_GSI: {
> /* Userspace is not allowed to create special QPs: */
> - if (pd->uobject) {
> + if (rdma_is_user_pd(pd)) {
> dev_err(dev, "not support usr space GSI\n");
> return ERR_PTR(-EINVAL);
> }
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index cb2aef874ca8..8c8f4d0f47f0 100644
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -673,7 +673,7 @@ 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) {
> + if (rdma_is_user_pd(ibpd) && ibpd->uobject->context) {
What is ibpd->uobject->context doing?
AFAIK, if we are in a driver callback then the uobject->context is
guarenteed to exist.
> @@ -768,7 +768,7 @@ static struct ib_qp *i40iw_create_qp(struct ib_pd *ibpd,
> iwdev->qp_table[qp_num] = iwqp;
> i40iw_add_pdusecount(iwqp->iwpd);
> i40iw_add_devusecount(iwdev);
> - if (ibpd->uobject && udata) {
> + if (rdma_is_user_pd(ibpd) && udata) {
Also confusing, why is this testing for user two ways?
> @@ -1201,13 +1201,13 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
> mlx4_mtt_cleanup(dev->dev, &qp->mtt);
>
> err_buf:
> - if (pd->uobject)
> + if (rdma_is_user_pd(pd))
> ib_umem_release(qp->umem);
Seems bonkers, why not !qp->umem or something more direct
> @@ -4050,7 +4051,7 @@ struct ib_wq *mlx4_ib_create_wq(struct ib_pd *pd,
> struct mlx4_ib_create_wq ucmd;
> int err, required_cmd_sz;
>
> - if (!(udata && pd->uobject))
> + if (!(udata && rdma_is_user_pd(pd)))
> return ERR_PTR(-EINVAL);
Again with the udata..
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index 3455b50705cd..ce1881d47db6 100644
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -1742,7 +1742,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 (pd && rdma_is_user_pd(pd)) {
> if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
> mlx5_ib_dbg(dev, "copy failed\n");
> return -EFAULT;
This wrong too, if udata is not NULL then it should be used
> @@ -1803,14 +1803,14 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
>
> qp->has_rq = qp_has_rq(init_attr);
> err = set_rq_size(dev, &init_attr->cap, qp->has_rq,
> - qp, (pd && pd->uobject) ? &ucmd : NULL);
> + qp, (pd && rdma_is_user_pd(pd)) ? &ucmd : NULL);
Same
It would be better to make a patch before this cleaning away the
obviously incorrect uses of rdma_is_user_pd()
I'm not sure it makes any sense to have checks like that in cmds that
receive a udata. If udata is set then this is absolutely a user
command, there is no doubt.
Jason
