Linux NFS development
 help / color / mirror / Atom feed
From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: linux-nfs@vger.kernel.org
Cc: Chuck Lever <chuck.lever@oracle.com>,
	Tom Talpey <thomas.talpey@netapp.com>,
	Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: [PATCH 30/37] SUNRPC: Ensure all transports set rq_xtime consistently
Date: Thu, 12 Jun 2008 15:22:01 -0400	[thread overview]
Message-ID: <20080612192201.24528.52047.stgit@localhost.localdomain> (raw)
In-Reply-To: <20080612192159.24528.43756.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

From: Chuck Lever <chuck.lever@oracle.com>

The RPC client uses the rq_xtime field in each RPC request to determine the
round-trip time of the request.  Currently, the rq_xtime field is
initialized by each transport just before it starts enqueing a request to
be sent.  However, transports do not handle initializing this value
consistently; sometimes they don't initialize it at all.

To make the measurement of request round-trip time consistent for all
RPC client transport capabilities, pull rq_xtime initialization into the
RPC client's generic transport logic.  Now all transports will get a
standardized RTT measure automatically, from:

  xprt_transmit()

to

  xprt_complete_rqst()

This makes round-trip time calculation more accurate for the TCP transport.
The socket ->sendmsg() method can return "-EAGAIN" if the socket's output
buffer is full, so the TCP transport's ->send_request() method may call
the ->sendmsg() method repeatedly until it gets all of the request's bytes
queued in the socket's buffer.

Currently, the TCP transport sets the rq_xtime field every time through
that loop so the final value is the timestamp just before the *last* call
to the underlying socket's ->sendmsg() method.  After this patch, the
rq_xtime field contains a timestamp that reflects the time just before the
*first* call to ->sendmsg().

This is consequential under heavy workloads because large requests often
take multiple ->sendmsg() calls to get all the bytes of a request queued.
The TCP transport causes the request to sleep until the remote end of the
socket has received enough bytes to clear space in the socket's local
output buffer.  This delay can be quite significant.

The method introduced by this patch is a more accurate measure of RTT
for stream transports, since the server can cause enough back pressure
to delay (ie increase the latency of) requests from the client.

Additionally, this patch corrects the behavior of the RDMA transport, which
entirely neglected to initialize the rq_xtime field.  RPC performance
metrics for RDMA transports now display correct RPC request round trip
times.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Acked-by: Tom Talpey <thomas.talpey@netapp.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 net/sunrpc/xprt.c     |    1 +
 net/sunrpc/xprtsock.c |    2 --
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 67996bd..99a52aa 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -872,6 +872,7 @@ void xprt_transmit(struct rpc_task *task)
 		return;
 
 	req->rq_connect_cookie = xprt->connect_cookie;
+	req->rq_xtime = jiffies;
 	status = xprt->ops->send_request(task);
 	if (status == 0) {
 		dprintk("RPC: %5u xmit complete\n", task->tk_pid);
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index ddbe981..4486c59 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -579,7 +579,6 @@ static int xs_udp_send_request(struct rpc_task *task)
 				req->rq_svec->iov_base,
 				req->rq_svec->iov_len);
 
-	req->rq_xtime = jiffies;
 	status = xs_sendpages(transport->sock,
 			      xs_addr(xprt),
 			      xprt->addrlen, xdr,
@@ -671,7 +670,6 @@ static int xs_tcp_send_request(struct rpc_task *task)
 	 * to cope with writespace callbacks arriving _after_ we have
 	 * called sendmsg(). */
 	while (1) {
-		req->rq_xtime = jiffies;
 		status = xs_sendpages(transport->sock,
 					NULL, 0, xdr, req->rq_bytes_sent);
 


  parent reply	other threads:[~2008-06-12 19:35 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-12 19:22 [PATCH 00/37] Patches to be added to the 'devel' branch Trond Myklebust
     [not found] ` <20080612192159.24528.43756.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-06-12 19:22   ` [PATCH 10/37] NFS: Update help text for CONFIG_NFS_FS Trond Myklebust
2008-06-12 19:22   ` [PATCH 05/37] SUNRPC: An ENOMEM error from call_encode is always fatal Trond Myklebust
     [not found]     ` <20080612192200.24528.71693.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-06-13 21:10       ` J. Bruce Fields
2008-06-12 19:22   ` [PATCH 01/37] NFS: Fix a preemption count leak in nfs_update_request Trond Myklebust
2008-06-12 19:22   ` [PATCH 15/37] SUNRPC: Display some debugging information as text rather than numbers Trond Myklebust
2008-06-12 19:22   ` [PATCH 14/37] SUNRPC: Refactor rpc_show_tasks Trond Myklebust
2008-06-12 19:22   ` [PATCH 06/37] NFS: Optimise append writes with holes Trond Myklebust
2008-06-12 19:22   ` [PATCH 03/37] NFS: Add correct bounds checking to NFSv2 locks Trond Myklebust
2008-06-12 19:22   ` [PATCH 07/37] NFS: Revert commit 44dd151d Trond Myklebust
2008-06-12 19:22   ` [PATCH 12/37] SUNRPC: Rename "call_" functions that are no longer FSM states Trond Myklebust
2008-06-12 19:22   ` [PATCH 16/37] NFS: Make nfs_fsync methods consistent Trond Myklebust
2008-06-12 19:22   ` [PATCH 11/37] SUNRPC: Add a function to display the name of an RPC procedure Trond Myklebust
2008-06-12 19:22   ` [PATCH 04/37] SUNRPC: Ensure we exit early in case of an encode error Trond Myklebust
2008-06-12 19:22   ` [PATCH 17/37] NFS: Make nfs_llseek methods consistent Trond Myklebust
2008-06-12 19:22   ` [PATCH 09/37] NFS: do_setlk(): don't flush caches when we have a delegation Trond Myklebust
2008-06-12 19:22   ` [PATCH 08/37] SUNRPC: Use GFP_NOFS when allocating credentials Trond Myklebust
     [not found]     ` <20080612192200.24528.65570.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2008-06-13 21:17       ` J. Bruce Fields
2008-06-13 21:26         ` Trond Myklebust
2008-06-13 21:31           ` J. Bruce Fields
2008-06-13 21:34             ` Trond Myklebust
2008-06-13 21:58               ` J. Bruce Fields
2008-06-13 22:07                 ` Trond Myklebust
2008-06-12 19:22   ` [PATCH 13/37] SUNRPC: Don't display the rpc_show_tasks header if there are no tasks Trond Myklebust
2008-06-12 19:22   ` [PATCH 02/37] NFS: nfs_updatepage(): don't mark page as dirty if an error occurred Trond Myklebust
2008-06-12 19:22   ` [PATCH 36/37] NFS: Treat "intr" and "nointr" options as deprecated Trond Myklebust
2008-06-12 19:22   ` [PATCH 28/37] rpc: minor cleanup of scheduler callback code Trond Myklebust
2008-06-12 19:22   ` [PATCH 29/37] NFS: Fix the ftruncate() credential problem Trond Myklebust
2008-06-12 19:22   ` [PATCH 32/37] NFS: Move fs/nfs/iostat.h to include/linux Trond Myklebust
2008-06-12 19:22   ` [PATCH 37/37] NFS: missing newline in NFS mount debugging message Trond Myklebust
2008-06-12 19:22   ` [PATCH 22/37] SUNRPC: Remove obsolete messages during transport connect Trond Myklebust
2008-06-12 19:22   ` [PATCH 26/37] rpc: eliminate unused variable in auth_gss upcall code Trond Myklebust
2008-06-12 19:22   ` [PATCH 23/37] fs/nfs/nfsroot.c: remove CVS keyword Trond Myklebust
2008-06-12 19:22   ` [PATCH 21/37] NFS: Fix trace debugging nits in write.c Trond Myklebust
2008-06-12 19:22   ` Trond Myklebust [this message]
2008-06-12 19:22   ` [PATCH 24/37] NFS: implement option checking when remounting NFS filesystems (resend) Trond Myklebust
2008-06-12 19:22   ` [PATCH 27/37] rpc: remove some unused macros Trond Myklebust
2008-06-12 19:22   ` [PATCH 25/37] rpc: bring back cl_chatty Trond Myklebust
2008-06-12 19:22   ` [PATCH 31/37] NFS: Remove the redundant file_open entry from struct nfs_rpc_ops Trond Myklebust
2008-06-12 19:22   ` [PATCH 34/37] NFS: Ensure we zap only the access and acl caches when setting new acls Trond Myklebust
2008-06-12 19:22   ` [PATCH 19/37] NFS: Add debugging facility for NFS aops Trond Myklebust
2008-06-12 19:22   ` [PATCH 20/37] NFS: Use NFSDBG_FILE for all fops Trond Myklebust
2008-06-12 19:22   ` [PATCH 35/37] NFS: Allow any value for the "retry" option Trond Myklebust
2008-06-12 19:22   ` [PATCH 33/37] NFS: Fix a warning in nfs4_async_handle_error Trond Myklebust
2008-06-12 19:22   ` [PATCH 18/37] NFS: Make nfs_open methods consistent Trond Myklebust

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=20080612192201.24528.52047.stgit@localhost.localdomain \
    --to=trond.myklebust@netapp.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=thomas.talpey@netapp.com \
    /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