Linux NFS development
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagig@dev.mellanox.co.il>
To: Chuck Lever <chuck.lever@oracle.com>,
	linux-rdma@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH v1 05/10] svcrdma: Find rmsgp more reliably
Date: Sun, 11 Jan 2015 19:37:36 +0200	[thread overview]
Message-ID: <54B2B4E0.5060901@dev.mellanox.co.il> (raw)
In-Reply-To: <20150109192237.4901.92644.stgit@klimt.1015granger.net>

On 1/9/2015 9:22 PM, Chuck Lever wrote:
> xdr_start() can return the wrong rmsgp address if an assumption
> about how the xdr_buf was constructed changes.  When it gets it
> wrong, the client receives a reply that has gibberish in the
> RPC/RDMA header, preventing it from matching a waiting RPC request.
>
> Instead, make (and document) just one assumption: that the RDMA
> header for the client's RPC call is at the start of the first page
> in rq_pages.

Would it make more sense to add another pointer assigned at req
initialization (maybe in the RDMA request context) instead of hard
coding this assumption? I may be completely wrong here though...

>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>
>   net/sunrpc/xprtrdma/svc_rdma_sendto.c |   18 ++++--------------
>   1 files changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> index 7d79897..7de33d1 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> @@ -483,18 +483,6 @@ void svc_rdma_prep_reply_hdr(struct svc_rqst *rqstp)
>   {
>   }
>
> -/*
> - * Return the start of an xdr buffer.
> - */
> -static void *xdr_start(struct xdr_buf *xdr)
> -{
> -	return xdr->head[0].iov_base -
> -		(xdr->len -
> -		 xdr->page_len -
> -		 xdr->tail[0].iov_len -
> -		 xdr->head[0].iov_len);
> -}
> -
>   int svc_rdma_sendto(struct svc_rqst *rqstp)
>   {
>   	struct svc_xprt *xprt = rqstp->rq_xprt;
> @@ -512,8 +500,10 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
>
>   	dprintk("svcrdma: sending response for rqstp=%p\n", rqstp);
>
> -	/* Get the RDMA request header. */
> -	rdma_argp = xdr_start(&rqstp->rq_arg);
> +	/* Get the RDMA request header. The receive logic always
> +	 * places this at the start of page 0.
> +	 */
> +	rdma_argp = page_address(rqstp->rq_pages[0]);
>
>   	/* Build an req vec for the XDR */
>   	ctxt = svc_rdma_get_context(rdma);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


  reply	other threads:[~2015-01-11 17:37 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-09 19:21 [PATCH v1 00/10] NFS/RDMA server for 3.20 Chuck Lever
2015-01-09 19:22 ` [PATCH v1 01/10] svcrdma: Clean up dprintk Chuck Lever
2015-01-09 19:22 ` [PATCH v1 02/10] svcrdma: Remove unused variable Chuck Lever
2015-01-09 19:22 ` [PATCH v1 03/10] svcrdma: Clean up read chunk counting Chuck Lever
2015-01-09 19:22 ` [PATCH v1 04/10] svcrdma: Scrub BUG_ON() and WARN_ON() call sites Chuck Lever
2015-01-09 19:22 ` [PATCH v1 05/10] svcrdma: Find rmsgp more reliably Chuck Lever
2015-01-11 17:37   ` Sagi Grimberg [this message]
2015-01-12  0:30     ` Chuck Lever
2015-01-13 10:07       ` Sagi Grimberg
2015-01-09 19:22 ` [PATCH v1 06/10] svcrdma: Plant reader function in struct svcxprt_rdma Chuck Lever
2015-01-11 17:45   ` Sagi Grimberg
2015-01-12  0:41     ` Chuck Lever
2015-01-12 16:08       ` Steve Wise
2015-01-12 16:20         ` Chuck Lever
2015-01-12 16:26           ` Steve Wise
2015-01-12 16:45             ` Steve Wise
2015-01-13 10:05               ` Sagi Grimberg
2015-01-13 15:40                 ` Steve Wise
2015-01-09 19:22 ` [PATCH v1 07/10] svcrdma: rc_position sanity checking Chuck Lever
2015-01-09 19:23 ` [PATCH v1 08/10] svcrdma: Support RDMA_NOMSG requests Chuck Lever
2015-01-09 19:23 ` [PATCH v1 09/10] Move read list XDR round-up logic Chuck Lever
2015-01-09 20:14   ` J. Bruce Fields
2015-01-09 20:20     ` Chuck Lever
2015-01-09 19:23 ` [PATCH v1 10/10] svcrdma: Handle additional inline content Chuck Lever
2015-01-11 18:01   ` Sagi Grimberg
2015-01-12  1:13     ` Chuck Lever
2015-01-13 10:11       ` Sagi Grimberg
2015-01-13 14:35         ` Chuck Lever
2015-01-09 20:39 ` [PATCH v1 00/10] NFS/RDMA server for 3.20 J. Bruce Fields
2015-01-09 20:40   ` Chuck Lever
2015-01-09 20:44     ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54B2B4E0.5060901@dev.mellanox.co.il \
    --to=sagig@dev.mellanox.co.il \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox