linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: linux-rdma@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: [PATCH v1 05/14] SUNRPC: Proper metric accounting when RPC is not transmitted
Date: Wed, 09 Nov 2016 14:05:29 -0500	[thread overview]
Message-ID: <20161109190529.15007.84163.stgit@manet.1015granger.net> (raw)
In-Reply-To: <20161109184735.15007.96507.stgit@manet.1015granger.net>

I noticed recently that during an xfstests on a krb5i mount, the
retransmit count for certain operations had gone negative, and the
backlog value became unreasonably large. I recall that Andy has
pointed this out to me in the past.

When call_refresh fails to find a valid credential for an RPC, the
RPC exits immediately without sending anything on the wire. This
leaves rq_ntrans, rq_xtime, and rq_rtt set to zero.

The solution for om_queue is to not add the to RPC's running backlog
queue total whenever rq_xtime is zero.

For om_ntrans, it's a bit more difficult. A zero rq_ntrans causes
om_ops to become larger than om_ntrans. The design of the RPC
metrics API assumes that ntrans will always be equal to or larger
than the ops count. The result is that when an RPC fails to find
credentials, the RPC operation's reported retransmit count, which is
computed in user space as the difference between ops and ntrans,
goes negative.

Ideally the kernel API should report a separate retransmit and
"exited before initial transmission" metric, so that user space can
sort out the difference properly.

To avoid kernel API changes and changes to the way rq_ntrans is used
when performing transport locking, account for untransmitted RPCs
so that om_ntrans keeps up with om_ops: always add one or more to
om_ntrans.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/stats.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
index 2ecb994..caeb01a 100644
--- a/net/sunrpc/stats.c
+++ b/net/sunrpc/stats.c
@@ -157,15 +157,17 @@ void rpc_count_iostats_metrics(const struct rpc_task *task,
 	spin_lock(&op_metrics->om_lock);
 
 	op_metrics->om_ops++;
-	op_metrics->om_ntrans += req->rq_ntrans;
+	/* kernel API: om_ops must never become larger than om_ntrans */
+	op_metrics->om_ntrans += max(req->rq_ntrans, 1);
 	op_metrics->om_timeouts += task->tk_timeouts;
 
 	op_metrics->om_bytes_sent += req->rq_xmit_bytes_sent;
 	op_metrics->om_bytes_recv += req->rq_reply_bytes_recvd;
 
-	delta = ktime_sub(req->rq_xtime, task->tk_start);
-	op_metrics->om_queue = ktime_add(op_metrics->om_queue, delta);
-
+	if (ktime_to_ns(req->rq_xtime)) {
+		delta = ktime_sub(req->rq_xtime, task->tk_start);
+		op_metrics->om_queue = ktime_add(op_metrics->om_queue, delta);
+	}
 	op_metrics->om_rtt = ktime_add(op_metrics->om_rtt, req->rq_rtt);
 
 	delta = ktime_sub(now, task->tk_start);


  parent reply	other threads:[~2016-11-09 19:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-09 19:04 [PATCH v1 00/14] client-side NFS/RDMA patches proposed for v4.10 Chuck Lever
2016-11-09 19:04 ` [PATCH v1 01/14] xprtrdma: Fix DMAR failure in frwr_op_map() after reconnect Chuck Lever
2016-11-09 19:05 ` [PATCH v1 02/14] xprtrdma: Cap size of callback buffer resources Chuck Lever
2016-11-09 19:05 ` [PATCH v1 03/14] xprtrdma: Make FRWR send queue entry accounting more accurate Chuck Lever
2016-11-09 19:05 ` [PATCH v1 04/14] xprtrdma: Support for SG_GAP devices Chuck Lever
2016-11-09 19:05 ` Chuck Lever [this message]
2016-11-09 19:05 ` [PATCH v1 06/14] xprtrdma: Address coverity complaint about wait_for_completion() Chuck Lever
2016-11-09 19:05 ` [PATCH v1 07/14] xprtrdma: Avoid calls to ro_unmap_safe() Chuck Lever
2016-11-09 19:05 ` [PATCH v1 08/14] xprtrdma: Squelch "max send, max recv" messages at connect time Chuck Lever
2016-11-09 19:06 ` [PATCH v1 09/14] xprtrdma: Shorten QP access error message Chuck Lever
2016-11-09 19:06 ` [PATCH v1 10/14] xprtrdma: Update dprintk in rpcrdma_count_chunks Chuck Lever
2016-11-09 19:06 ` [PATCH v1 11/14] xprtrdma: Relocate connection helper functions Chuck Lever
2016-11-09 19:06 ` [PATCH v1 12/14] xprtrdma: Simplify synopsis of rpcrdma_ep_connect() Chuck Lever
2016-11-09 19:06 ` [PATCH v1 13/14] xprtrdma: Refactor FRMR invalidation Chuck Lever
2016-11-09 19:06 ` [PATCH v1 14/14] xprtrdma: Update documenting comment Chuck Lever

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=20161109190529.15007.84163.stgit@manet.1015granger.net \
    --to=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;
as well as URLs for NNTP newsgroup(s).