Return-Path: Received: from fieldses.org ([173.255.197.46]:55735 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751531AbbFASks (ORCPT ); Mon, 1 Jun 2015 14:40:48 -0400 Date: Mon, 1 Jun 2015 14:40:47 -0400 From: "J. Bruce Fields" To: Chuck Lever Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH v2 01/10] svcrdma: Fix byte-swapping in svc_rdma_sendto.c Message-ID: <20150601184047.GE26489@fieldses.org> References: <20150526174401.7061.43137.stgit@klimt.1015granger.net> <20150526174837.7061.88468.stgit@klimt.1015granger.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20150526174837.7061.88468.stgit@klimt.1015granger.net> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, May 26, 2015 at 01:48:37PM -0400, Chuck Lever wrote: > In send_write_chunks(), we have: > > for (xdr_off = rqstp->rq_res.head[0].iov_len, chunk_no = 0; > xfer_len && chunk_no < arg_ary->wc_nchunks; > chunk_no++) { > . . . > } > > Note that arg_ary->wc_nchunk is in network byte-order. For the > comparison to work correctly, both have to be in native byte-order. > > In send_reply_chunks, we have: > > write_len = min(xfer_len, htonl(ch->rs_length)); > > xfer_len is in native byte-order, and ch->rs_length is in > network byte-order. be32_to_cpu() is the correct byte swap > for ch->rs_length. > > As an additional clean up, replace ntohl() with be32_to_cpu() in > a few other places. Why? (Not arguing, really, just wondering.) --b. > > This appears to address a problem with large rsize hangs while > using PHYSICAL memory registration. I suspect that is the only > registration mode that uses more than one chunk element. > > BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=248 > Signed-off-by: Chuck Lever > --- > > net/sunrpc/xprtrdma/svc_rdma_sendto.c | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c > index 7de33d1..109e967 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c > @@ -240,6 +240,7 @@ static int send_write_chunks(struct svcxprt_rdma *xprt, > u32 xdr_off; > int chunk_off; > int chunk_no; > + int nchunks; > struct rpcrdma_write_array *arg_ary; > struct rpcrdma_write_array *res_ary; > int ret; > @@ -251,14 +252,15 @@ static int send_write_chunks(struct svcxprt_rdma *xprt, > &rdma_resp->rm_body.rm_chunks[1]; > > /* Write chunks start at the pagelist */ > + nchunks = be32_to_cpu(arg_ary->wc_nchunks); > for (xdr_off = rqstp->rq_res.head[0].iov_len, chunk_no = 0; > - xfer_len && chunk_no < arg_ary->wc_nchunks; > + xfer_len && chunk_no < nchunks; > chunk_no++) { > struct rpcrdma_segment *arg_ch; > u64 rs_offset; > > arg_ch = &arg_ary->wc_array[chunk_no].wc_target; > - write_len = min(xfer_len, ntohl(arg_ch->rs_length)); > + write_len = min(xfer_len, be32_to_cpu(arg_ch->rs_length)); > > /* Prepare the response chunk given the length actually > * written */ > @@ -270,7 +272,7 @@ static int send_write_chunks(struct svcxprt_rdma *xprt, > chunk_off = 0; > while (write_len) { > ret = send_write(xprt, rqstp, > - ntohl(arg_ch->rs_handle), > + be32_to_cpu(arg_ch->rs_handle), > rs_offset + chunk_off, > xdr_off, > write_len, > @@ -318,13 +320,13 @@ static int send_reply_chunks(struct svcxprt_rdma *xprt, > &rdma_resp->rm_body.rm_chunks[2]; > > /* xdr offset starts at RPC message */ > - nchunks = ntohl(arg_ary->wc_nchunks); > + nchunks = be32_to_cpu(arg_ary->wc_nchunks); > for (xdr_off = 0, chunk_no = 0; > xfer_len && chunk_no < nchunks; > chunk_no++) { > u64 rs_offset; > ch = &arg_ary->wc_array[chunk_no].wc_target; > - write_len = min(xfer_len, htonl(ch->rs_length)); > + write_len = min(xfer_len, be32_to_cpu(ch->rs_length)); > > /* Prepare the reply chunk given the length actually > * written */ > @@ -335,7 +337,7 @@ static int send_reply_chunks(struct svcxprt_rdma *xprt, > chunk_off = 0; > while (write_len) { > ret = send_write(xprt, rqstp, > - ntohl(ch->rs_handle), > + be32_to_cpu(ch->rs_handle), > rs_offset + chunk_off, > xdr_off, > write_len,