On 01-Apr-19 11:47, Leon Romanovsky wrote:
> From: Mark Zhang <markz@xxxxxxxxxxxx>
>
> In auto mode all QPs belong to one category are bind automatically to
> a single counter set. Currently only "qp type" is supported.
>
> In this mode the qp counter is set in RST2INIT modification, and when
> a qp is destroyed the counter is unbound.
This seems like a strange lifetime choice, why set it on RST2INIT and not on
creation? Isn't that very tied to mlx5 implementation?
>
> Signed-off-by: Mark Zhang <markz@xxxxxxxxxxxx>
> Reviewed-by: Majd Dibbiny <majd@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> ---
> drivers/infiniband/core/counters.c | 222 +++++++++++++++++++++++++++++
> drivers/infiniband/core/device.c | 2 +
> drivers/infiniband/core/verbs.c | 9 ++
> include/rdma/ib_verbs.h | 20 +++
> include/rdma/rdma_counter.h | 8 ++
> 5 files changed, 261 insertions(+)
> +
> +static int __rdma_counter_bind_qp(struct rdma_counter *counter,
> + struct ib_qp *qp)
> +{
> + int ret;
> +
> + if (qp->counter)
> + return -EINVAL;
So a certain resource (QP) cannot be bind to more than one counter?
> +
> + if (!qp->device->ops.counter_bind_qp)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&counter->lock);
> + ret = qp->device->ops.counter_bind_qp(counter, qp);
> + mutex_unlock(&counter->lock);
> +
> + return ret;
> +}
> +
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index ab61be545c6c..c52f07f33eb5 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -1791,6 +1791,11 @@ struct ib_qp {
> * Implementation details of the RDMA core, don't use in drivers:
> */
> struct rdma_restrack_entry res;
> +
> + /*
> + * The counter qp bind to
> + */
A single line comment is enough here, while at it I would change to "The counter
the qp is bind to".
> + struct rdma_counter *counter;
> };
>
> struct ib_dm {
> @@ -2556,6 +2561,21 @@ struct ib_device_ops {
> */
> void (*dealloc_driver)(struct ib_device *dev);
>
> + /**
> + * counter_bind_qp - Bind a QP to a counter.
> + * @counter - The counter to be bound. If counter->id is zero then
> + * the driver needs to allocate a new counter and set counter->id
> + */
> + int (*counter_bind_qp)(struct rdma_counter *counter, struct ib_qp *qp);
> + /**
> + * counter_unbind_qp - Unbind the qp from the dynamically-allocated
> + * counter and bind it onto the default one. If this is the last
> + * bound qp, then this counter will be deallocated.
> + * @force - If it is true then free the counter in case of any error.
> + * used in cases like qp destroy.
> + */
> + int (*counter_unbind_qp)(struct ib_qp *qp, bool force);
> +
Are we going to add bind/unbind callbacks for every resource type? Maybe have
one bind/unbind callback with a resource type parameter?
> DECLARE_RDMA_OBJ_SIZE(ib_ah);
> DECLARE_RDMA_OBJ_SIZE(ib_pd);
> DECLARE_RDMA_OBJ_SIZE(ib_srq);
