Commit b334eaab authored by Tom Tucker's avatar Tom Tucker Committed by Trond Myklebust

RPC/RDMA: fix connection IRD/ORD setting

This logic sets the connection parameter that configures the local device
and informs the remote peer how many concurrent incoming RDMA_READ
requests are supported. The original logic didn't really do what was
intended for two reasons:

- The max number supported by the device is typically smaller than
any one factor in the calculation used, and

- The field in the connection parameter structure where the value is
stored is a u8 and always overflows for the default settings.

So what really happens is the value requested for responder resources
is the left over 8 bits from the "desired value". If the desired value
happened to be a multiple of 256, the result was zero and it wouldn't
connect at all.

Given the above and the fact that max_requests is almost always larger
than the max responder resources supported by the adapter, this patch
simplifies this logic and simply requests the max supported by the device,
subject to a reasonable limit.

This bug was found by Jim Schutt at Sandia.
Signed-off-by: default avatarTom Tucker <tom@opengridcomputing.com>
Acked-by: default avatarTom Talpey <talpey@netapp.com>
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 3197d309
...@@ -705,30 +705,13 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, ...@@ -705,30 +705,13 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia,
ep->rep_remote_cma.private_data_len = 0; ep->rep_remote_cma.private_data_len = 0;
/* Client offers RDMA Read but does not initiate */ /* Client offers RDMA Read but does not initiate */
switch (ia->ri_memreg_strategy) { ep->rep_remote_cma.initiator_depth = 0;
case RPCRDMA_BOUNCEBUFFERS: if (ia->ri_memreg_strategy == RPCRDMA_BOUNCEBUFFERS)
ep->rep_remote_cma.responder_resources = 0; ep->rep_remote_cma.responder_resources = 0;
break; else if (devattr.max_qp_rd_atom > 32) /* arbitrary but <= 255 */
case RPCRDMA_MTHCAFMR: ep->rep_remote_cma.responder_resources = 32;
case RPCRDMA_REGISTER: else
case RPCRDMA_FRMR:
ep->rep_remote_cma.responder_resources = cdata->max_requests *
(RPCRDMA_MAX_DATA_SEGS / 8);
break;
case RPCRDMA_MEMWINDOWS:
case RPCRDMA_MEMWINDOWS_ASYNC:
#if RPCRDMA_PERSISTENT_REGISTRATION
case RPCRDMA_ALLPHYSICAL:
#endif
ep->rep_remote_cma.responder_resources = cdata->max_requests *
(RPCRDMA_MAX_DATA_SEGS / 2);
break;
default:
break;
}
if (ep->rep_remote_cma.responder_resources > devattr.max_qp_rd_atom)
ep->rep_remote_cma.responder_resources = devattr.max_qp_rd_atom; ep->rep_remote_cma.responder_resources = devattr.max_qp_rd_atom;
ep->rep_remote_cma.initiator_depth = 0;
ep->rep_remote_cma.retry_count = 7; ep->rep_remote_cma.retry_count = 7;
ep->rep_remote_cma.flow_control = 0; ep->rep_remote_cma.flow_control = 0;
...@@ -858,14 +841,6 @@ if (strnicmp(ia->ri_id->device->dma_device->bus->name, "pci", 3) == 0) { ...@@ -858,14 +841,6 @@ if (strnicmp(ia->ri_id->device->dma_device->bus->name, "pci", 3) == 0) {
} }
} }
/* Theoretically a client initiator_depth > 0 is not needed,
* but many peers fail to complete the connection unless they
* == responder_resources! */
if (ep->rep_remote_cma.initiator_depth !=
ep->rep_remote_cma.responder_resources)
ep->rep_remote_cma.initiator_depth =
ep->rep_remote_cma.responder_resources;
ep->rep_connected = 0; ep->rep_connected = 0;
rc = rdma_connect(ia->ri_id, &ep->rep_remote_cma); rc = rdma_connect(ia->ri_id, &ep->rep_remote_cma);
...@@ -894,14 +869,16 @@ if (strnicmp(ia->ri_id->device->dma_device->bus->name, "pci", 3) == 0) { ...@@ -894,14 +869,16 @@ if (strnicmp(ia->ri_id->device->dma_device->bus->name, "pci", 3) == 0) {
if (ep->rep_connected <= 0) { if (ep->rep_connected <= 0) {
/* Sometimes, the only way to reliably connect to remote /* Sometimes, the only way to reliably connect to remote
* CMs is to use same nonzero values for ORD and IRD. */ * CMs is to use same nonzero values for ORD and IRD. */
ep->rep_remote_cma.initiator_depth = if (retry_count++ <= RDMA_CONNECT_RETRY_MAX + 1 &&
ep->rep_remote_cma.responder_resources; (ep->rep_remote_cma.responder_resources == 0 ||
if (ep->rep_remote_cma.initiator_depth == 0) ep->rep_remote_cma.initiator_depth !=
++ep->rep_remote_cma.initiator_depth; ep->rep_remote_cma.responder_resources)) {
if (ep->rep_remote_cma.responder_resources == 0) if (ep->rep_remote_cma.responder_resources == 0)
++ep->rep_remote_cma.responder_resources; ep->rep_remote_cma.responder_resources = 1;
if (retry_count++ == 0) ep->rep_remote_cma.initiator_depth =
ep->rep_remote_cma.responder_resources;
goto retry; goto retry;
}
rc = ep->rep_connected; rc = ep->rep_connected;
} else { } else {
dprintk("RPC: %s: connected\n", __func__); dprintk("RPC: %s: connected\n", __func__);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment