On Wed, Jan 30, 2019 at 12:49:06PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> Avoid direct access to internal restrack values by providing
> special wrappers and update previous occurrence for
> rdma_res_to_id(), which translated from restrack entry to CM_ID
> and not to restrack ID.
>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/cma.c | 8 +++----
> drivers/infiniband/core/core_priv.h | 2 +-
> drivers/infiniband/core/uverbs_cmd.c | 8 +++----
> drivers/infiniband/core/uverbs_std_types_cq.c | 2 +-
> drivers/infiniband/core/verbs.c | 6 ++---
> drivers/infiniband/hw/cxgb4/restrack.c | 2 +-
> include/rdma/rdma_cm.h | 2 +-
> include/rdma/restrack.h | 22 +++++++++++++++++++
> 8 files changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 83aa2ad0c27e..9570b98fdaeb 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -162,17 +162,17 @@ struct iw_cm_id *rdma_iw_cm_id(struct rdma_cm_id *id)
> EXPORT_SYMBOL(rdma_iw_cm_id);
>
> /**
> - * rdma_res_to_id() - return the rdma_cm_id pointer for this restrack.
> + * rdma_res_to_cmid() - return the rdma_cm_id pointer for this restrack.
> * @res: rdma resource tracking entry pointer
> */
> -struct rdma_cm_id *rdma_res_to_id(struct rdma_restrack_entry *res)
> +struct rdma_cm_id *rdma_res_to_cmid(struct rdma_restrack_entry *res)
> {
> struct rdma_id_private *id_priv =
> container_of(res, struct rdma_id_private, res);
>
> return &id_priv->id;
> }
> -EXPORT_SYMBOL(rdma_res_to_id);
> +EXPORT_SYMBOL(rdma_res_to_cmid);
>
> static void cma_add_one(struct ib_device *device);
> static void cma_remove_one(struct ib_device *device, void *client_data);
> @@ -881,7 +881,7 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
> return ERR_PTR(-ENOMEM);
>
> rdma_restrack_set_task(&id_priv->res, caller);
> - id_priv->res.type = RDMA_RESTRACK_CM_ID;
> + rdma_rt_set_type(&id_priv->res, RDMA_RESTRACK_CM_ID);
> id_priv->state = RDMA_CM_IDLE;
> id_priv->id.context = context;
> id_priv->id.event_handler = event_handler;
> diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
> index 42a49982f66e..cda2db1add39 100644
> +++ b/drivers/infiniband/core/core_priv.h
> @@ -298,8 +298,8 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
> * and more importantly they are created internaly by driver,
> * see mlx5 create_dev_resources() as an example.
> */
> + rdma_rt_set_type(&qp->res, RDMA_RESTRACK_QP);
> if (attr->qp_type < IB_QPT_XRC_INI) {
> - qp->res.type = RDMA_RESTRACK_QP;
> if (uobj)
> rdma_restrack_uadd(&qp->res);
> else
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index aa260cafbd85..4ac97a61d567 100644
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -262,7 +262,7 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
>
> fd_install(resp.async_fd, filp);
>
> - ucontext->res.type = RDMA_RESTRACK_CTX;
> + rdma_rt_set_type(&ucontext->res, RDMA_RESTRACK_CTX);
> rdma_restrack_uadd(&ucontext->res);
>
> /*
> @@ -421,7 +421,7 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
> uobj->object = pd;
> memset(&resp, 0, sizeof resp);
> resp.pd_handle = uobj->id;
> - pd->res.type = RDMA_RESTRACK_PD;
> + rdma_rt_set_type(&pd->res, RDMA_RESTRACK_PD);
> rdma_restrack_uadd(&pd->res);
>
> ret = uverbs_response(attrs, &resp, sizeof(resp));
> @@ -737,7 +737,7 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> mr->dm = NULL;
> mr->uobject = uobj;
> atomic_inc(&pd->usecnt);
> - mr->res.type = RDMA_RESTRACK_MR;
> + rdma_rt_set_type(&mr->res, RDMA_RESTRACK_MR);
> rdma_restrack_uadd(&mr->res);
>
> uobj->object = mr;
> @@ -1015,7 +1015,7 @@ static struct ib_ucq_object *create_cq(struct uverbs_attr_bundle *attrs,
> resp.base.cqe = cq->cqe;
> resp.response_length = uverbs_response_length(attrs, sizeof(resp));
>
> - cq->res.type = RDMA_RESTRACK_CQ;
> + rdma_rt_set_type(&cq->res, RDMA_RESTRACK_CQ);
> rdma_restrack_uadd(&cq->res);
>
> ret = uverbs_response(attrs, &resp, sizeof(resp));
> diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
> index a59ea89e3f2b..6ca5d5016423 100644
> +++ b/drivers/infiniband/core/uverbs_std_types_cq.c
> @@ -125,7 +125,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
> obj->uobject.object = cq;
> obj->uobject.user_handle = user_handle;
> atomic_set(&cq->usecnt, 0);
> - cq->res.type = RDMA_RESTRACK_CQ;
> + rdma_rt_set_type(&cq->res, RDMA_RESTRACK_CQ);
> rdma_restrack_uadd(&cq->res);
>
> ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe,
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 3220fb42ecce..814cbc814cf4 100644
> +++ b/drivers/infiniband/core/verbs.c
> @@ -275,7 +275,7 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
> mr_access_flags |= IB_ACCESS_REMOTE_READ | IB_ACCESS_REMOTE_WRITE;
> }
>
> - pd->res.type = RDMA_RESTRACK_PD;
> + rdma_rt_set_type(&pd->res, RDMA_RESTRACK_PD);
> rdma_restrack_set_task(&pd->res, caller);
> rdma_restrack_kadd(&pd->res);
>
> @@ -1914,7 +1914,7 @@ struct ib_cq *__ib_create_cq(struct ib_device *device,
> cq->event_handler = event_handler;
> cq->cq_context = cq_context;
> atomic_set(&cq->usecnt, 0);
> - cq->res.type = RDMA_RESTRACK_CQ;
> + rdma_rt_set_type(&cq->res, RDMA_RESTRACK_CQ);
> rdma_restrack_set_task(&cq->res, caller);
> rdma_restrack_kadd(&cq->res);
> }
> @@ -1997,7 +1997,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd,
> mr->uobject = NULL;
> atomic_inc(&pd->usecnt);
> mr->need_inval = false;
> - mr->res.type = RDMA_RESTRACK_MR;
> + rdma_rt_set_type(&mr->res, RDMA_RESTRACK_MR);
> rdma_restrack_kadd(&mr->res);
> }
>
> diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
> index 9a7520ee41e0..7224742f8b9a 100644
> +++ b/drivers/infiniband/hw/cxgb4/restrack.c
> @@ -198,7 +198,7 @@ union union_ep {
> static int fill_res_ep_entry(struct sk_buff *msg,
> struct rdma_restrack_entry *res)
> {
> - struct rdma_cm_id *cm_id = rdma_res_to_id(res);
> + struct rdma_cm_id *cm_id = rdma_res_to_cmid(res);
> struct nlattr *table_attr;
> struct c4iw_ep_common *epcp;
> struct c4iw_listen_ep *listen_ep = NULL;
> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index 71f48cfdc24c..c1615e71ac0c 100644
> +++ b/include/rdma/rdma_cm.h
> @@ -427,6 +427,6 @@ void rdma_read_gids(struct rdma_cm_id *cm_id, union ib_gid *sgid,
> union ib_gid *dgid);
>
> struct iw_cm_id *rdma_iw_cm_id(struct rdma_cm_id *cm_id);
> -struct rdma_cm_id *rdma_res_to_id(struct rdma_restrack_entry *res);
> +struct rdma_cm_id *rdma_res_to_cmid(struct rdma_restrack_entry *res);
>
> #endif /* RDMA_CM_H */
> diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> index 92d11c35221a..c81e43d043cf 100644
> +++ b/include/rdma/restrack.h
> @@ -161,4 +161,26 @@ struct xarray *rdma_dev_to_xa(struct ib_device *dev,
> enum rdma_restrack_type type);
> void rdma_rt_read_lock(struct ib_device *dev, enum rdma_restrack_type type);
> void rdma_rt_read_unlock(struct ib_device *dev, enum rdma_restrack_type type);
> +
> +/**
> + * rdma_res_to_id() - Unique ID as seen by restrack
> + * @res: resrouce to operate
> + *
> + * Return: unique ID
> + */
> +static inline u32 rdma_res_to_id(struct rdma_restrack_entry *res)
> +{
> + return res->id;
> +}
> +/**
> + * rdma_rt_set_type() - Set restrack entry type,
> + * prior to call rdma_restrack_add()
> + * @res: resrouce to operate
> + * @type: Actual type
> + */
> +static inline void rdma_rt_set_type(struct rdma_restrack_entry *res,
> + enum rdma_restrack_type type)
> +{
> + res->type = type;
> +}
This seems a bit obfuscating, generally kernel code tries to avoid
this sort of trivial accessor style, I thought
If there is some particular reason to do this it should be mentioned
in the comment, and possibly this patch should be in a series with the
patch that needs it.
Jason
