Re: [PATCH rdma-core 2/2] srp_daemon: fix CQ handling

[Bart Van Assche <Bart.VanAssche-Sjgp3cTcYWE@public.gmane.org>]

On Tue, 2017-12-12 at 18:28 +0100, Nicolas Morey-Chaisemartin wrote:
> Le 12/12/2017 à 18:04, Bart Van Assche a écrit :
> > On Tue, 2017-12-12 at 15:08 +0100, Nicolas Morey-Chaisemartin wrote:
> > > +	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?

Hello Nicolas,

If you think another approach would work better for the return values than
what I proposed it's fine to use that approach. My primary concern is that
the meaning of the return values gets documented and also that the meaning
of the return values is clear without having to read the implementation of
this function.

Bart.