Thread (1 message) 1 message, 1 author, 2016-05-29

Re: [PATCH V4 libibverbs 2/7] Add member functions to poll an extended CQ

From: Doug Ledford <hidden>
Date: 2016-05-29 23:47:54


Sent from my iPhone
On May 29, 2016, at 7:30 PM, Jason Gunthorpe [off-list ref] wrote:
quoted
On Sun, May 29, 2016 at 06:38:41PM -0400, Doug Ledford wrote:

So I've been busy with kernel stuff, but I don't really like this patch.
I'm not really enamored with the entire poll_ex API.
You should really read the whole prior discussion
If there's something in particular you'd like to point out, feel free. 
quoted
First, I don't like the start_poll/poll/end_poll setup.  I would rather
we do something like using refcounting on the WCs.  Maybe something like
returning an array of pointers to WCs where each WC already has a
ref
Refs? Yuk. That doesn't fit the typical use model, and refs
involve more expensive locking per wc.
It doesn't have to be refs. If I recall correctly, the libmlx4 driver has to clear each cqe by changing the ownership status back to the hardware or something like that. The release could just as easily be setting that ownership on the cqe.
quoted
When a cq is created by the driver, it is responsible for filling out
the offsets array (can be static if your driver only has one CQE
format,
*shrug* Careful benchmarking would have to prove if this is better or
not.
Agreed.
Based on Yishai's comments I expect it is not better, since I
expect an offsets array to perform worse than the original bitmask
thing.
The original bitmask version included a copy to an interim, variable struct.  The idea here is that the cqe is an opaque pointer directly to the hardware cqe. There would be no interim copies, just retrieving the desired data directly from the cqe. Maybe I should have called it a hcqe to be more clear. 
Yishai's said this benchmarked better than the bitmask
I would totally expect that. The bitmap version had both a data copy and a variable struct requiring lots of branches.
and equal to
or better than today's versions..
Today's version has a data copy. What I'm suggesting here eliminates that entirely and all of the branches can be optimized away at compile time, leaving a load from a hot cache line of an offset value and then a direct load from that offset in the hcqe. I would think that because of the elimination of the data copy and the compile time elimination of branches, this has the potential to meet or exceed today's implementation. I would certainly like to see the numbers. 
This is a whole picture optimization and the user side is only one
part of the equation - the function pointer scheme is also optimizing
the driver path quite heavily, which is why it is showing positively
in benchmarks.
Passing an opaque hcqe pointer to the user and then only getting the data they want out of it should similarly help optimize the driver's path I would think.--
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help