Le 12/12/2017 à 18:04, Bart Van Assche a écrit :
> On Tue, 2017-12-12 at 15:08 +0100, Nicolas Morey-Chaisemartin wrote:
>> +static int poll_cq_once(struct sync_resources *sync_res, struct ibv_cq *cq,
>> + struct ibv_wc *wc)
>> +{
>> + int ret;
>> + ret = ibv_poll_cq(cq, 1, wc);
> Please leave one blank line between declarations and statements.
OK
>
>> + if (ret < 0) {
>> + pr_err("poll CQ failed\n");
>> + return ret;
>> + }
>> +
>> + if (ret > 0 && wc->status != IBV_WC_SUCCESS) {
>> + if (!stop_threads(sync_res))
>> + pr_err("got bad completion with status: 0x%x\n",
>> + wc->status);
>> + return -ret;
>> + }
>> +
>> + return ret;
>> +}
> This function can return negative values, positive values or zero. Please
> modify it such that either 0 or a negative Unix error code is returned. This
> will make it easier to read this function and also the code that calls this
> function.
We still need to differenciate between OK with 0 completions polled, and OK with 1 completion.
I did it this way to limit the side effect of factoring the code.
Would you want poll_cq_once to return -EAGAIN when no completion was found?
I just realized the code is broken anyway.
if poll_cq_once returns an error in the second poll_cq loop, it is not returned by poll_cq itself (simply return 0).
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
