Re: [PATCH V3 for-next 3/7] IB/core: Add idr based standard types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 5, 2017 at 8:05 PM, Hefty, Sean <sean.hefty@xxxxxxxxx> wrote:
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_cq = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_ucq_object),
>> 0),
>> +     .destroy_object = uverbs_free_cq,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_qp = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_uqp_object),
>> 0),
>> +     .destroy_object = uverbs_free_qp,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_mw = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR(0),
>> +     .destroy_object = uverbs_free_mw,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_mr = {
>> +     /* 1 is used in order to free the MR after all the MWs */
>> +     .type = UVERBS_TYPE_ALLOC_IDR(1),
>> +     .destroy_object = uverbs_free_mr,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_srq = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_usrq_object),
>> 0),
>> +     .destroy_object = uverbs_free_srq,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_ah = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR(0),
>> +     .destroy_object = uverbs_free_ah,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_flow = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR(0),
>> +     .destroy_object = uverbs_free_flow,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_wq = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_uwq_object),
>> 0),
>> +     .destroy_object = uverbs_free_wq,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_rwq_ind_table = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR(0),
>> +     .destroy_object = uverbs_free_rwq_ind_tbl,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_xrcd = {
>> +     .type = UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_uxrcd_object),
>> 0),
>> +     .destroy_object = uverbs_free_xrcd,
>> +};
>> +
>> +const struct uverbs_obj_idr_type uverbs_type_attrs_pd = {
>> +     /* 2 is used in order to free the PD after MRs */
>> +     .type = UVERBS_TYPE_ALLOC_IDR(2),
>> +     .destroy_object = uverbs_free_pd,
>> +};
>
> I wonder if it wouldn't make more sense to have destroy order be independent from object creation assumptions.  For example, QPs must be destroyed prior to their associated CQs.  This code works, since the CQ is passed in during QP creation and ends up on the list correctly, but I wonder if all hardware would actually need that restriction.  Basically, the destroy order value as used is not actually capturing the true destroy order; there are other assumptions baked into the code.
>

Actually, this code gives you the freedom to do both. You could have
every type in its own "order group" or you could share these groups.
It's guaranteed that all objects in "order group" x will be destroyed
after
objects in "order group" x+1.
The rational here is to destroy objects in a LIFO order in every order
group. That should fit most objects pretty well. It's obvious that if
a QP reports to a CQ, you can't destroy this CQ before destroying the
QP.
This is the case now, when creating object x which depends on object
y, we increase the ref count of object y. Object y will only be
destroyed when its reference count is zero.
So, effectively, we capture the same logic here.
The only exception is objects which their dependency is either
set/breaks in the user-space/hardware (without kernel awareness). In
these cases we need to fall back to destroy objects by their type
class order (a
good example could be MWs and MRs).

> Otherwise:
>
> Reviewed-by: Sean Hefty <sean.hefty@xxxxxxxxx>

Thanks :)

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux