On Mon, Apr 01, 2019 at 05:19:30PM +0300, Gal Pressman wrote:
> 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.
Other drivers cal allocate counter whenever they want, in RST2INIT they
actually connected.
>
> >>> + 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.
We are talking about it internally, it will take time till we implement. :)
Thanks
>
> >
> >>
> >>> DECLARE_RDMA_OBJ_SIZE(ib_ah);
> >>> DECLARE_RDMA_OBJ_SIZE(ib_pd);
> >>> DECLARE_RDMA_OBJ_SIZE(ib_srq);
Attachment:
signature.asc
Description: PGP signature
