NFS: include filelayout DS rpc stats in mountstats

Include RPC statistics from all data servers in /proc/self/mountstats for pNFS filelayout mounts.

The additional info printed on the "per-op statistics" line does not break current mountstats(8) from nfs-utils.

Signed-off-by Weston Andros Adamson <[email protected]>

---

This is clearly a better approach than I was taking trying to keep DS rpc stats separate in /proc/self/mountstats.

FYI, there are no other callers of rpc_print_iostats().

The new output of /proc/self/mountstats works fine with current mountstats userland tool. The additional data on the per-op statistics line doesn't make it into the output, but I left it in /proc/self/mountstats because it doesn't break anything and can be included in newer versions of mountstats.

I plan on adding the per-DS stats (rpc and others) somewhere in /proc/fs/nfsfs/ soon.

fs/nfs/nfs4filelayout.c        |   30 ++++++++++++++++++++++++++
fs/nfs/pnfs.h                  |   23 ++++++++++++++++++++
fs/nfs/pnfs_dev.c              |   45 ++++++++++++++++++++++++++++++++++++++++
fs/nfs/super.c                 |    3 +-
include/linux/sunrpc/metrics.h |   13 +++++++++-
net/sunrpc/stats.c             |   38 ++++++++++++++++++++++++++++++++-
6 files changed, 147 insertions(+), 5 deletions(-)

fs/nfs/nfs4filelayout.c +30 -0

index 79be7ac..51c71e5 100644
@@ -33,6 +33,8 @@
 #include <linux/nfs_page.h>
 #include <linux/module.h>
 
+#include <linux/sunrpc/metrics.h>
+
 #include "internal.h"
 #include "nfs4filelayout.h"
 

@@ -918,6 +920,33 @@ filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d)
 	nfs4_fl_free_deviceid(container_of(d, struct nfs4_file_layout_dsaddr, id_node));
 }
 
+static unsigned int
+filelayout_rpc_add_iostats(struct rpc_iostats *total,
+			   unsigned int maxproc,
+			   struct nfs4_deviceid_node *d)
+{
+	struct nfs4_file_layout_dsaddr *dsaddr;
+	struct nfs4_pnfs_ds *ds;
+	unsigned int num_ds = 0;
+	u32 i;
+
+	dsaddr = container_of(d, struct nfs4_file_layout_dsaddr, id_node);
+
+	for (i = 0; i < dsaddr->ds_num; i++) {
+		ds = dsaddr->ds_list[i];
+
+		/* only add iostats if nfs_client is different from MDS */
+		if (ds && ds->ds_clp && d->nfs_client != ds->ds_clp) {
+			struct rpc_clnt *clnt = ds->ds_clp->cl_rpcclient;
+			rpc_add_iostats(total, clnt->cl_metrics,
+					min(maxproc, clnt->cl_maxproc));
+			num_ds++;
+		}
+	}
+
+	return num_ds;
+}
+
 static struct pnfs_layoutdriver_type filelayout_type = {
 	.id			= LAYOUT_NFSV4_1_FILES,
 	.name			= "LAYOUT_NFSV4_1_FILES",

@@ -932,6 +961,7 @@ static struct pnfs_layoutdriver_type filelayout_type = {
 	.read_pagelist		= filelayout_read_pagelist,
 	.write_pagelist		= filelayout_write_pagelist,
 	.free_deviceid_node	= filelayout_free_deveiceid_node,
+	.ds_rpc_add_iostats	= filelayout_rpc_add_iostats,
 };
 
 static int __init nfs4filelayout_init(void)

fs/nfs/pnfs.h +23 -0

index 53d593a..d1b82eb 100644
@@ -72,6 +72,7 @@ enum layoutdriver_policy_flags {
 };
 
 struct nfs4_deviceid_node;
+struct rpc_iostats;
 
 /* Per-layout driver specific registration structure */
 struct pnfs_layoutdriver_type {

@@ -119,6 +120,9 @@ struct pnfs_layoutdriver_type {
 	void (*encode_layoutcommit) (struct pnfs_layout_hdr *layoutid,
 				     struct xdr_stream *xdr,
 				     const struct nfs4_layoutcommit_args *args);
+
+	unsigned int (*ds_rpc_add_iostats) (struct rpc_iostats *, unsigned int,
+					    struct nfs4_deviceid_node *);
 };
 
 struct pnfs_layout_hdr {

@@ -239,6 +243,10 @@ void nfs4_init_deviceid_node(struct nfs4_deviceid_node *,
 struct nfs4_deviceid_node *nfs4_insert_deviceid_node(struct nfs4_deviceid_node *);
 bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *);
 void nfs4_deviceid_purge_client(const struct nfs_client *);
+void nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				     const struct nfs_client *clp,
+				     const struct pnfs_layoutdriver_type *ld);
+
 
 static inline int lo_fail_bit(u32 iomode)
 {

@@ -328,6 +336,15 @@ static inline int pnfs_return_layout(struct inode *ino)
 	return 0;
 }
 
+static inline int
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	if (!pnfs_enabled_sb(nfss))
+		return 0;
+	nfs4_deviceid_rpc_print_iostats(m, nfss->nfs_client,
+					nfss->pnfs_curr_ld);
+	return 1;
+}
 #else  /* CONFIG_NFS_V4_1 */
 
 static inline void pnfs_destroy_all_layouts(struct nfs_client *clp)

@@ -429,6 +446,12 @@ static inline int pnfs_layoutcommit_inode(struct inode *inode, bool sync)
 static inline void nfs4_deviceid_purge_client(struct nfs_client *ncl)
 {
 }
+
+static inline void
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	return 0;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 #endif /* FS_NFS_PNFS_H */

fs/nfs/pnfs_dev.c +45 -0

index 4f359d2..aa5d102 100644
@@ -29,6 +29,9 @@
 */
 
 #include <linux/export.h>
+
+#include <linux/sunrpc/metrics.h>
+
 #include "pnfs.h"
 
 #define NFSDBG_FACILITY		NFSDBG_PNFS

@@ -115,6 +118,48 @@ nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 }
 EXPORT_SYMBOL_GPL(nfs4_find_get_deviceid);
 
+
+void
+nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				const struct nfs_client *clp,
+				const struct pnfs_layoutdriver_type *ld)
+{
+	struct nfs4_deviceid_node *d;
+	struct hlist_node *n;
+	struct rpc_iostats *totals;
+	char msgbuf[64];
+	unsigned int num_ds = 0;
+	unsigned int maxproc;
+	long h;
+
+	if (!ld->ds_rpc_add_iostats)
+		return;
+	totals = rpc_alloc_iostats(clp->cl_rpcclient);
+	if (!totals)
+		return;
+	maxproc = clp->cl_rpcclient->cl_maxproc;
+	rpc_add_iostats(totals, clp->cl_rpcclient->cl_metrics, maxproc);
+
+	rcu_read_lock();
+	for (h = 0; h < NFS4_DEVICE_ID_HASH_SIZE; h++) {
+		hlist_for_each_entry_rcu(d, n, &nfs4_deviceid_cache[h], node) {
+			if (d->ld == ld && d->nfs_client == clp) {
+				if (atomic_read(&d->ref))
+					num_ds += ld->ds_rpc_add_iostats(totals,
+							maxproc, d);
+			}
+		}
+	}
+	rcu_read_unlock();
+
+	snprintf(msgbuf, sizeof(msgbuf),
+		 " for the metadata server and %u data server%s", num_ds,
+		 (num_ds != 1) ? "s" : "");
+	rpc_print_iostats(seq, clp->cl_rpcclient, totals, msgbuf);
+	rpc_free_iostats(totals);
+}
+EXPORT_SYMBOL_GPL(nfs4_deviceid_rpc_print_iostats);
+
 /*
 * Remove a deviceid from cache
 *

fs/nfs/super.c +2 -1

index d18a90b..76d0850 100644
@@ -870,7 +870,8 @@ static int nfs_show_stats(struct seq_file *m, struct dentry *root)
 #endif
 	seq_printf(m, "\n");
 
-	rpc_print_iostats(m, nfss->client);
+	if (!pnfs_rpc_print_iostats(m, nfss))
+		rpc_print_iostats(m, nfss->client, NULL, NULL);
 
 	return 0;
 }

include/linux/sunrpc/metrics.h +11 -2

index b6edbc0..e6e04e4 100644
@@ -75,14 +75,23 @@ struct rpc_clnt;
 
 struct rpc_iostats *	rpc_alloc_iostats(struct rpc_clnt *);
 void			rpc_count_iostats(struct rpc_task *);
-void			rpc_print_iostats(struct seq_file *, struct rpc_clnt *);
+void			rpc_print_iostats(struct seq_file *,
+					  struct rpc_clnt *,
+					  struct rpc_iostats *,
+					  const char *);
+void			rpc_add_iostats(struct rpc_iostats *,
+					struct rpc_iostats *, unsigned int);
 void			rpc_free_iostats(struct rpc_iostats *);
 
 #else  /*  CONFIG_PROC_FS  */
 
 static inline struct rpc_iostats *rpc_alloc_iostats(struct rpc_clnt *clnt) { return NULL; }
 static inline void rpc_count_iostats(struct rpc_task *task) {}
-static inline void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt) {}
+static inline void rpc_print_iostats(struct seq_file *seq,
+				     struct rpc_iostats *stat) {}
+static inline void rpc_add_iostats(struct rpc_iostats *total,
+				   struct rpc_iostats *new,
+				   unsigned int maxproc) {}
 static inline void rpc_free_iostats(struct rpc_iostats *stats) {}
 
 #endif  /*  CONFIG_PROC_FS  */

net/sunrpc/stats.c +36 -2

index 3c4f688..342df57 100644
@@ -176,12 +176,16 @@ static void _print_name(struct seq_file *seq, unsigned int op,
 		seq_printf(seq, "\t%12u: ", op);
 }
 
-void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
+void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt,
+		       struct rpc_iostats *altstats,
+		       const char *altstats_msg)
 {
 	struct rpc_iostats *stats = clnt->cl_metrics;
 	struct rpc_xprt *xprt = clnt->cl_xprt;
 	unsigned int op, maxproc = clnt->cl_maxproc;
 
+	if (altstats)
+		stats = altstats;
 	if (!stats)
 		return;
 

@@ -192,7 +196,8 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 	if (xprt)
 		xprt->ops->print_stats(xprt, seq);
 
-	seq_printf(seq, "\tper-op statistics\n");
+	seq_printf(seq, "\tper-op statistics%s\n",
+		   (altstats && altstats_msg) ? altstats_msg : "");
 	for (op = 0; op < maxproc; op++) {
 		struct rpc_iostats *metrics = &stats[op];
 		_print_name(seq, op, clnt->cl_procinfo);

@@ -209,6 +214,35 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 }
 EXPORT_SYMBOL_GPL(rpc_print_iostats);
 
+void
+rpc_add_iostats(struct rpc_iostats *totals, struct rpc_iostats *new,
+		unsigned int maxproc)
+{
+	unsigned int op;
+
+	for (op = 0; op < maxproc; op++) {
+		struct rpc_iostats *op_totals = &totals[op],
+				   *op_new = &new[op];
+
+		op_totals->om_ops += op_new->om_ops;
+		op_totals->om_ntrans += op_new->om_ntrans;
+		op_totals->om_timeouts += op_new->om_timeouts;
+
+		op_totals->om_bytes_sent += op_new->om_bytes_sent;
+		op_totals->om_bytes_recv += op_new->om_bytes_recv;
+
+		op_totals->om_queue = ktime_add(op_totals->om_queue,
+					       op_new->om_queue);
+
+		op_totals->om_rtt = ktime_add(op_totals->om_rtt,
+					      op_new->om_rtt);
+
+		op_totals->om_execute = ktime_add(op_totals->om_execute,
+						 op_new->om_execute);
+	}
+}
+EXPORT_SYMBOL_GPL(rpc_add_iostats);
+
 /*
 * Register/unregister RPC proc files
 */

Signature · 1 line

1.7.4.4

Quoted 6 lines · Weston Andros Adamson

Include RPC statistics from all data servers in /proc/self/mountstats for pNFS filelayout mounts.

The additional info printed on the "per-op statistics" line does not break current mountstats(8) from nfs-utils.

Quoted 5 lines

Signed-off-by Weston Andros Adamson <[email protected]>

---

This is clearly a better approach than I was taking trying to keep DS rpc stats separate in /proc/self/mountstats.

I guess you could argue that since these are "mount" stats, it's a per mount point thing, not necessarily a per server thing. We've simply had it that way because prior to pNFS, you couldn't have more than one server per mount point.

At some point we might consider also displaying some fs_locations data here, and perhaps a count of how many migration or replication failover events have been seen. For another day.

Quoted 314 lines

FYI, there are no other callers of rpc_print_iostats().

The new output of /proc/self/mountstats works fine with current mountstats userland tool. The additional data on the per-op statistics line doesn't make it into the output, but I left it in /proc/self/mountstats because it doesn't break anything and can be included in newer versions of mountstats.

I plan on adding the per-DS stats (rpc and others) somewhere in /proc/fs/nfsfs/ soon.

fs/nfs/nfs4filelayout.c        |   30 ++++++++++++++++++++++++++
fs/nfs/pnfs.h                  |   23 ++++++++++++++++++++
fs/nfs/pnfs_dev.c              |   45 ++++++++++++++++++++++++++++++++++++++++
fs/nfs/super.c                 |    3 +-
include/linux/sunrpc/metrics.h |   13 +++++++++-
net/sunrpc/stats.c             |   38 ++++++++++++++++++++++++++++++++-
6 files changed, 147 insertions(+), 5 deletions(-)

fs/nfs/nfs4filelayout.c +30 -0

index 79be7ac..51c71e5 100644
@@ -33,6 +33,8 @@
 #include <linux/nfs_page.h>
 #include <linux/module.h>
 
+#include <linux/sunrpc/metrics.h>
+
 #include "internal.h"
 #include "nfs4filelayout.h"
 

@@ -918,6 +920,33 @@ filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d)
 	nfs4_fl_free_deviceid(container_of(d, struct nfs4_file_layout_dsaddr, id_node));
 }
 
+static unsigned int
+filelayout_rpc_add_iostats(struct rpc_iostats *total,
+			   unsigned int maxproc,
+			   struct nfs4_deviceid_node *d)
+{
+	struct nfs4_file_layout_dsaddr *dsaddr;
+	struct nfs4_pnfs_ds *ds;
+	unsigned int num_ds = 0;
+	u32 i;
+
+	dsaddr = container_of(d, struct nfs4_file_layout_dsaddr, id_node);
+
+	for (i = 0; i < dsaddr->ds_num; i++) {
+		ds = dsaddr->ds_list[i];
+
+		/* only add iostats if nfs_client is different from MDS */
+		if (ds && ds->ds_clp && d->nfs_client != ds->ds_clp) {
+			struct rpc_clnt *clnt = ds->ds_clp->cl_rpcclient;
+			rpc_add_iostats(total, clnt->cl_metrics,
+					min(maxproc, clnt->cl_maxproc));
+			num_ds++;
+		}
+	}
+
+	return num_ds;
+}
+
 static struct pnfs_layoutdriver_type filelayout_type = {
 	.id			= LAYOUT_NFSV4_1_FILES,
 	.name			= "LAYOUT_NFSV4_1_FILES",

@@ -932,6 +961,7 @@ static struct pnfs_layoutdriver_type filelayout_type = {
 	.read_pagelist		= filelayout_read_pagelist,
 	.write_pagelist		= filelayout_write_pagelist,
 	.free_deviceid_node	= filelayout_free_deveiceid_node,
+	.ds_rpc_add_iostats	= filelayout_rpc_add_iostats,
 };
 
 static int __init nfs4filelayout_init(void)

fs/nfs/pnfs.h +23 -0

index 53d593a..d1b82eb 100644
@@ -72,6 +72,7 @@ enum layoutdriver_policy_flags {
 };
 
 struct nfs4_deviceid_node;
+struct rpc_iostats;
 
 /* Per-layout driver specific registration structure */
 struct pnfs_layoutdriver_type {

@@ -119,6 +120,9 @@ struct pnfs_layoutdriver_type {
 	void (*encode_layoutcommit) (struct pnfs_layout_hdr *layoutid,
 				     struct xdr_stream *xdr,
 				     const struct nfs4_layoutcommit_args *args);
+
+	unsigned int (*ds_rpc_add_iostats) (struct rpc_iostats *, unsigned int,
+					    struct nfs4_deviceid_node *);
 };
 
 struct pnfs_layout_hdr {

@@ -239,6 +243,10 @@ void nfs4_init_deviceid_node(struct nfs4_deviceid_node *,
 struct nfs4_deviceid_node *nfs4_insert_deviceid_node(struct nfs4_deviceid_node *);
 bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *);
 void nfs4_deviceid_purge_client(const struct nfs_client *);
+void nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				     const struct nfs_client *clp,
+				     const struct pnfs_layoutdriver_type *ld);
+
 
 static inline int lo_fail_bit(u32 iomode)
 {

@@ -328,6 +336,15 @@ static inline int pnfs_return_layout(struct inode *ino)
 	return 0;
 }
 
+static inline int
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	if (!pnfs_enabled_sb(nfss))
+		return 0;
+	nfs4_deviceid_rpc_print_iostats(m, nfss->nfs_client,
+					nfss->pnfs_curr_ld);
+	return 1;
+}
 #else  /* CONFIG_NFS_V4_1 */
 
 static inline void pnfs_destroy_all_layouts(struct nfs_client *clp)

@@ -429,6 +446,12 @@ static inline int pnfs_layoutcommit_inode(struct inode *inode, bool sync)
 static inline void nfs4_deviceid_purge_client(struct nfs_client *ncl)
 {
 }
+
+static inline void
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	return 0;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 #endif /* FS_NFS_PNFS_H */

fs/nfs/pnfs_dev.c +45 -0

index 4f359d2..aa5d102 100644
@@ -29,6 +29,9 @@
 */
 
 #include <linux/export.h>
+
+#include <linux/sunrpc/metrics.h>
+
 #include "pnfs.h"
 
 #define NFSDBG_FACILITY		NFSDBG_PNFS

@@ -115,6 +118,48 @@ nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 }
 EXPORT_SYMBOL_GPL(nfs4_find_get_deviceid);
 
+
+void
+nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				const struct nfs_client *clp,
+				const struct pnfs_layoutdriver_type *ld)
+{
+	struct nfs4_deviceid_node *d;
+	struct hlist_node *n;
+	struct rpc_iostats *totals;
+	char msgbuf[64];
+	unsigned int num_ds = 0;
+	unsigned int maxproc;
+	long h;
+
+	if (!ld->ds_rpc_add_iostats)
+		return;
+	totals = rpc_alloc_iostats(clp->cl_rpcclient);
+	if (!totals)
+		return;
+	maxproc = clp->cl_rpcclient->cl_maxproc;
+	rpc_add_iostats(totals, clp->cl_rpcclient->cl_metrics, maxproc);
+
+	rcu_read_lock();
+	for (h = 0; h < NFS4_DEVICE_ID_HASH_SIZE; h++) {
+		hlist_for_each_entry_rcu(d, n, &nfs4_deviceid_cache[h], node) {
+			if (d->ld == ld && d->nfs_client == clp) {
+				if (atomic_read(&d->ref))
+					num_ds += ld->ds_rpc_add_iostats(totals,
+							maxproc, d);
+			}
+		}
+	}
+	rcu_read_unlock();
+
+	snprintf(msgbuf, sizeof(msgbuf),
+		 " for the metadata server and %u data server%s", num_ds,
+		 (num_ds != 1) ? "s" : "");
+	rpc_print_iostats(seq, clp->cl_rpcclient, totals, msgbuf);
+	rpc_free_iostats(totals);
+}
+EXPORT_SYMBOL_GPL(nfs4_deviceid_rpc_print_iostats);
+
 /*
 * Remove a deviceid from cache
 *

fs/nfs/super.c +2 -1

index d18a90b..76d0850 100644
@@ -870,7 +870,8 @@ static int nfs_show_stats(struct seq_file *m, struct dentry *root)
 #endif
 	seq_printf(m, "\n");
 
-	rpc_print_iostats(m, nfss->client);
+	if (!pnfs_rpc_print_iostats(m, nfss))
+		rpc_print_iostats(m, nfss->client, NULL, NULL);
 
 	return 0;
 }

include/linux/sunrpc/metrics.h +11 -2

index b6edbc0..e6e04e4 100644
@@ -75,14 +75,23 @@ struct rpc_clnt;
 
 struct rpc_iostats *	rpc_alloc_iostats(struct rpc_clnt *);
 void			rpc_count_iostats(struct rpc_task *);
-void			rpc_print_iostats(struct seq_file *, struct rpc_clnt *);
+void			rpc_print_iostats(struct seq_file *,
+					  struct rpc_clnt *,
+					  struct rpc_iostats *,
+					  const char *);
+void			rpc_add_iostats(struct rpc_iostats *,
+					struct rpc_iostats *, unsigned int);
 void			rpc_free_iostats(struct rpc_iostats *);
 
 #else  /*  CONFIG_PROC_FS  */
 
 static inline struct rpc_iostats *rpc_alloc_iostats(struct rpc_clnt *clnt) { return NULL; }
 static inline void rpc_count_iostats(struct rpc_task *task) {}
-static inline void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt) {}
+static inline void rpc_print_iostats(struct seq_file *seq,
+				     struct rpc_iostats *stat) {}
+static inline void rpc_add_iostats(struct rpc_iostats *total,
+				   struct rpc_iostats *new,
+				   unsigned int maxproc) {}
 static inline void rpc_free_iostats(struct rpc_iostats *stats) {}
 
 #endif  /*  CONFIG_PROC_FS  */

net/sunrpc/stats.c +36 -2

index 3c4f688..342df57 100644
@@ -176,12 +176,16 @@ static void _print_name(struct seq_file *seq, unsigned int op,
 		seq_printf(seq, "\t%12u: ", op);
 }
 
-void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
+void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt,
+		       struct rpc_iostats *altstats,
+		       const char *altstats_msg)
 {
 	struct rpc_iostats *stats = clnt->cl_metrics;
 	struct rpc_xprt *xprt = clnt->cl_xprt;
 	unsigned int op, maxproc = clnt->cl_maxproc;
 
+	if (altstats)
+		stats = altstats;
 	if (!stats)
 		return;
 

@@ -192,7 +196,8 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 	if (xprt)
 		xprt->ops->print_stats(xprt, seq);
 
-	seq_printf(seq, "\tper-op statistics\n");
+	seq_printf(seq, "\tper-op statistics%s\n",
+		   (altstats && altstats_msg) ? altstats_msg : "");
 	for (op = 0; op < maxproc; op++) {
 		struct rpc_iostats *metrics = &stats[op];
 		_print_name(seq, op, clnt->cl_procinfo);

@@ -209,6 +214,35 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 }
 EXPORT_SYMBOL_GPL(rpc_print_iostats);
 
+void
+rpc_add_iostats(struct rpc_iostats *totals, struct rpc_iostats *new,
+		unsigned int maxproc)
+{
+	unsigned int op;
+
+	for (op = 0; op < maxproc; op++) {
+		struct rpc_iostats *op_totals = &totals[op],
+				   *op_new = &new[op];
+
+		op_totals->om_ops += op_new->om_ops;
+		op_totals->om_ntrans += op_new->om_ntrans;
+		op_totals->om_timeouts += op_new->om_timeouts;
+
+		op_totals->om_bytes_sent += op_new->om_bytes_sent;
+		op_totals->om_bytes_recv += op_new->om_bytes_recv;
+
+		op_totals->om_queue = ktime_add(op_totals->om_queue,
+					       op_new->om_queue);
+
+		op_totals->om_rtt = ktime_add(op_totals->om_rtt,
+					      op_new->om_rtt);
+
+		op_totals->om_execute = ktime_add(op_totals->om_execute,
+						 op_new->om_execute);
+	}
+}
+EXPORT_SYMBOL_GPL(rpc_add_iostats);
+
 /*
 * Register/unregister RPC proc files
 */

Signature · 1 line

1.7.4.4

Signature · 2 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Quoted 61 lines · Weston Andros Adamson

Include RPC statistics from all data servers in /proc/self/mountstats for pNFS filelayout mounts.

The additional info printed on the "per-op statistics" line does not break current mountstats(8) from nfs-utils.

Signed-off-by Weston Andros Adamson <[email protected]>

---

This is clearly a better approach than I was taking trying to keep DS rpc stats separate in /proc/self/mountstats.

FYI, there are no other callers of rpc_print_iostats().

The new output of /proc/self/mountstats works fine with current mountstats userland tool. The additional data on the per-op statistics line doesn't make it into the output, but I left it in /proc/self/mountstats because it doesn't break anything and can be included in newer versions of mountstats.

I plan on adding the per-DS stats (rpc and others) somewhere in /proc/fs/nfsfs/ soon.

fs/nfs/nfs4filelayout.c        |   30 ++++++++++++++++++++++++++
fs/nfs/pnfs.h                  |   23 ++++++++++++++++++++
fs/nfs/pnfs_dev.c              |   45 ++++++++++++++++++++++++++++++++++++++++
fs/nfs/super.c                 |    3 +-
include/linux/sunrpc/metrics.h |   13 +++++++++-
net/sunrpc/stats.c             |   38 ++++++++++++++++++++++++++++++++-
6 files changed, 147 insertions(+), 5 deletions(-)

fs/nfs/nfs4filelayout.c +16 -0

index 79be7ac..51c71e5 100644
@@ -33,6 +33,8 @@
 #include <linux/nfs_page.h>
 #include <linux/module.h>
 
+#include <linux/sunrpc/metrics.h>
+
 #include "internal.h"
 #include "nfs4filelayout.h"
 

@@ -918,6 +920,33 @@ filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d)
 	nfs4_fl_free_deviceid(container_of(d, struct nfs4_file_layout_dsaddr, id_node));
 }
 
+static unsigned int
+filelayout_rpc_add_iostats(struct rpc_iostats *total,
+			   unsigned int maxproc,
+			   struct nfs4_deviceid_node *d)
+{
+	struct nfs4_file_layout_dsaddr *dsaddr;
+	struct nfs4_pnfs_ds *ds;
+	unsigned int num_ds = 0;
+	u32 i;
+
+	dsaddr = container_of(d, struct nfs4_file_layout_dsaddr, id_node);
+
+	for (i = 0; i < dsaddr->ds_num; i++) {
+		ds = dsaddr->ds_list[i];

fs/nfs/nfs4filelayout.c filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d) · line 940

Quoted 4 lines

fs/nfs/nfs4filelayout.c +4 -0

+
+		/* only add iostats if nfs_client is different from MDS */
+		if (ds && ds->ds_clp && d->nfs_client != ds->ds_clp) {
+			struct rpc_clnt *clnt = ds->ds_clp->cl_rpcclient;

Hmmm.... Shouldn't this rather be the NFS_CLIENT(inode)? Also, why not do this in the read/write completion code? Why walk the deviceid list at all?

Quoted 137 lines

fs/nfs/nfs4filelayout.c +10 -0

+			rpc_add_iostats(total, clnt->cl_metrics,
+					min(maxproc, clnt->cl_maxproc));
+			num_ds++;
+		}
+	}
+
+	return num_ds;
+}
+
 static struct pnfs_layoutdriver_type filelayout_type = {
 	.id			= LAYOUT_NFSV4_1_FILES,
 	.name			= "LAYOUT_NFSV4_1_FILES",

@@ -932,6 +961,7 @@ static struct pnfs_layoutdriver_type filelayout_type = {
 	.read_pagelist		= filelayout_read_pagelist,
 	.write_pagelist		= filelayout_write_pagelist,
 	.free_deviceid_node	= filelayout_free_deveiceid_node,
+	.ds_rpc_add_iostats	= filelayout_rpc_add_iostats,
 };
 
 static int __init nfs4filelayout_init(void)

fs/nfs/pnfs.h +23 -0

index 53d593a..d1b82eb 100644
@@ -72,6 +72,7 @@ enum layoutdriver_policy_flags {
 };
 
 struct nfs4_deviceid_node;
+struct rpc_iostats;
 
 /* Per-layout driver specific registration structure */
 struct pnfs_layoutdriver_type {

@@ -119,6 +120,9 @@ struct pnfs_layoutdriver_type {
 	void (*encode_layoutcommit) (struct pnfs_layout_hdr *layoutid,
 				     struct xdr_stream *xdr,
 				     const struct nfs4_layoutcommit_args *args);
+
+	unsigned int (*ds_rpc_add_iostats) (struct rpc_iostats *, unsigned int,
+					    struct nfs4_deviceid_node *);
 };
 
 struct pnfs_layout_hdr {

@@ -239,6 +243,10 @@ void nfs4_init_deviceid_node(struct nfs4_deviceid_node *,
 struct nfs4_deviceid_node *nfs4_insert_deviceid_node(struct nfs4_deviceid_node *);
 bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *);
 void nfs4_deviceid_purge_client(const struct nfs_client *);
+void nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				     const struct nfs_client *clp,
+				     const struct pnfs_layoutdriver_type *ld);
+
 
 static inline int lo_fail_bit(u32 iomode)
 {

@@ -328,6 +336,15 @@ static inline int pnfs_return_layout(struct inode *ino)
 	return 0;
 }
 
+static inline int
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	if (!pnfs_enabled_sb(nfss))
+		return 0;
+	nfs4_deviceid_rpc_print_iostats(m, nfss->nfs_client,
+					nfss->pnfs_curr_ld);
+	return 1;
+}
 #else  /* CONFIG_NFS_V4_1 */
 
 static inline void pnfs_destroy_all_layouts(struct nfs_client *clp)

@@ -429,6 +446,12 @@ static inline int pnfs_layoutcommit_inode(struct inode *inode, bool sync)
 static inline void nfs4_deviceid_purge_client(struct nfs_client *ncl)
 {
 }
+
+static inline void
+pnfs_rpc_print_iostats(struct seq_file *m, struct nfs_server *nfss)
+{
+	return 0;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 #endif /* FS_NFS_PNFS_H */

fs/nfs/pnfs_dev.c +39 -0

index 4f359d2..aa5d102 100644
@@ -29,6 +29,9 @@
 */
 
 #include <linux/export.h>
+
+#include <linux/sunrpc/metrics.h>
+
 #include "pnfs.h"
 
 #define NFSDBG_FACILITY		NFSDBG_PNFS

@@ -115,6 +118,48 @@ nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *ld,
 }
 EXPORT_SYMBOL_GPL(nfs4_find_get_deviceid);
 
+
+void
+nfs4_deviceid_rpc_print_iostats(struct seq_file *seq,
+				const struct nfs_client *clp,
+				const struct pnfs_layoutdriver_type *ld)
+{
+	struct nfs4_deviceid_node *d;
+	struct hlist_node *n;
+	struct rpc_iostats *totals;
+	char msgbuf[64];
+	unsigned int num_ds = 0;
+	unsigned int maxproc;
+	long h;
+
+	if (!ld->ds_rpc_add_iostats)
+		return;
+	totals = rpc_alloc_iostats(clp->cl_rpcclient);
+	if (!totals)
+		return;
+	maxproc = clp->cl_rpcclient->cl_maxproc;
+	rpc_add_iostats(totals, clp->cl_rpcclient->cl_metrics, maxproc);
+
+	rcu_read_lock();
+	for (h = 0; h < NFS4_DEVICE_ID_HASH_SIZE; h++) {
+		hlist_for_each_entry_rcu(d, n, &nfs4_deviceid_cache[h], node) {
+			if (d->ld == ld && d->nfs_client == clp) {
+				if (atomic_read(&d->ref))
+					num_ds += ld->ds_rpc_add_iostats(totals,
+							maxproc, d);
+			}
+		}
+	}
+	rcu_read_unlock();
+
+	snprintf(msgbuf, sizeof(msgbuf),
+		 " for the metadata server and %u data server%s", num_ds,

fs/nfs/pnfs_dev.c nfs4_find_get_deviceid(const struct pnfs_layoutdriver_type *ld, · line 161

Quoted 5 lines

fs/nfs/pnfs_dev.c +5 -0

+		 (num_ds != 1) ? "s" : "");
+	rpc_print_iostats(seq, clp->cl_rpcclient, totals, msgbuf);
+	rpc_free_iostats(totals);
+}
+EXPORT_SYMBOL_GPL(nfs4_deviceid_rpc_print_iostats);

Is this needed? AFAICS it is used in fs/nfs/super.c, which is in the same module (i.e. nfs.ko).

Quoted 116 lines

fs/nfs/pnfs_dev.c +0 -0

 /*
 * Remove a deviceid from cache
 *

fs/nfs/super.c +2 -1

index d18a90b..76d0850 100644
@@ -870,7 +870,8 @@ static int nfs_show_stats(struct seq_file *m, struct dentry *root)
 #endif
 	seq_printf(m, "\n");
 
-	rpc_print_iostats(m, nfss->client);
+	if (!pnfs_rpc_print_iostats(m, nfss))
+		rpc_print_iostats(m, nfss->client, NULL, NULL);
 
 	return 0;
 }

include/linux/sunrpc/metrics.h +11 -2

index b6edbc0..e6e04e4 100644
@@ -75,14 +75,23 @@ struct rpc_clnt;
 
 struct rpc_iostats *	rpc_alloc_iostats(struct rpc_clnt *);
 void			rpc_count_iostats(struct rpc_task *);
-void			rpc_print_iostats(struct seq_file *, struct rpc_clnt *);
+void			rpc_print_iostats(struct seq_file *,
+					  struct rpc_clnt *,
+					  struct rpc_iostats *,
+					  const char *);
+void			rpc_add_iostats(struct rpc_iostats *,
+					struct rpc_iostats *, unsigned int);
 void			rpc_free_iostats(struct rpc_iostats *);
 
 #else  /*  CONFIG_PROC_FS  */
 
 static inline struct rpc_iostats *rpc_alloc_iostats(struct rpc_clnt *clnt) { return NULL; }
 static inline void rpc_count_iostats(struct rpc_task *task) {}
-static inline void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt) {}
+static inline void rpc_print_iostats(struct seq_file *seq,
+				     struct rpc_iostats *stat) {}
+static inline void rpc_add_iostats(struct rpc_iostats *total,
+				   struct rpc_iostats *new,
+				   unsigned int maxproc) {}
 static inline void rpc_free_iostats(struct rpc_iostats *stats) {}
 
 #endif  /*  CONFIG_PROC_FS  */

net/sunrpc/stats.c +36 -2

index 3c4f688..342df57 100644
@@ -176,12 +176,16 @@ static void _print_name(struct seq_file *seq, unsigned int op,
 		seq_printf(seq, "\t%12u: ", op);
 }
 
-void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
+void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt,
+		       struct rpc_iostats *altstats,
+		       const char *altstats_msg)
 {
 	struct rpc_iostats *stats = clnt->cl_metrics;
 	struct rpc_xprt *xprt = clnt->cl_xprt;
 	unsigned int op, maxproc = clnt->cl_maxproc;
 
+	if (altstats)
+		stats = altstats;
 	if (!stats)
 		return;
 

@@ -192,7 +196,8 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 	if (xprt)
 		xprt->ops->print_stats(xprt, seq);
 
-	seq_printf(seq, "\tper-op statistics\n");
+	seq_printf(seq, "\tper-op statistics%s\n",
+		   (altstats && altstats_msg) ? altstats_msg : "");
 	for (op = 0; op < maxproc; op++) {
 		struct rpc_iostats *metrics = &stats[op];
 		_print_name(seq, op, clnt->cl_procinfo);

@@ -209,6 +214,35 @@ void rpc_print_iostats(struct seq_file *seq, struct rpc_clnt *clnt)
 }
 EXPORT_SYMBOL_GPL(rpc_print_iostats);
 
+void
+rpc_add_iostats(struct rpc_iostats *totals, struct rpc_iostats *new,
+		unsigned int maxproc)
+{
+	unsigned int op;
+
+	for (op = 0; op < maxproc; op++) {
+		struct rpc_iostats *op_totals = &totals[op],
+				   *op_new = &new[op];
+
+		op_totals->om_ops += op_new->om_ops;
+		op_totals->om_ntrans += op_new->om_ntrans;
+		op_totals->om_timeouts += op_new->om_timeouts;
+
+		op_totals->om_bytes_sent += op_new->om_bytes_sent;
+		op_totals->om_bytes_recv += op_new->om_bytes_recv;
+
+		op_totals->om_queue = ktime_add(op_totals->om_queue,
+					       op_new->om_queue);
+
+		op_totals->om_rtt = ktime_add(op_totals->om_rtt,
+					      op_new->om_rtt);
+
+		op_totals->om_execute = ktime_add(op_totals->om_execute,
+						 op_new->om_execute);
+	}
+}
+EXPORT_SYMBOL_GPL(rpc_add_iostats);
+
 /*
 * Register/unregister RPC proc files
 */

Signature · 6 lines

Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

Quoted 77 lines · Dros

Quoted 62 lines · Trond

Quoted 61 lines · Weston Andros Adamson

Include RPC statistics from all data servers in /proc/self/mountstats for pNFS filelayout mounts.

The additional info printed on the "per-op statistics" line does not break current mountstats(8) from nfs-utils.

Signed-off-by Weston Andros Adamson <[email protected]>

---

This is clearly a better approach than I was taking trying to keep DS rpc stats separate in /proc/self/mountstats.

FYI, there are no other callers of rpc_print_iostats().

The new output of /proc/self/mountstats works fine with current mountstats userland tool. The additional data on the per-op statistics line doesn't make it into the output, but I left it in /proc/self/mountstats because it doesn't break anything and can be included in newer versions of mountstats.

I plan on adding the per-DS stats (rpc and others) somewhere in /proc/fs/nfsfs/ soon.

fs/nfs/nfs4filelayout.c        |   30 ++++++++++++++++++++++++++
fs/nfs/pnfs.h                  |   23 ++++++++++++++++++++
fs/nfs/pnfs_dev.c              |   45 ++++++++++++++++++++++++++++++++++++++++
fs/nfs/super.c                 |    3 +-
include/linux/sunrpc/metrics.h |   13 +++++++++-
net/sunrpc/stats.c             |   38 ++++++++++++++++++++++++++++++++-
6 files changed, 147 insertions(+), 5 deletions(-)

fs/nfs/nfs4filelayout.c +16 -0

index 79be7ac..51c71e5 100644
@@ -33,6 +33,8 @@
 #include <linux/nfs_page.h>
 #include <linux/module.h>
 
+#include <linux/sunrpc/metrics.h>
+
 #include "internal.h"
 #include "nfs4filelayout.h"
 

@@ -918,6 +920,33 @@ filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d)
 	nfs4_fl_free_deviceid(container_of(d, struct nfs4_file_layout_dsaddr, id_node));
 }
 
+static unsigned int
+filelayout_rpc_add_iostats(struct rpc_iostats *total,
+			   unsigned int maxproc,
+			   struct nfs4_deviceid_node *d)
+{
+	struct nfs4_file_layout_dsaddr *dsaddr;
+	struct nfs4_pnfs_ds *ds;
+	unsigned int num_ds = 0;
+	u32 i;
+
+	dsaddr = container_of(d, struct nfs4_file_layout_dsaddr, id_node);
+
+	for (i = 0; i < dsaddr->ds_num; i++) {
+		ds = dsaddr->ds_list[i];

Quoted 8 lines

fs/nfs/nfs4filelayout.c filelayout_free_deveiceid_node(struct nfs4_deviceid_node *d) · line 940

Quoted 4 lines

fs/nfs/nfs4filelayout.c +4 -0

+
+		/* only add iostats if nfs_client is different from MDS */
+		if (ds && ds->ds_clp && d->nfs_client != ds->ds_clp) {
+			struct rpc_clnt *clnt = ds->ds_clp->cl_rpcclient;

Hmmm.... Shouldn't this rather be the NFS_CLIENT(inode)? Also, why not do this in the read/write completion code? Why walk the deviceid list at all?

I'm not really following you. Note that these are rpc_iostats not nfs_iostats.
nfs_iostats have been properly counting operations to DSs for some time now.
rpc_iostats count operations, rtt, exectime, etc per rpc procedure (or in the case of NFSv4.X there is a hack to use operations within compounds).

Quoted 1 line

We have to walk the device list to get access to the underlying DS's nfs_client->rpc_client -- I can't just put this in the read/write completion because, unlike nfs_iostats, rpc_iostats count all operations and not just read/write events/bytes (not to mention that the accumulation is done at the rpc layer).

In doing this, you are assuming that DSes are not shared among multiple filesystems or MDSes (i.e. that each DS is used by a single struct nfs_server). For something like a NetApp filer in c-mode, that is clearly not a valid assumption.

Quoted 2 lines

I do have an implementation that doesn't need to walk the deviceid list by allowing a shared rpc_iostats struct between multiple rpc_clients (in addition to the current rpc_iostats structure), but that required adding locking and reference counting -- all for printing stats (obviously not what we want).

This might be more in line with what we want. Note that it should be easy to clone an rpc_client and then replace its rpc_iostats struct. I don't think that needs any extra locking. We're already ignoring locking here between different rpc_tasks, so throwing in different rpc_clients to the mix will make no difference.

Quoted 11 lines · Dros

Quoted 8 lines

Quoted 2 lines

I do have an implementation that doesn't need to walk the deviceid list by allowing a shared rpc_iostats struct between multiple rpc_clients (in addition to the current rpc_iostats structure), but that required adding locking and reference counting -- all for printing stats (obviously not what we want).

This might be more in line with what we want. Note that it should be easy to clone an rpc_client and then replace its rpc_iostats struct. I don't think that needs any extra locking. We're already ignoring locking here between different rpc_tasks, so throwing in different rpc_clients to the mix will make no difference.

Yeah, that's easy enough and i guess we could ignore locking, but we are still left with the same problem: how is this supposed to account for different mount points using the same nfs_client? nfs_client only has one rpc_client member. I doubt we want to make a hash lookup on nfs_server to get the right rpc_client (which could all use the same underlying xprt).

Quoted 1 line

Maybe it's time to move these stats into fs/nfs/ where they really belong? We could get rid of the hack that overloads procnum with opnum from inside the compound for v4+ and finally only show a specific mount's RPC stats.

Actually, how about just adding a callback into struct rpc_call_ops that, if it is defined, would be called instead of rpc_count_iostats(). If you then modify rpc_count_iostats() to take the 'stats' variable as a parameter, you can simply have the new callback call rpc_count_iostats with the right arguments.

Signature · 7 lines

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com

Quoted 29 lines · Dros

Quoted 12 lines · Trond

Quoted 11 lines · Dros

Quoted 8 lines

Quoted 2 lines

I do have an implementation that doesn't need to walk the deviceid list by allowing a shared rpc_iostats struct between multiple rpc_clients (in addition to the current rpc_iostats structure), but that required adding locking and reference counting -- all for printing stats (obviously not what we want).

This might be more in line with what we want. Note that it should be easy to clone an rpc_client and then replace its rpc_iostats struct. I don't think that needs any extra locking. We're already ignoring locking here between different rpc_tasks, so throwing in different rpc_clients to the mix will make no difference.

Yeah, that's easy enough and i guess we could ignore locking, but we are still left with the same problem: how is this supposed to account for different mount points using the same nfs_client? nfs_client only has one rpc_client member. I doubt we want to make a hash lookup on nfs_server to get the right rpc_client (which could all use the same underlying xprt).

Quoted 7 lines

Quoted 1 line

Maybe it's time to move these stats into fs/nfs/ where they really belong? We could get rid of the hack that overloads procnum with opnum from inside the compound for v4+ and finally only show a specific mount's RPC stats.

Actually, how about just adding a callback into struct rpc_call_ops that, if it is defined, would be called instead of rpc_count_iostats(). If you then modify rpc_count_iostats() to take the 'stats' variable as a parameter, you can simply have the new callback call rpc_count_iostats with the right arguments.

Yeah, that could work! On my walk home from the cafe (they always seem to help) I came up with a slightly different strategy:

- remove the cl_metrics (struct rpc_iostats) member from rpc_clnt
- add an *optional* rpc_iostats pointer to rpc_task (ignored if NULL)
- allocate one rpc_iostats structure (really array of structs, but you know what I mean) per nfs_server structure
- when setting up rpc_tasks in nfs-land, pass the correct rpc_iostats

Quoted 3 lines

with this strategy we don't accumulate stats when they aren't ever used (like an rpc_client used for mount protocol). again, only NFS uses the rpc stats interface.

I kind of like this better than a callback, but either way is fine with me. Which way would you prefer?

I'd prefer not to have to grow the size of the rpc_task unless we really need to. That's why I suggested the callback instead.