Re: [PATCH V1 libibverbs 1/8] Add ibv_poll_cq_ex verb

Jason Gunthorpe
2016-03-01T17:24:48Z

On Tue, Mar 01, 2016 at 10:52:33AM +0200, Matan Barak (External) wrote:
> >By my count it scores about a 2 on the Rusty API scale, which is
> >pretty bad.
>
> This is like writing something that means nothing....

If you don't agree with the points on the Rusty scale as what
constitutes good API design then we really have no common ground on
this point at all.

> >This series trade cache line efficiency for computation efficiency,
> >and it isn't at all clear to me that is going to be a win. Better
> >include some good benchmarks.
> >
>
> WRONG. Which computational overhead are you talking about?

I don't see the driver side posted, but obviously there must be
indexing maths and possibly branching, depending on the design, that
is overhead.

> The user-space driver writes one completion at a time, which (depending on
> the user configuration) is probably less than a cache line. Why do you think
> it's worse than how ibv_poll_cq behaves? The way the user-space driver
> optimizes/dirties the cache line is unrelated to the API.

I didn't say it was worse, I questioned that this should be the
overarching goal when no data to support this has been posted and it
doesn't even make inutitive sense that worrying about the size of
*dirty cache lines* is even very important, (well, within limits).

Christoph may have some data, but I do wonder if his results are
polluted by the current ibv_poll_cq driver implementations which do
trigger RMW overheads.

> Function per every completion fields permutation is uglier for sure (there
> are currently 9 optional completion fields in this proposal, you could
> easily do the math to calculate how many permutations we have).

Why would you do every permutation?

Try a non-memcpy API design. Provide an opaque 'cursor' and functions
to return certain data. This has more unconditional branches, but may
avoid branching within the driver and certainly avoid memcpy's and
pretty much all cache line dirtying. Sean had some results that
were positive on this sort of direction.

Justify this horrible API is *necessary* with something concrete, just
not random hand waving and mumbling about cache lines. That means
benchmark several options.

Jason