On 01-Apr-19 16:35, Leon Romanovsky wrote:
> On Mon, Apr 01, 2019 at 03:07:09PM +0300, Gal Pressman wrote:
>> 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?
>
> It is our attempt to delay depletion of HW objects (counters). They are
> limited in number and change RST2INIT means that this QP was enabled.
We can leave it to the individual driver to decide when to do the actual hw
allocation, it's better than the core forcing all drivers to do it on QP
modification.
>>> + 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?
>
> It is internal to kernel and to our subsystem, so I preferred to see how it
> is evolving before asking internally for implementing very general thing.
> I can definitely be wrong about it.
Makes sense to keep it simple for now, but if you're already working on MRs it
might be worth to do both in the same callback.
>
>>
>>> DECLARE_RDMA_OBJ_SIZE(ib_ah);
>>> DECLARE_RDMA_OBJ_SIZE(ib_pd);
>>> DECLARE_RDMA_OBJ_SIZE(ib_srq);
