On Thu, Jan 17, 2019 at 09:11:12PM +0200, Shamir Rabinovitch wrote:
> Now when we have the udata passed to all the ib_xxx object creation APIs
> and the additional function 'rdma_get_ucontext' to get the ib_ucontext
> from ib_udata stored in uverbs_attr_bundle, we can finally start to remove
> the dependency of the drivers in the ib_xxx->uobject->context.
>
> Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 33 +++++--
> drivers/infiniband/hw/cxgb3/iwch_provider.c | 4 +-
> drivers/infiniband/hw/cxgb4/qp.c | 8 +-
> drivers/infiniband/hw/hns/hns_roce_qp.c | 15 +++-
> drivers/infiniband/hw/i40iw/i40iw_verbs.c | 15 +++-
> drivers/infiniband/hw/mlx4/doorbell.c | 6 ++
> drivers/infiniband/hw/mlx4/mr.c | 9 +-
> drivers/infiniband/hw/mlx4/qp.c | 92 +++++++++++++-------
> drivers/infiniband/hw/mlx4/srq.c | 13 ++-
> drivers/infiniband/hw/mlx5/qp.c | 68 +++++++++++----
> drivers/infiniband/hw/mlx5/srq.c | 13 ++-
> drivers/infiniband/hw/mthca/mthca_provider.c | 28 ++++--
> drivers/infiniband/hw/mthca/mthca_qp.c | 19 ++--
> drivers/infiniband/hw/mthca/mthca_srq.c | 25 ++++--
> drivers/infiniband/hw/nes/nes_verbs.c | 13 ++-
> drivers/infiniband/hw/qedr/verbs.c | 8 +-
> drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 7 +-
> drivers/infiniband/sw/rdmavt/qp.c | 10 ++-
> drivers/infiniband/sw/rdmavt/srq.c | 10 ++-
> drivers/infiniband/sw/rxe/rxe_qp.c | 5 +-
> drivers/infiniband/sw/rxe/rxe_verbs.c | 5 +-
> 21 files changed, 287 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index 9bc637e49faa..d811efe49e77 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -733,11 +733,16 @@ struct ib_ah *bnxt_re_create_ah(struct ib_pd *ib_pd,
>
> /* Write AVID to shared page. */
> if (udata) {
> - struct ib_ucontext *ib_uctx = ib_pd->uobject->context;
> + struct ib_ucontext *ib_uctx;
> struct bnxt_re_ucontext *uctx;
> unsigned long flag;
> u32 *wrptr;
>
> + ib_uctx = rdma_get_ucontext(udata);
> + if (IS_ERR(ib_uctx)) {
> + rc = PTR_ERR(ib_uctx);
> + goto fail;
> + }
> uctx = container_of(ib_uctx, struct bnxt_re_ucontext, ib_uctx);
> spin_lock_irqsave(&uctx->sh_lock, flag);
> wrptr = (u32 *)(uctx->shpg + BNXT_RE_AVID_OFFT);
> @@ -883,10 +888,15 @@ static int bnxt_re_init_user_qp(struct bnxt_re_dev *rdev, struct bnxt_re_pd *pd,
> struct bnxt_qplib_qp *qplib_qp = &qp->qplib_qp;
> struct ib_umem *umem;
> int bytes = 0;
> - struct ib_ucontext *context = pd->ib_pd.uobject->context;
> - struct bnxt_re_ucontext *cntx = container_of(context,
> - struct bnxt_re_ucontext,
> - ib_uctx);
> + struct bnxt_re_ucontext *cntx;
> + struct ib_ucontext *context;
> +
> + context = rdma_get_ucontext(udata);
> + if (IS_ERR(context))
> + return PTR_ERR(context);
> +
> + cntx = container_of(context, struct bnxt_re_ucontext, ib_uctx);
> +
> if (ib_copy_from_udata(&ureq, udata, sizeof(ureq)))
> return -EFAULT;
>
> @@ -1360,10 +1370,15 @@ static int bnxt_re_init_user_srq(struct bnxt_re_dev *rdev,
> struct bnxt_qplib_srq *qplib_srq = &srq->qplib_srq;
> struct ib_umem *umem;
> int bytes = 0;
> - struct ib_ucontext *context = pd->ib_pd.uobject->context;
> - struct bnxt_re_ucontext *cntx = container_of(context,
> - struct bnxt_re_ucontext,
> - ib_uctx);
> + struct bnxt_re_ucontext *cntx;
> + struct ib_ucontext *context;
> +
> + context = rdma_get_ucontext(udata);
> + if (IS_ERR(context))
> + return PTR_ERR(context);
> +
> + cntx = container_of(context, struct bnxt_re_ucontext, ib_uctx);
> +
> if (ib_copy_from_udata(&ureq, udata, sizeof(ureq)))
> return -EFAULT;
>
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> index 07c20cd07f33..b692cd2bbe92 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> @@ -796,6 +796,7 @@ static struct ib_qp *iwch_create_qp(struct ib_pd *pd,
> struct iwch_cq *schp;
> struct iwch_cq *rchp;
> struct iwch_create_qp_resp uresp;
> + struct ib_ucontext *ib_ucontext;
> int wqsize, sqsize, rqsize;
> struct iwch_ucontext *ucontext;
>
> @@ -836,7 +837,8 @@ static struct ib_qp *iwch_create_qp(struct ib_pd *pd,
> * Kernel users need more wq space for fastreg WRs which can take
> * 2 WR fragments.
> */
> - ucontext = udata ? to_iwch_ucontext(pd->uobject->context) : NULL;
> + ib_ucontext = rdma_get_ucontext(udata);
> + ucontext = IS_ERR(ib_ucontext) ? NULL : to_iwch_ucontext(ib_ucontext);
> if (!ucontext && wqsize < (rqsize + (2 * sqsize)))
> wqsize = roundup_pow_of_two(rqsize +
> roundup_pow_of_two(attrs->cap.max_send_wr * 2));
> diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
> index 03f4c66c2659..d2c452b4282c 100644
> --- a/drivers/infiniband/hw/cxgb4/qp.c
> +++ b/drivers/infiniband/hw/cxgb4/qp.c
> @@ -2137,6 +2137,7 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
> struct c4iw_create_qp_resp uresp;
> unsigned int sqsize, rqsize = 0;
> struct c4iw_ucontext *ucontext;
> + struct ib_ucontext *ib_ucontext;
> int ret;
> struct c4iw_mm_entry *sq_key_mm, *rq_key_mm = NULL, *sq_db_key_mm;
> struct c4iw_mm_entry *rq_db_key_mm = NULL, *ma_sync_key_mm = NULL;
> @@ -2170,7 +2171,8 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
> if (sqsize < 8)
> sqsize = 8;
>
> - ucontext = udata ? to_c4iw_ucontext(pd->uobject->context) : NULL;
> + ib_ucontext = rdma_get_ucontext(udata);
> + ucontext = IS_ERR(ib_ucontext) ? NULL : to_c4iw_ucontext(ib_ucontext);
>
> qhp = kzalloc(sizeof(*qhp), GFP_KERNEL);
> if (!qhp)
> @@ -2697,6 +2699,7 @@ struct ib_srq *c4iw_create_srq(struct ib_pd *pd, struct ib_srq_init_attr *attrs,
> struct c4iw_create_srq_resp uresp;
> struct c4iw_ucontext *ucontext;
> struct c4iw_mm_entry *srq_key_mm, *srq_db_key_mm;
> + struct ib_ucontext *ib_ucontext;
> int rqsize;
> int ret;
> int wr_len;
> @@ -2719,7 +2722,8 @@ struct ib_srq *c4iw_create_srq(struct ib_pd *pd, struct ib_srq_init_attr *attrs,
> rqsize = attrs->attr.max_wr + 1;
> rqsize = roundup_pow_of_two(max_t(u16, rqsize, 16));
>
> - ucontext = udata ? to_c4iw_ucontext(pd->uobject->context) : NULL;
> + ib_ucontext = rdma_get_ucontext(udata);
> + ucontext = IS_ERR(ib_ucontext) ? NULL : to_c4iw_ucontext(ib_ucontext);
>
> srq = kzalloc(sizeof(*srq), GFP_KERNEL);
> if (!srq)
> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
> index accf9ce1507d..b9c25eaf2d75 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_qp.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> @@ -542,12 +542,19 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> struct device *dev = hr_dev->dev;
> struct hns_roce_ib_create_qp ucmd;
> struct hns_roce_ib_create_qp_resp resp = {};
> + struct ib_ucontext *ucontext;
> unsigned long qpn = 0;
> int ret = 0;
> u32 page_shift;
> u32 npages;
> int i;
>
> + ucontext = rdma_get_ucontext(udata);
> + if (IS_ERR(ucontext)) {
> + ret = PTR_ERR(ucontext);
> + goto err_out;
> + }
> +
> mutex_init(&hr_qp->mutex);
> spin_lock_init(&hr_qp->sq.lock);
> spin_lock_init(&hr_qp->rq.lock);
> @@ -653,7 +660,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> (udata->outlen >= sizeof(resp)) &&
> hns_roce_qp_has_sq(init_attr)) {
> ret = hns_roce_db_map_user(
> - to_hr_ucontext(ib_pd->uobject->context), udata,
> + to_hr_ucontext(ucontext), udata,
> ucmd.sdb_addr, &hr_qp->sdb);
> if (ret) {
> dev_err(dev, "sq record doorbell map failed!\n");
> @@ -669,7 +676,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> (udata->outlen >= sizeof(resp)) &&
> hns_roce_qp_has_rq(init_attr)) {
> ret = hns_roce_db_map_user(
> - to_hr_ucontext(ib_pd->uobject->context), udata,
> + to_hr_ucontext(ucontext), udata,
> ucmd.db_addr, &hr_qp->rdb);
> if (ret) {
> dev_err(dev, "rq record doorbell map failed!\n");
> @@ -815,7 +822,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> (udata->outlen >= sizeof(resp)) &&
> hns_roce_qp_has_rq(init_attr))
> hns_roce_db_unmap_user(
> - to_hr_ucontext(ib_pd->uobject->context),
> + to_hr_ucontext(ucontext),
> &hr_qp->rdb);
> } else {
> kfree(hr_qp->sq.wrid);
> @@ -829,7 +836,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
> (udata->outlen >= sizeof(resp)) &&
> hns_roce_qp_has_sq(init_attr))
> hns_roce_db_unmap_user(
> - to_hr_ucontext(ib_pd->uobject->context),
> + to_hr_ucontext(ucontext),
> &hr_qp->sdb);
>
> err_mtt:
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index 12b31a8440be..194cd911c9de 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -580,11 +580,16 @@ static struct ib_qp *i40iw_create_qp(struct ib_pd *ibpd,
> struct i40iw_create_qp_info *qp_info;
> struct i40iw_cqp_request *cqp_request;
> struct cqp_commands_info *cqp_info;
> + struct ib_ucontext *ib_ucontext;
>
> struct i40iw_qp_host_ctx_info *ctx_info;
> struct i40iwarp_offload_info *iwarp_info;
> unsigned long flags;
>
> + ib_ucontext = rdma_get_ucontext(udata);
> + if (udata && IS_ERR(ib_ucontext))
> + return ERR_CAST(ib_ucontext);
> +
> if (iwdev->closing)
> return ERR_PTR(-ENODEV);
>
> @@ -674,7 +679,7 @@ static struct ib_qp *i40iw_create_qp(struct ib_pd *ibpd,
> }
> iwqp->ctx_info.qp_compl_ctx = req.user_compl_ctx;
> iwqp->user_mode = 1;
> - ucontext = to_ucontext(ibpd->uobject->context);
> + ucontext = to_ucontext(ib_ucontext);
>
> if (req.user_wqe_buffers) {
> struct i40iw_pbl *iwpbl;
> @@ -1832,6 +1837,7 @@ static struct ib_mr *i40iw_reg_user_mr(struct ib_pd *pd,
> struct i40iw_pd *iwpd = to_iwpd(pd);
> struct i40iw_device *iwdev = to_iwdev(pd->device);
> struct i40iw_ucontext *ucontext;
> + struct ib_ucontext *ib_ucontext;
> struct i40iw_pble_alloc *palloc;
> struct i40iw_pbl *iwpbl;
> struct i40iw_mr *iwmr;
> @@ -1847,6 +1853,12 @@ static struct ib_mr *i40iw_reg_user_mr(struct ib_pd *pd,
> int ret;
> int pg_shift;
>
> + ib_ucontext = rdma_get_ucontext(udata);
> + if (IS_ERR(ib_ucontext))
> + return ERR_CAST(ib_ucontext);
> +
> + ucontext = to_ucontext(ib_ucontext);
> +
> if (iwdev->closing)
> return ERR_PTR(-ENODEV);
>
> @@ -1872,7 +1884,6 @@ static struct ib_mr *i40iw_reg_user_mr(struct ib_pd *pd,
> iwmr->region = region;
> iwmr->ibmr.pd = pd;
> iwmr->ibmr.device = pd->device;
> - ucontext = to_ucontext(pd->uobject->context);
>
> iwmr->page_size = PAGE_SIZE;
> iwmr->page_msk = PAGE_MASK;
> diff --git a/drivers/infiniband/hw/mlx4/doorbell.c b/drivers/infiniband/hw/mlx4/doorbell.c
> index 3aab71b29ce8..1a4c6d3f8078 100644
> --- a/drivers/infiniband/hw/mlx4/doorbell.c
> +++ b/drivers/infiniband/hw/mlx4/doorbell.c
> @@ -48,6 +48,9 @@ int mlx4_ib_db_map_user(struct mlx4_ib_ucontext *context,
> struct mlx4_ib_user_db_page *page;
> int err = 0;
>
> + if (!context)
> + return -EINVAL;
> +
> mutex_lock(&context->db_page_mutex);
>
> list_for_each_entry(page, &context->db_page_list, list)
> @@ -84,6 +87,9 @@ int mlx4_ib_db_map_user(struct mlx4_ib_ucontext *context,
>
> void mlx4_ib_db_unmap_user(struct mlx4_ib_ucontext *context, struct mlx4_db *db)
> {
> + if (WARN_ON(!context))
> + return;
I wonder, how did you come to need of those checks?
Aren't our flows protected now and we are safe to have context != NULL?
Thanks
Attachment:
signature.asc
Description: PGP signature
