Sean Hefty wrote:
+static inline int
+rdma_get_send_comp(struct rdma_cm_id *id, struct ibv_wc *wc)
+{
+ struct ibv_cq *cq;
+ void *context;
+ int ret;
+
+ ret = ibv_poll_cq(id->send_cq, 1, wc);
+ if (ret)
+ return ret;
+
+ ret = ibv_req_notify_cq(id->send_cq, 0);
+ if (ret)
+ return ret;
+
+ ret = ibv_poll_cq(id->send_cq, 1, wc);
+ if (ret)
+ return ret;
+
+ ret = ibv_get_cq_event(id->send_cq_channel, &cq, &context);
+ if (ret)
+ return ret;
This doesn't look correct. If the send isn't complete by the time the
2nd ibv_poll_cq() completes, then this function will return without
having filled in the wc. Or am I missing something? Shouldn't the
ibv_get_cq_event() be the first thing this function does? The same
issue/question exists for rdma_get_recv_comp().
I think it's possible for the function to return without having filled in a wc.
So its busted? Or is this intended behavior?
If the 2nd poll removes a completion, it can leave a cq event on the channel,
which a subsequent call could retrieve, but then find the cq empty.
The idea for this call is to abstract poll, notify_cq, and get_cq_event, but
still provide decent performance. (Scalability is a separate matter. I
couldn't find a decent way to abstract a CQ shared across QPs or between the
receive and send queues.)
To avoid returning from the call without a completion, I think the following
structure works:
poll()
notify_cq()
poll()
while (no completion) {
get_cq_event()
poll()
}
Is rdma_get_send_completion() supposed to return exactly one wc? If so
then the 2 polls can cause a wc to get silently discarded. I must
still not be understanding the intended use?
I would think this should just be:
get_cq_event()
notify_cq()
poll()
The only drawback I see is that it's theoretically possible to build up a queue
of cq events in the kernel. Not sure how to fix that. Any ideas?
That can always happen, yes?
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
