Re: [PATCH rdma-next v1 1/6] IB/uverbs: Allow CQ moderation with modify CQ

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

On Mon, Oct 30, 2017 at 09:52:36AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 30, 2017 at 05:28:15PM +0200, Leon Romanovsky wrote:
> > > + * ib_modify_cq - Modifies moderation params of the CQ
> > > + * @cq: The CQ to modify.
> > > + * @cq_count: number of CQEs that will trigger an event
> > > + * @cq_period: max period of time in usec before triggering an event
> > > + *
> > > + */
> > > +int ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period);
> >
> > I see it differently, this is extendable version of modify_cq, which is
> > going to benefit all other users who will decide to extend it.
>
> The only reason we have these goofy 'thick' APIs like modify_qp is
> because people said they needed a very high rate of qp modifications
> and could not tolerate the performance downside of a dis-aggregated
> API.
>
> I don't think CQ falls into that reasoning, so it should be
> dis-aggregated.
>
> Many things in verbs are terrible examples of how to make APIs, we
> should not just blindly copy things..

"Many things" ???
At least, you found something that is not terrible, I didn't find yet.

For the libibverb's API, I completely agree with you that we need to
strive to more simple API without overloaded functions with a lot of
parameters and you can count on my voice and support for any effort
to do so.

However, I don't think that kernel to libibverbs API should follow the
same path. The centralized entry points to the kernel provides better
enforcement and minimizes system call bloat.

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]