xprtrdma: Fix calculation of ri_max_send_sges

"Linux Kernel Mailing List" <[email protected]>
Newsgroups gmane.linux.kernel.commits.head
Message-ID <[email protected]>
Web:        https://git.kernel.org/torvalds/c/1179e2c27efe21167ec9d882b14becefba2ee990
Commit:     1179e2c27efe21167ec9d882b14becefba2ee990
Parent:     21ead9ff3dc72604d89499a1da5a18cc193ec4ff
Refname:    refs/heads/master
Author:     Chuck Lever <[email protected]>
AuthorDate: Wed Jan 31 12:34:05 2018 -0500
Committer:  Anna Schumaker <[email protected]>
CommitDate: Fri Feb 2 13:29:57 2018 -0500

    xprtrdma: Fix calculation of ri_max_send_sges
    
    Commit 16f906d66cd7 ("xprtrdma: Reduce required number of send
    SGEs") introduced the rpcrdma_ia::ri_max_send_sges field. This fixes
    a problem where xprtrdma would not work if the device's max_sge
    capability was small (low single digits).
    
    At least RPCRDMA_MIN_SEND_SGES are needed for the inline parts of
    each RPC. ri_max_send_sges is set to this value:
    
      ia->ri_max_send_sges = max_sge - RPCRDMA_MIN_SEND_SGES;
    
    Then when marshaling each RPC, rpcrdma_args_inline uses that value
    to determine whether the device has enough Send SGEs to convey an
    NFS WRITE payload inline, or whether instead a Read chunk is
    required.
    
    More recently, commit ae72950abf99 ("xprtrdma: Add data structure to
    manage RDMA Send arguments") used the ri_max_send_sges value to
    calculate the size of an array, but that commit erroneously assumed
    ri_max_send_sges contains a value similar to the device's max_sge,
    and not one that was reduced by the minimum SGE count.
    
    This assumption results in the calculated size of the sendctx's
    Send SGE array to be too small. When the array is used to marshal
    an RPC, the code can write Send SGEs into the following sendctx
    element in that array, corrupting it. When the device's max_sge is
    large, this issue is entirely harmless; but it results in an oops
    in the provider's post_send method, if dev.attrs.max_sge is small.
    
    So let's straighten this out: ri_max_send_sges will now contain a
    value with the same meaning as dev.attrs.max_sge, which makes
    the code easier to understand, and enables rpcrdma_sendctx_create
    to calculate the size of the SGE array correctly.
    
    Reported-by: Michal Kalderon <[email protected]>
    Fixes: 16f906d66cd7 ("xprtrdma: Reduce required number of send SGEs")
    Signed-off-by: Chuck Lever <[email protected]>
    Tested-by: Michal Kalderon <[email protected]>
    Cc: [email protected] # v4.10+
    Signed-off-by: Anna Schumaker <[email protected]>
---
 net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
 net/sunrpc/xprtrdma/verbs.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index 162e5dd82466..f0855a959a27 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -143,7 +143,7 @@ static bool rpcrdma_args_inline(struct rpcrdma_xprt *r_xprt,
 	if (xdr->page_len) {
 		remaining = xdr->page_len;
 		offset = offset_in_page(xdr->page_base);
-		count = 0;
+		count = RPCRDMA_MIN_SEND_SGES;
 		while (remaining) {
 			remaining -= min_t(unsigned int,
 					   PAGE_SIZE - offset, remaining);
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index f4eb63e8e689..bb56b9d849c4 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -505,7 +505,7 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia,
 		pr_warn("rpcrdma: HCA provides only %d send SGEs\n", max_sge);
 		return -ENOMEM;
 	}
-	ia->ri_max_send_sges = max_sge - RPCRDMA_MIN_SEND_SGES;
+	ia->ri_max_send_sges = max_sge;
 
 	if (ia->ri_device->attrs.max_qp_wr <= RPCRDMA_BACKWARD_WRS) {
 		dprintk("RPC:       %s: insufficient wqe's available\n",
--
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.