> On Oct 1, 2015, at 1:13 PM, Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Oct 01, 2015 at 12:37:36PM -0400, Chuck Lever wrote:
>
>> A missed WC will result in an RPC/RDMA transport deadlock. In
>> fact that is the reason for this particular patch (although
>> it addresses only one source of missed WCs). So I would like
>> to see that there are no windows here.
>
> WCs are never missed.
The review comment earlier in this thread suggested there is
a race condition where a WC can be “delayed” resulting in,
well, I’m still not certain what the consequences are.
There was some interest in going back to a single WC array
argument when calling ib_poll_cq. I’d like to avoid that.
If there is an issue using a WC array, I’d like to
understand what it is.
> The issue is a race where re-arming the CQ might not work, meaning you
> don't get an event.
>
> You can certainly use arrays with poll_cq. There is no race in the API
> here.
>
> But you have to use the IB_CQ_REPORT_MISSED_EVENTS scheme to guarantee
> the CQ is actually armed or continue to loop again.
>
> Basically you have to loop until ib_req_notify_cq succeeds.
>
> Any driver that doesn't support this is broken, do we know of any?
>
> while (1) {
> struct ib_wc wcs[100];
> int rc = ib_poll_cq(cw, NELEMS(wcs), wcs);
>
> .. process rc wcs ..
>
> if (rc != NELEMS(wcs))
> if (ib_req_notify_cq(cq, IB_CQ_NEXT_COMP |
> IB_CQ_REPORT_MISSED_EVENTS) == 0)
> break;
> }
>
> API wise, we should probably look at forcing
> IB_CQ_REPORT_MISSED_EVENTS on and dropping the flag.
It’s been suggested that it’s not clear what a positive
return value from ib_req_notify_cq() means when the
REPORT_MISSED_EVENTS flags is set: does it mean that
the CQ has been re-armed? I had assumed that a positive
RC meant both missed events and a successful re-arm,
but the pseudo-code above suggests that is not the
case.
—
Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
