On 6/12/2019 1:54 AM, Jason Gunthorpe wrote:
> On Thu, Jun 06, 2019 at 01:53:37PM +0300, Leon Romanovsky wrote:
>> From: Mark Zhang <markz@xxxxxxxxxxxx>
>>
>> Add support for ib callbacks counter_bind_qp() and counter_unbind_qp().
>>
>> Signed-off-by: Mark Zhang <markz@xxxxxxxxxxxx>
>> Reviewed-by: Majd Dibbiny <majd@xxxxxxxxxxxx>
>> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>> drivers/infiniband/hw/mlx5/main.c | 53 +++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
>> index 8b7bcf8f68fb..66c94a060718 100644
>> +++ b/drivers/infiniband/hw/mlx5/main.c
>> @@ -5533,6 +5533,57 @@ static int mlx5_ib_get_hw_stats(struct ib_device *ibdev,
>> return num_counters;
>> }
>>
>> +static int mlx5_ib_counter_bind_qp(struct rdma_counter *counter,
>> + struct ib_qp *qp)
>> +{
>> + struct mlx5_ib_dev *dev = to_mdev(qp->device);
>> + u16 cnt_set_id = 0;
>> + int err;
>> +
>> + if (!counter->id) {
>> + err = mlx5_cmd_alloc_q_counter(dev->mdev,
>> + &cnt_set_id,
>> + MLX5_SHARED_RESOURCE_UID);
>> + if (err)
>> + return err;
>> + counter->id = cnt_set_id;
>> + }
>> +
>> + err = mlx5_ib_qp_set_counter(qp, counter);
>> + if (err)
>> + goto fail_set_counter;
>> +
>> + return 0;
>> +
>> +fail_set_counter:
>> + mlx5_core_dealloc_q_counter(dev->mdev, cnt_set_id);
>> + counter->id = 0;
>> +
>> + return err;
>> +}
>> +
>> +static int mlx5_ib_counter_unbind_qp(struct ib_qp *qp, bool force)
>> +{
>> + struct mlx5_ib_dev *dev = to_mdev(qp->device);
>> + struct rdma_counter *counter = qp->counter;
>> + int err;
>> +
>> + err = mlx5_ib_qp_set_counter(qp, NULL);
>> + if (err && !force)
>> + return err;
>> +
>> + /*
>> + * Deallocate the counter if this is the last QP bound on it;
>> + * If @force is set then we still deallocate the q counter
>> + * no matter if there's any error in previous. used for cases
>> + * like qp destroy.
>> + */
>> + if (atomic_read(&counter->usecnt) == 1)
>> + return mlx5_core_dealloc_q_counter(dev->mdev, counter->id);
>
> This looks like a nonsense thing to write, what it is trying to do
> with that atomic?
>
> I still can't see why this isn't a normal kref.
>
Hi Jason,
Have discussed with Leon, unlike other resources, counter alloc/dealloc
isn't called explicitly. So we need a refcount to record how many QPs
are bound on this counter, when it comes to 0 then the counter can be
deallocated. Whether to use atomic or kref the code is similar, it is
not able to take advantage of kref/completion.
> Jason
>
