Re: [PATCH for-next 6/7] IB/core: Declare all common IB types

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Jan 11, 2017 at 12:53:52PM +0200, Matan Barak wrote:

> diff --git a/drivers/infiniband/core/uverbs_ioctl_cmd.c b/drivers/infiniband/core/uverbs_ioctl_cmd.c
> new file mode 100644

This is not really an appropriate file name for this point in the
series...

> +DECLARE_UVERBS_TYPE(uverbs_type_comp_channel,
> +		    &UVERBS_TYPE_ALLOC_FD(0, sizeof(struct ib_uobject) + sizeof(struct ib_uverbs_event_file),
> +					  uverbs_free_event_file,
> +					  &uverbs_event_fops,
> +					  "[infinibandevent]",
> O_RDONLY));

Really hate these macros.

const struct ib_uobject_type uverbs_type_comp_channel = {
       .size = sizeof(struct ib_uverbs_event_file,
       .destroy = uverbs_destroy_event_file, // destroy = release device resources!!
       .fd = {
          .fops = &uverbs_event_fops,
	  .name = "[infinibandevent]"
       },
};

If you can't do it without macros something else is wrong.

> +DECLARE_UVERBS_TYPES(uverbs_common_types,
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_PD, uverbs_type_pd),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_MR, uverbs_type_mr),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_COMP_CHANNEL, uverbs_type_comp_channel),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_CQ, uverbs_type_cq),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_QP, uverbs_type_qp),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_AH, uverbs_type_ah),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_MW, uverbs_type_mw),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_SRQ, uverbs_type_srq),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_FLOW, uverbs_type_flow),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_WQ, uverbs_type_wq),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_RWQ_IND_TBL,
> +				     uverbs_type_rwq_ind_table),
> +		     ADD_UVERBS_TYPE(UVERBS_TYPE_XRCD, uverbs_type_xrcd),
> +);
> +EXPORT_SYMBOL(uverbs_common_types);

No purpose at this point in the series.

I didn't look carefully at patch 7 due to to convoluted it is, split
it please..

Everything else looks broadly reasonable to me and is a good overall
improvement I think.

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