[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DB24E2C7-5CA1-49B7-A748-B3DE435018F5@oracle.com>
Date: Fri, 21 Apr 2017 23:37:31 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Geliang Tang <geliangtang@...il.com>
Cc: "J. Bruce Fields" <bfields@...ldses.org>,
Jeff Layton <jlayton@...chiereds.net>,
Trond Myklebust <trond.myklebust@...marydata.com>,
Anna Schumaker <anna.schumaker@...app.com>,
"David S. Miller" <davem@...emloft.net>,
Sagi Grimberg <sagi@...mberg.me>,
Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] xprtrdma: use offset_in_page() macro
> On Apr 21, 2017, at 9:21 PM, Geliang Tang <geliangtang@...il.com> wrote:
>
> Use offset_in_page() macro instead of open-coding.
>
> Signed-off-by: Geliang Tang <geliangtang@...il.com>
> ---
> net/sunrpc/xprtrdma/rpc_rdma.c | 4 ++--
> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 3 +--
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> index a044be2..429beea 100644
> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> @@ -540,7 +540,7 @@ rpcrdma_prepare_msg_sges(struct rpcrdma_ia *ia, struct rpcrdma_req *req,
> goto out;
>
> page = virt_to_page(xdr->tail[0].iov_base);
> - page_base = (unsigned long)xdr->tail[0].iov_base & ~PAGE_MASK;
> + page_base = offset_in_page(xdr->tail[0].iov_base);
>
> /* If the content in the page list is an odd length,
> * xdr_write_pages() has added a pad at the beginning
> @@ -587,7 +587,7 @@ rpcrdma_prepare_msg_sges(struct rpcrdma_ia *ia, struct rpcrdma_req *req,
> */
> if (xdr->tail[0].iov_len) {
> page = virt_to_page(xdr->tail[0].iov_base);
> - page_base = (unsigned long)xdr->tail[0].iov_base & ~PAGE_MASK;
> + page_base = offset_in_page(xdr->tail[0].iov_base);
> len = xdr->tail[0].iov_len;
>
> map_tail:
There are several other sites that use PAGE_MASK in
rpc_rdma.c. Should those be included in this patch?
Do you have a way to test this change? If not I
can take it (once the above comment is addressed),
run it through the usual battery of NFS/RDMA
testing, and then pass it along to Anna.
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> index 1736337..60b3f29 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> @@ -306,12 +306,11 @@ static int svc_rdma_dma_map_buf(struct svcxprt_rdma *rdma,
> unsigned char *base,
> unsigned int len)
> {
> - unsigned long offset = (unsigned long)base & ~PAGE_MASK;
> struct ib_device *dev = rdma->sc_cm_id->device;
> dma_addr_t dma_addr;
>
> dma_addr = ib_dma_map_page(dev, virt_to_page(base),
> - offset, len, DMA_TO_DEVICE);
> + offset_in_page(base), len, DMA_TO_DEVICE);
> if (ib_dma_mapping_error(dev, dma_addr))
> return -EIO;
>
This hunk conflicts with a rewrite of svc_rdma_sendto.c that
Bruce has already accepted for v4.12. I would prefer this
be dropped.
The rewritten code also has this issue. I can submit a patch
separately that adds offset_in_page in the appropriate place.
--
Chuck Lever
Powered by blists - more mailing lists