[PATCH 0/3] nfsstat: Fixing and improving the new --sleep and --list options

[PATCH 0/3] nfsstat: Fixing and improving the new --sleep and --list options

[Steve Dickson]

Using the new --sleep and --list options to nfsstats,
I found a server side bug and an annoyance when particular
protocols where specified. 

When the --sleep options as used with a particular protocol
option (i.e. -2, -3 or -4) and there was no traffic, blank
lines were being echoed to the screen causing useful data
to be scrolled of the screen.

Also with the --sleep option, the server stats were not
being updated which meant the same values would be echoed
again and again when there was not new server traffic.

Finally when using the --sleep and/or --list options, zero 
stats will not be displayed. I found it much more useful to
show what happen instead of showing what did not happen. 
Plus not showing zero stats makes the --list much easier to 
read and digest as well as keeping useful data from scrolling
off the screen.

The server side bug was the fact --list didn't on server
stats. The client stats were being check when the server
stats should have been.

steved.

[PATCH 1/3] nfsstat: The --list option does not work on server stats.

[Steve Dickson]

The print_stats_list() routine was using the client's
stats to decide whether to display any stats. This did
not work when there was only server stats.
    
This patch breaks up print_stats_list into two different
routines allowing both server and clients stats to be
listed.
    
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
index 5adb571..7e9f327 100644
--- a/utils/nfsstat/nfsstat.c
+++ b/utils/nfsstat/nfsstat.c
@@ -170,7 +170,7 @@ DECLARE_CLT(cltinfo, _old);
 static void		print_all_stats(int, int, int);
 static void		print_server_stats(int, int);
 static void		print_client_stats(int, int);
-static void		print_stats_list(int);
+static void		print_stats_list(int, int, int);
 static void		print_numbers(const char *, unsigned int *,
 					unsigned int);
 static void		print_callstats(const char *, const char **,
@@ -436,7 +436,7 @@ main(int argc, char **argv)
 				diff_stats(clientinfo_tmp, clientinfo, 0);
 			}
 			if (opt_list) {
-				print_stats_list(opt_prt);
+				print_stats_list(opt_srv, opt_clt, opt_prt);
 			} else {
 				print_all_stats(opt_srv, opt_clt, opt_prt);
 			}
@@ -451,7 +451,7 @@ main(int argc, char **argv)
 		}	
 	} else {
 		if (opt_list) {
-			print_stats_list(opt_prt);
+			print_stats_list(opt_srv, opt_clt, opt_prt);
 		} else {
 			print_all_stats(opt_srv, opt_clt, opt_prt);
 		}
@@ -606,7 +606,7 @@ print_client_stats(int opt_clt, int opt_prt)
 			if (opt_sleep && !has_stats(cltproc3info)) {
 				;
 			} else {
-				print_callstats( LABEL_cltproc3,
+				print_callstats(LABEL_cltproc3,
 					nfsv3name, cltproc3info + 1, 
 					sizeof(nfsv3name)/sizeof(char *));
 			}
@@ -625,38 +625,89 @@ print_client_stats(int opt_clt, int opt_prt)
 }
 
 static void
-print_stats_list(int opt_prt) 
+print_clnt_list(int opt_prt) 
 {
 	if (opt_prt & PRNT_CALLS) {
-		if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc2info))) {
-			print_callstats_list(
-			"nfs v2 server",
-			nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *));
-			print_callstats_list(
-			"nfs v2 client",
-			nfsv2name, cltproc2info + 1,  sizeof(nfsv2name)/sizeof(char *));
+		if ((opt_prt & PRNT_V2) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc2info))) {
+			if (opt_sleep && !has_stats(cltproc2info)) {
+				;
+			} else {
+				print_callstats_list("nfs v2 client",
+					nfsv2name, cltproc2info + 1,  
+					sizeof(nfsv2name)/sizeof(char *));
+			}
+		}
+		if ((opt_prt & PRNT_V3) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
+			if (opt_sleep && !has_stats(cltproc3info)) {
+				;
+			} else { 
+				print_callstats_list("nfs v3 client",
+					nfsv3name, cltproc3info + 1, 
+					sizeof(nfsv3name)/sizeof(char *));
+			}
+		}
+		if ((opt_prt & PRNT_V4) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
+			if (opt_sleep && !has_stats(cltproc4info)) {
+				;
+			} else {
+				print_callstats_list("nfs v4 ops",
+					nfssrvproc4opname, srvproc4opsinfo + 1, 
+					sizeof(nfssrvproc4opname)/sizeof(char *));
+				print_callstats_list("nfs v4 client",
+					nfscltproc4name, cltproc4info + 1,  
+					sizeof(nfscltproc4name)/sizeof(char *));
+			}
+		}
+	}
+}
+static void
+print_serv_list(int opt_prt) 
+{
+	if (opt_prt & PRNT_CALLS) {
+		if ((opt_prt & PRNT_V2) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc2info))) {
+			if (opt_sleep && !has_stats(srvproc2info)) {
+				;
+			} else {
+				print_callstats_list("nfs v2 server",
+					nfsv2name, srvproc2info + 1, 
+					sizeof(nfsv2name)/sizeof(char *));
+			}
 		}
-		if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
-			print_callstats_list(
-			"nfs v3 server",
-			nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
-			print_callstats_list(
-			"nfs v3 client",
-			nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
+		if ((opt_prt & PRNT_V3) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc3info))) {
+			if (opt_sleep && !has_stats(srvproc3info)) {
+				;
+			} else {
+				print_callstats_list("nfs v3 server",
+					nfsv3name, srvproc3info + 1, 
+					sizeof(nfsv3name)/sizeof(char *));
+			}
 		}
-		if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
-			print_callstats_list(
-			"nfs v4 server",
-			nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *));
-			print_callstats_list(
-			"nfs v4 ops",
-			nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *));
-			print_callstats_list(
-			"nfs v4 client",
-			nfscltproc4name, cltproc4info + 1,  sizeof(nfscltproc4name)/sizeof(char *));
+		if ((opt_prt & PRNT_V4) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc4opsinfo))) {
+			if (opt_sleep && !has_stats(srvproc4info)) {
+				;
+			} else {
+				print_callstats_list("nfs v4 ops",
+					nfssrvproc4opname, srvproc4opsinfo + 1, 
+					sizeof(nfssrvproc4opname)/sizeof(char *));
+			}
 		}
 	}
 }
+static void
+print_stats_list(int opt_srv, int opt_clt, int opt_prt) 
+{
+	if (opt_srv)
+		print_serv_list(opt_prt);
+
+	if (opt_clt)
+		print_clnt_list(opt_prt);
+}
 
 static statinfo *
 get_stat_info(const char *sp, struct statinfo *statp)

[PATCH 2/3] nfsstat: -Z showing incorrect server stats

[Steve Dickson]

The server stats were not being updated with the
-Z options causing the stats to be incorrect.
    
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
index 6d81f66..5adb571 100644
--- a/utils/nfsstat/nfsstat.c
+++ b/utils/nfsstat/nfsstat.c
@@ -442,7 +442,11 @@ main(int argc, char **argv)
 			}
 			fflush(stdout);
 
-			update_old_counters(clientinfo_tmp, clientinfo);
+			if (opt_srv)
+				update_old_counters(serverinfo_tmp, serverinfo);
+			if (opt_clt)
+				update_old_counters(clientinfo_tmp, clientinfo);
+
 			sleep(sleep_time);
 		}	
 	} else {

[PATCH 3/3] nfsstat: Eliminate the displaying zero stats

[Steve Dickson]

Eliminate the displaying zero stats when the explicit protocol
is specified (-2, -3, -4) the -Z and or --list options.
    
When a particular protocol is specified and either
the -Z or --list options are used, zeros or blank lines
are echoed to the screen when there is not any NFS traffic.
This cause any useful data to be scroll off the screen.
    
With this patch only non-zero stats will be shown, which
makes the output of these options more condensed and
in turn more useful.
    
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
index d5bdad7..6d81f66 100644
--- a/utils/nfsstat/nfsstat.c
+++ b/utils/nfsstat/nfsstat.c
@@ -466,79 +466,96 @@ print_all_stats (int opt_srv, int opt_clt, int opt_prt)
 static void 
 print_server_stats(int opt_srv, int opt_prt) 
 {
-	if (opt_srv) {
-		if (opt_prt & PRNT_NET) {
-			print_numbers(
-			LABEL_srvnet
-			"packets    udp        tcp        tcpconn\n",
-			srvnetinfo, 4
-			);
+	if (!opt_srv)
+		return;
+
+	if (opt_prt & PRNT_NET) {
+		if (opt_sleep && !has_rpcstats(srvnetinfo, 4)) {
+		} else {
+			print_numbers( LABEL_srvnet
+				"packets    udp        tcp        tcpconn\n",
+			srvnetinfo, 4);
 			printf("\n");
 		}
-		if (opt_prt & PRNT_RPC) {
-			if (!opt_sleep && !has_rpcstats(srvrpcinfo, 5)) {
-				print_numbers(
-				LABEL_srvrpc
+	}
+	if (opt_prt & PRNT_RPC) {
+		if (opt_sleep && !has_rpcstats(srvrpcinfo, 5)) {
+			;
+		} else {
+			print_numbers(LABEL_srvrpc
 				"calls      badcalls   badauth    badclnt    xdrcall\n",
-				srvrpcinfo, 5
-				);
-				printf("\n");
-			}
+				srvrpcinfo, 5);
+			printf("\n");
 		}
-		if (opt_prt & PRNT_RC) {
-			print_numbers(
-			LABEL_srvrc
-			"hits       misses     nocache\n",
-			srvrcinfo, 3
-			);
+	}
+	if (opt_prt & PRNT_RC) {
+		if (opt_sleep && !has_rpcstats(srvrcinfo, 3)) {
+			;
+		} else {
+			print_numbers(LABEL_srvrc
+				"hits       misses     nocache\n",
+				srvrcinfo, 3);
 			printf("\n");
 		}
+	}
 
-		/*
-		 * 2.2 puts all fh-related info after the 'rc' header
-		 * 2.4 puts all fh-related info after the 'fh' header, but relocates
-		 *     'stale' to the start and swaps dir and nondir :-(  
-		 *     We preseve the 2.2 order
-		 */
-		if (opt_prt & PRNT_FH) {
-			if (get_stat_info("fh", srvinfo)) {	/* >= 2.4 */
-				int t = srvfhinfo[3];
-				srvfhinfo[3]=srvfhinfo[4];
-				srvfhinfo[4]=t;
-				
-				srvfhinfo[5]=srvfhinfo[0]; /* relocate 'stale' */
-				
-				print_numbers(
-					LABEL_srvfh
-					"lookup     anon       ncachedir  ncachedir  stale\n",
-					srvfhinfo + 1, 5);
-			} else					/* < 2.4 */
-				print_numbers(
-					LABEL_srvfh
-					"lookup     anon       ncachedir  ncachedir  stale\n",
-					srvrcinfo + 3, 5);
-			printf("\n");
+	/*
+	 * 2.2 puts all fh-related info after the 'rc' header
+	 * 2.4 puts all fh-related info after the 'fh' header, but relocates
+	 *     'stale' to the start and swaps dir and nondir :-(  
+	 *     We preseve the 2.2 order
+	 */
+	if (opt_prt & PRNT_FH) {
+		if (get_stat_info("fh", srvinfo)) {	/* >= 2.4 */
+			int t = srvfhinfo[3];
+			srvfhinfo[3]=srvfhinfo[4];
+			srvfhinfo[4]=t;
+			
+			srvfhinfo[5]=srvfhinfo[0]; /* relocate 'stale' */
+			
+			print_numbers(
+				LABEL_srvfh
+				"lookup     anon       ncachedir  ncachedir  stale\n",
+				srvfhinfo + 1, 5);
+		} else					/* < 2.4 */
+			print_numbers(
+				LABEL_srvfh
+				"lookup     anon       ncachedir  ncachedir  stale\n",
+				srvrcinfo + 3, 5);
+		printf("\n");
+	}
+	if (opt_prt & PRNT_CALLS) {
+		if ((opt_prt & PRNT_V2) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc2info))) {
+			if (opt_sleep && !has_stats(srvproc2info)) {
+				;
+			} else {
+				print_callstats(LABEL_srvproc2,
+					nfsv2name, srvproc2info + 1, 
+					sizeof(nfsv2name)/sizeof(char *));
+			}
+		}
+		if ((opt_prt & PRNT_V3) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc3info))) {
+			if (opt_sleep && !has_stats(srvproc3info)) {
+				;
+			} else {
+				print_callstats(LABEL_srvproc3,
+					nfsv3name, srvproc3info + 1, 
+					sizeof(nfsv3name)/sizeof(char *));
+			}
 		}
-		if (opt_prt & PRNT_CALLS) {
-			if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc2info)))
-				print_callstats(
-				LABEL_srvproc2,
-				nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *)
-				);
-			if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc3info)))
-				print_callstats(
-				LABEL_srvproc3,
-				nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *)
-				);
-			if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc4info))) {
-				print_callstats(
-				LABEL_srvproc4,
-				nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *)
-				);
-				print_callstats(
-				LABEL_srvproc4ops,
-				nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *)
-				);
+		if ((opt_prt & PRNT_V4) || 
+				((opt_prt & PRNT_AUTO) && has_stats(srvproc4info))) {
+			if (opt_sleep && !has_stats(srvproc4info)) {
+				;
+			} else {
+				print_callstats( LABEL_srvproc4,
+					nfssrvproc4name, srvproc4info + 1, 
+					sizeof(nfssrvproc4name)/sizeof(char *));
+				print_callstats(LABEL_srvproc4ops,
+					nfssrvproc4opname, srvproc4opsinfo + 1, 
+					sizeof(nfssrvproc4opname)/sizeof(char *));
 			}
 		}
 	}
@@ -546,41 +563,59 @@ print_server_stats(int opt_srv, int opt_prt)
 static void
 print_client_stats(int opt_clt, int opt_prt) 
 {
-	if (opt_clt) {
-		if (opt_prt & PRNT_NET) {
-			print_numbers(
-			LABEL_cltnet
-			"packets    udp        tcp        tcpconn\n",
-			cltnetinfo, 4
-			);
+	if (!opt_clt)
+		return;
+
+	if (opt_prt & PRNT_NET) {
+		if (opt_sleep && !has_rpcstats(cltnetinfo, 4)) {
+			;
+		} else { 
+			print_numbers(LABEL_cltnet
+				"packets    udp        tcp        tcpconn\n",
+				cltnetinfo, 4);
 			printf("\n");
 		}
-		if (opt_prt & PRNT_RPC) {
-			if (!opt_sleep && !has_rpcstats(cltrpcinfo, 3)) {
-				print_numbers(
-				LABEL_cltrpc
+	}
+	if (opt_prt & PRNT_RPC) {
+		if (opt_sleep && !has_rpcstats(cltrpcinfo, 3)) {
+			;
+		} else {
+			print_numbers(LABEL_cltrpc
 				"calls      retrans    authrefrsh\n",
-				cltrpcinfo, 3
-				);
-				printf("\n");
+				cltrpcinfo, 3);
+			printf("\n");
+		}
+	}
+	if (opt_prt & PRNT_CALLS) {
+		if ((opt_prt & PRNT_V2) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc2info))) {
+			if (opt_sleep && !has_stats(cltproc2info)) {
+				;
+			} else {
+				print_callstats(LABEL_cltproc2,
+					nfsv2name, cltproc2info + 1,  
+					sizeof(nfsv2name)/sizeof(char *));
+			}
+		}
+		if ((opt_prt & PRNT_V3) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
+			if (opt_sleep && !has_stats(cltproc3info)) {
+				;
+			} else {
+				print_callstats( LABEL_cltproc3,
+					nfsv3name, cltproc3info + 1, 
+					sizeof(nfsv3name)/sizeof(char *));
 			}
 		}
-		if (opt_prt & PRNT_CALLS) {
-			if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc2info)))
-				print_callstats(
-				LABEL_cltproc2,
-				nfsv2name, cltproc2info + 1,  sizeof(nfsv2name)/sizeof(char *)
-				);
-			if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info)))
-				print_callstats(
-				LABEL_cltproc3,
-				nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *)
-				);
-			if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info)))
-				print_callstats(
-				LABEL_cltproc4,
-				nfscltproc4name, cltproc4info + 1,  sizeof(nfscltproc4name)/sizeof(char *)
-				);
+		if ((opt_prt & PRNT_V4) || 
+				((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
+			if (opt_sleep && !has_stats(cltproc4info)) {
+				;
+			} else {
+				print_callstats(LABEL_cltproc4,
+					nfscltproc4name, cltproc4info + 1,  
+					sizeof(nfscltproc4name)/sizeof(char *));
+			}
 		}
 	}
 }
@@ -593,35 +628,28 @@ print_stats_list(int opt_prt)
 			print_callstats_list(
 			"nfs v2 server",
 			nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *));
-			printf("\n");
 			print_callstats_list(
 			"nfs v2 client",
 			nfsv2name, cltproc2info + 1,  sizeof(nfsv2name)/sizeof(char *));
-			printf("\n");
 		}
 		if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
 			print_callstats_list(
 			"nfs v3 server",
 			nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
-			printf("\n");
 			print_callstats_list(
 			"nfs v3 client",
 			nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
-			printf("\n");
 		}
 		if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
 			print_callstats_list(
 			"nfs v4 server",
 			nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *));
-			printf("\n");
 			print_callstats_list(
 			"nfs v4 ops",
 			nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *));
-			printf("\n");
 			print_callstats_list(
 			"nfs v4 client",
 			nfscltproc4name, cltproc4info + 1,  sizeof(nfscltproc4name)/sizeof(char *));
-			printf("\n");
 		}
 	}
 }
@@ -686,11 +714,15 @@ print_callstats_list(const char *hdr, const char **names,
 	for (i = 0, calltotal = 0; i < nr; i++) {
 		calltotal += callinfo[i];
 	}
+	if (!calltotal)
+		return;
 	printf("%13s %13s %8llu \n", hdr, "total:", calltotal);
 	printf("------------- ------------- --------\n");
 	for (i = 0; i < nr; i++) {
-			printf("%13s %12s: %8u \n", hdr, names[i], callinfo[i]);
+			if (callinfo[i])
+				printf("%13s %12s: %8u \n", hdr, names[i], callinfo[i]);
 	}
+	printf("\n");
 		
 }
 

Re: [PATCH 3/3] nfsstat: Eliminate the displaying zero stats

[J. Bruce Fields]

On Wed, Apr 08, 2009 at 12:05:08PM -0400, Steve Dickson wrote:
> Eliminate the displaying zero stats when the explicit protocol
> is specified (-2, -3, -4) the -Z and or --list options.
>     
> When a particular protocol is specified and either
> the -Z or --list options are used, zeros or blank lines
> are echoed to the screen when there is not any NFS traffic.
> This cause any useful data to be scroll off the screen.

Sound like good fixes.

>     
> With this patch only non-zero stats will be shown, which
> makes the output of these options more condensed and
> in turn more useful.

Couldn't this make the output harder to parse?  E.g. if you want to know
how many getattr's were sent each second, it might be simpler if you say
a 0 those times when there were no getattrs.

--b.

>     
> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
> index d5bdad7..6d81f66 100644
> --- a/utils/nfsstat/nfsstat.c
> +++ b/utils/nfsstat/nfsstat.c
> @@ -466,79 +466,96 @@ print_all_stats (int opt_srv, int opt_clt, int opt_prt)
>  static void 
>  print_server_stats(int opt_srv, int opt_prt) 
>  {
> -	if (opt_srv) {
> -		if (opt_prt & PRNT_NET) {
> -			print_numbers(
> -			LABEL_srvnet
> -			"packets    udp        tcp        tcpconn\n",
> -			srvnetinfo, 4
> -			);
> +	if (!opt_srv)
> +		return;
> +
> +	if (opt_prt & PRNT_NET) {
> +		if (opt_sleep && !has_rpcstats(srvnetinfo, 4)) {
> +		} else {
> +			print_numbers( LABEL_srvnet
> +				"packets    udp        tcp        tcpconn\n",
> +			srvnetinfo, 4);
>  			printf("\n");
>  		}
> -		if (opt_prt & PRNT_RPC) {
> -			if (!opt_sleep && !has_rpcstats(srvrpcinfo, 5)) {
> -				print_numbers(
> -				LABEL_srvrpc
> +	}
> +	if (opt_prt & PRNT_RPC) {
> +		if (opt_sleep && !has_rpcstats(srvrpcinfo, 5)) {
> +			;
> +		} else {
> +			print_numbers(LABEL_srvrpc
>  				"calls      badcalls   badauth    badclnt    xdrcall\n",
> -				srvrpcinfo, 5
> -				);
> -				printf("\n");
> -			}
> +				srvrpcinfo, 5);
> +			printf("\n");
>  		}
> -		if (opt_prt & PRNT_RC) {
> -			print_numbers(
> -			LABEL_srvrc
> -			"hits       misses     nocache\n",
> -			srvrcinfo, 3
> -			);
> +	}
> +	if (opt_prt & PRNT_RC) {
> +		if (opt_sleep && !has_rpcstats(srvrcinfo, 3)) {
> +			;
> +		} else {
> +			print_numbers(LABEL_srvrc
> +				"hits       misses     nocache\n",
> +				srvrcinfo, 3);
>  			printf("\n");
>  		}
> +	}
>  
> -		/*
> -		 * 2.2 puts all fh-related info after the 'rc' header
> -		 * 2.4 puts all fh-related info after the 'fh' header, but relocates
> -		 *     'stale' to the start and swaps dir and nondir :-(  
> -		 *     We preseve the 2.2 order
> -		 */
> -		if (opt_prt & PRNT_FH) {
> -			if (get_stat_info("fh", srvinfo)) {	/* >= 2.4 */
> -				int t = srvfhinfo[3];
> -				srvfhinfo[3]=srvfhinfo[4];
> -				srvfhinfo[4]=t;
> -				
> -				srvfhinfo[5]=srvfhinfo[0]; /* relocate 'stale' */
> -				
> -				print_numbers(
> -					LABEL_srvfh
> -					"lookup     anon       ncachedir  ncachedir  stale\n",
> -					srvfhinfo + 1, 5);
> -			} else					/* < 2.4 */
> -				print_numbers(
> -					LABEL_srvfh
> -					"lookup     anon       ncachedir  ncachedir  stale\n",
> -					srvrcinfo + 3, 5);
> -			printf("\n");
> +	/*
> +	 * 2.2 puts all fh-related info after the 'rc' header
> +	 * 2.4 puts all fh-related info after the 'fh' header, but relocates
> +	 *     'stale' to the start and swaps dir and nondir :-(  
> +	 *     We preseve the 2.2 order
> +	 */
> +	if (opt_prt & PRNT_FH) {
> +		if (get_stat_info("fh", srvinfo)) {	/* >= 2.4 */
> +			int t = srvfhinfo[3];
> +			srvfhinfo[3]=srvfhinfo[4];
> +			srvfhinfo[4]=t;
> +			
> +			srvfhinfo[5]=srvfhinfo[0]; /* relocate 'stale' */
> +			
> +			print_numbers(
> +				LABEL_srvfh
> +				"lookup     anon       ncachedir  ncachedir  stale\n",
> +				srvfhinfo + 1, 5);
> +		} else					/* < 2.4 */
> +			print_numbers(
> +				LABEL_srvfh
> +				"lookup     anon       ncachedir  ncachedir  stale\n",
> +				srvrcinfo + 3, 5);
> +		printf("\n");
> +	}
> +	if (opt_prt & PRNT_CALLS) {
> +		if ((opt_prt & PRNT_V2) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(srvproc2info))) {
> +			if (opt_sleep && !has_stats(srvproc2info)) {
> +				;
> +			} else {
> +				print_callstats(LABEL_srvproc2,
> +					nfsv2name, srvproc2info + 1, 
> +					sizeof(nfsv2name)/sizeof(char *));
> +			}
> +		}
> +		if ((opt_prt & PRNT_V3) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(srvproc3info))) {
> +			if (opt_sleep && !has_stats(srvproc3info)) {
> +				;
> +			} else {
> +				print_callstats(LABEL_srvproc3,
> +					nfsv3name, srvproc3info + 1, 
> +					sizeof(nfsv3name)/sizeof(char *));
> +			}
>  		}
> -		if (opt_prt & PRNT_CALLS) {
> -			if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc2info)))
> -				print_callstats(
> -				LABEL_srvproc2,
> -				nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *)
> -				);
> -			if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc3info)))
> -				print_callstats(
> -				LABEL_srvproc3,
> -				nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *)
> -				);
> -			if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(srvproc4info))) {
> -				print_callstats(
> -				LABEL_srvproc4,
> -				nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *)
> -				);
> -				print_callstats(
> -				LABEL_srvproc4ops,
> -				nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *)
> -				);
> +		if ((opt_prt & PRNT_V4) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(srvproc4info))) {
> +			if (opt_sleep && !has_stats(srvproc4info)) {
> +				;
> +			} else {
> +				print_callstats( LABEL_srvproc4,
> +					nfssrvproc4name, srvproc4info + 1, 
> +					sizeof(nfssrvproc4name)/sizeof(char *));
> +				print_callstats(LABEL_srvproc4ops,
> +					nfssrvproc4opname, srvproc4opsinfo + 1, 
> +					sizeof(nfssrvproc4opname)/sizeof(char *));
>  			}
>  		}
>  	}
> @@ -546,41 +563,59 @@ print_server_stats(int opt_srv, int opt_prt)
>  static void
>  print_client_stats(int opt_clt, int opt_prt) 
>  {
> -	if (opt_clt) {
> -		if (opt_prt & PRNT_NET) {
> -			print_numbers(
> -			LABEL_cltnet
> -			"packets    udp        tcp        tcpconn\n",
> -			cltnetinfo, 4
> -			);
> +	if (!opt_clt)
> +		return;
> +
> +	if (opt_prt & PRNT_NET) {
> +		if (opt_sleep && !has_rpcstats(cltnetinfo, 4)) {
> +			;
> +		} else { 
> +			print_numbers(LABEL_cltnet
> +				"packets    udp        tcp        tcpconn\n",
> +				cltnetinfo, 4);
>  			printf("\n");
>  		}
> -		if (opt_prt & PRNT_RPC) {
> -			if (!opt_sleep && !has_rpcstats(cltrpcinfo, 3)) {
> -				print_numbers(
> -				LABEL_cltrpc
> +	}
> +	if (opt_prt & PRNT_RPC) {
> +		if (opt_sleep && !has_rpcstats(cltrpcinfo, 3)) {
> +			;
> +		} else {
> +			print_numbers(LABEL_cltrpc
>  				"calls      retrans    authrefrsh\n",
> -				cltrpcinfo, 3
> -				);
> -				printf("\n");
> +				cltrpcinfo, 3);
> +			printf("\n");
> +		}
> +	}
> +	if (opt_prt & PRNT_CALLS) {
> +		if ((opt_prt & PRNT_V2) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(cltproc2info))) {
> +			if (opt_sleep && !has_stats(cltproc2info)) {
> +				;
> +			} else {
> +				print_callstats(LABEL_cltproc2,
> +					nfsv2name, cltproc2info + 1,  
> +					sizeof(nfsv2name)/sizeof(char *));
> +			}
> +		}
> +		if ((opt_prt & PRNT_V3) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
> +			if (opt_sleep && !has_stats(cltproc3info)) {
> +				;
> +			} else {
> +				print_callstats( LABEL_cltproc3,
> +					nfsv3name, cltproc3info + 1, 
> +					sizeof(nfsv3name)/sizeof(char *));
>  			}
>  		}
> -		if (opt_prt & PRNT_CALLS) {
> -			if ((opt_prt & PRNT_V2) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc2info)))
> -				print_callstats(
> -				LABEL_cltproc2,
> -				nfsv2name, cltproc2info + 1,  sizeof(nfsv2name)/sizeof(char *)
> -				);
> -			if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info)))
> -				print_callstats(
> -				LABEL_cltproc3,
> -				nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *)
> -				);
> -			if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info)))
> -				print_callstats(
> -				LABEL_cltproc4,
> -				nfscltproc4name, cltproc4info + 1,  sizeof(nfscltproc4name)/sizeof(char *)
> -				);
> +		if ((opt_prt & PRNT_V4) || 
> +				((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
> +			if (opt_sleep && !has_stats(cltproc4info)) {
> +				;
> +			} else {
> +				print_callstats(LABEL_cltproc4,
> +					nfscltproc4name, cltproc4info + 1,  
> +					sizeof(nfscltproc4name)/sizeof(char *));
> +			}
>  		}
>  	}
>  }
> @@ -593,35 +628,28 @@ print_stats_list(int opt_prt)
>  			print_callstats_list(
>  			"nfs v2 server",
>  			nfsv2name, srvproc2info + 1, sizeof(nfsv2name)/sizeof(char *));
> -			printf("\n");
>  			print_callstats_list(
>  			"nfs v2 client",
>  			nfsv2name, cltproc2info + 1,  sizeof(nfsv2name)/sizeof(char *));
> -			printf("\n");
>  		}
>  		if ((opt_prt & PRNT_V3) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc3info))) {
>  			print_callstats_list(
>  			"nfs v3 server",
>  			nfsv3name, srvproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
> -			printf("\n");
>  			print_callstats_list(
>  			"nfs v3 client",
>  			nfsv3name, cltproc3info + 1, sizeof(nfsv3name)/sizeof(char *));
> -			printf("\n");
>  		}
>  		if ((opt_prt & PRNT_V4) || ((opt_prt & PRNT_AUTO) && has_stats(cltproc4info))) {
>  			print_callstats_list(
>  			"nfs v4 server",
>  			nfssrvproc4name, srvproc4info + 1, sizeof(nfssrvproc4name)/sizeof(char *));
> -			printf("\n");
>  			print_callstats_list(
>  			"nfs v4 ops",
>  			nfssrvproc4opname, srvproc4opsinfo + 1, sizeof(nfssrvproc4opname)/sizeof(char *));
> -			printf("\n");
>  			print_callstats_list(
>  			"nfs v4 client",
>  			nfscltproc4name, cltproc4info + 1,  sizeof(nfscltproc4name)/sizeof(char *));
> -			printf("\n");
>  		}
>  	}
>  }
> @@ -686,11 +714,15 @@ print_callstats_list(const char *hdr, const char **names,
>  	for (i = 0, calltotal = 0; i < nr; i++) {
>  		calltotal += callinfo[i];
>  	}
> +	if (!calltotal)
> +		return;
>  	printf("%13s %13s %8llu \n", hdr, "total:", calltotal);
>  	printf("------------- ------------- --------\n");
>  	for (i = 0; i < nr; i++) {
> -			printf("%13s %12s: %8u \n", hdr, names[i], callinfo[i]);
> +			if (callinfo[i])
> +				printf("%13s %12s: %8u \n", hdr, names[i], callinfo[i]);
>  	}
> +	printf("\n");
>  		
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] nfsstat: Eliminate the displaying zero stats

[Steve Dickson]

Hey Bruce,

J. Bruce Fields wrote:
> On Wed, Apr 08, 2009 at 12:05:08PM -0400, Steve Dickson wrote:
>> Eliminate the displaying zero stats when the explicit protocol
>> is specified (-2, -3, -4) the -Z and or --list options.
>>     
>> When a particular protocol is specified and either
>> the -Z or --list options are used, zeros or blank lines
>> are echoed to the screen when there is not any NFS traffic.
>> This cause any useful data to be scroll off the screen.
> 
> Sound like good fixes.
> 
>>     
>> With this patch only non-zero stats will be shown, which
>> makes the output of these options more condensed and
>> in turn more useful.
> 
> Couldn't this make the output harder to parse?  E.g. if you want to know
> how many getattr's were sent each second, it might be simpler if you say
> a 0 those times when there were no getattrs.
> 
Maybe.. but if one was parsing the output and the parser didn't see a 
gettr stat, I'm assuming it would not increment the "gettr counter"
so it would be know there were zero gettars in that time period.. 

The main reason I did this was, when watching reads and writes, 90% of 
the output was zeros stats which made very difficult to see the non 
zero counts (i.e. read and write stats) since they generally got
scrolled of the screen... 

Also note, when the --list option is not used, zero stats are 
shown in the column output, but granted, the column output
is a bit harder to parse than the list output.

I guess I could change it so non zero stats are show only 
the --sleep option is *not* used....  

steved.

Re: [PATCH 3/3] nfsstat: Eliminate the displaying zero stats

[Kevin Constantine]

Steve Dickson wrote:
> Hey Bruce,
> 
> J. Bruce Fields wrote:
>> On Wed, Apr 08, 2009 at 12:05:08PM -0400, Steve Dickson wrote:
>>> Eliminate the displaying zero stats when the explicit protocol
>>> is specified (-2, -3, -4) the -Z and or --list options.
>>>     
>>> When a particular protocol is specified and either
>>> the -Z or --list options are used, zeros or blank lines
>>> are echoed to the screen when there is not any NFS traffic.
>>> This cause any useful data to be scroll off the screen.
>> Sound like good fixes.
>>
>>>     
>>> With this patch only non-zero stats will be shown, which
>>> makes the output of these options more condensed and
>>> in turn more useful.
>> Couldn't this make the output harder to parse?  E.g. if you want to know
>> how many getattr's were sent each second, it might be simpler if you say
>> a 0 those times when there were no getattrs.
>>
> Maybe.. but if one was parsing the output and the parser didn't see a 
> gettr stat, I'm assuming it would not increment the "gettr counter"
> so it would be know there were zero gettars in that time period.. 
> 
> The main reason I did this was, when watching reads and writes, 90% of 
> the output was zeros stats which made very difficult to see the non 
> zero counts (i.e. read and write stats) since they generally got
> scrolled of the screen... 
> 
> Also note, when the --list option is not used, zero stats are 
> shown in the column output, but granted, the column output
> is a bit harder to parse than the list output.
> 
> I guess I could change it so non zero stats are show only 
> the --sleep option is *not* used....  
> 

Dare I suggest another commandline argument?  I didn't really like the 
idea of leaving out the stats with 0's, but after trying it, the output 
does look a lot cleaner.  Perhaps giving users the choice would be the 
way to go.

-kevin

> steved.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
------------------------------------------------------------
Kevin Constantine

Systems Engineer		t: 818.460.8221
Walt Disney Animation Studios	e: kevin.constantine-P5ys19MLBK/QT0dZR+AlfA@public.gmane.org

Any sufficiently advanced technology is indistinguishable from magic.
     - Arthur C. Clarke

Re: [PATCH 3/3] nfsstat: Eliminate the displaying zero stats

[Steve Dickson]

Kevin Constantine wrote:
> Steve Dickson wrote:
>> Hey Bruce,
>>
>> J. Bruce Fields wrote:
>>> On Wed, Apr 08, 2009 at 12:05:08PM -0400, Steve Dickson wrote:
>>>> Eliminate the displaying zero stats when the explicit protocol
>>>> is specified (-2, -3, -4) the -Z and or --list options.
>>>>     When a particular protocol is specified and either
>>>> the -Z or --list options are used, zeros or blank lines
>>>> are echoed to the screen when there is not any NFS traffic.
>>>> This cause any useful data to be scroll off the screen.
>>> Sound like good fixes.
>>>
>>>>     With this patch only non-zero stats will be shown, which
>>>> makes the output of these options more condensed and
>>>> in turn more useful.
>>> Couldn't this make the output harder to parse?  E.g. if you want to know
>>> how many getattr's were sent each second, it might be simpler if you say
>>> a 0 those times when there were no getattrs.
>>>
>> Maybe.. but if one was parsing the output and the parser didn't see a
>> gettr stat, I'm assuming it would not increment the "gettr counter"
>> so it would be know there were zero gettars in that time period..
>> The main reason I did this was, when watching reads and writes, 90% of
>> the output was zeros stats which made very difficult to see the non
>> zero counts (i.e. read and write stats) since they generally got
>> scrolled of the screen...
>> Also note, when the --list option is not used, zero stats are shown in
>> the column output, but granted, the column output
>> is a bit harder to parse than the list output.
>>
>> I guess I could change it so non zero stats are show only the --sleep
>> option is *not* used.... 
> 
> Dare I suggest another commandline argument?  I didn't really like the
> idea of leaving out the stats with 0's, but after trying it, the output
> does look a lot cleaner.  Perhaps giving users the choice would be the
> way to go.
hmm.. I'm not sure yet another flag would be good...

I guess I like the idea of having the output show what happen 
instead of what has not happen... also remember, the non list output
will show the zeros... But I'm open for compromise... 

steved.