nfsstat printing overflow

PATCH Michael Griffith 3 participants 3 messages May 29, 2003 – Jun 2, 2003

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Michael Griffith

author

2003-05-29 23:27 UTC #0 lore raw

A small patch to handle when the RPC call counts are > 2^32/100.

Signature · 4 lines

Michael A. Griffith
<[email protected]>
408-907-2151
650-292-1516

Without this patch, nfsstat reports the wrong percentage for any RPC which has more than 2^32/100 calls. For example my machine running 1.0.1 reports:

l-sim-208-11:diag/testgen/batch_0/nv40_curie_ogtest/000001> nfsstat at -c
Client rpc stats:

calls      retrans    authrefrsh
61096094   181        0

Client nfs v2:
null getattr setattr root lookup readlink
0 0% 0 0% 0 0% 0 0% 0 0% 0 0%
read wrcache write create remove rename
0 0% 0 0% 0 0% 0 0% 0 0% 0 0%
link symlink mkdir rmdir readdir fsstat
0 0% 0 0% 0 0% 0 0% 0 0% 0 0%

Client nfs v3:

null       getattr    setattr    lookup     access     readlink
0       0% 2661431  4% 76995   0% 4938464  8% 3254610  5% 131     0%
read       write      create     mkdir      symlink    mknod

138409 0% 259382 0% 185867 0% 3570 0% 49 0% 0 0%
remove rmdir rename link readdir readdirplus
4035088 6% 318094 0% 157 0% 0 0% 1626490 2% 43597144 1%
fsstat fsinfo pathconf commit
18 0% 195 0% 0 0% 0 0%

Note that readdir plus should be 71%, not 1%.

nfsstat.c +6 -4

@@ -307,7 +307,8 @@
 print_callstats(const char *hdr, const char **names,
 				 unsigned int *info, unsigned int nr)
 {
-	unsigned int	total;
+	unsigned long long	total;
+        unsigned long long      pct;
 	int		i, j;
 
 	fputs(hdr, stdout);

@@ -319,9 +320,10 @@
 		for (j = 0; j < 6 && i + j < nr; j++)
 			printf("%-11s", names[i+j]);
 		printf("\n");
-		for (j = 0; j < 6 && i + j < nr; j++)
-			printf("%-6d %2d%% ",
-				info[i+j], 100 * info[i+j] / total);
+		for (j = 0; j < 6 && i + j < nr; j++) {
+                        pct = ((unsigned long long) info[i+j]*100)/total;
+                        printf("%-6d %2d%% ", info[i+j], pct);
+                }
 		printf("\n");
 	}
 	printf("\n");

Neil Brown

2003-05-30 05:43 UTC #1 lore raw

Quoted 2 lines · On Thursday May 29, [email protected] wrote:

A small patch to handle when the RPC call counts are > 2^32/100.

Thanks.
However:

Review

Quoted 6 lines

(quoted diff) +5 -0

+        unsigned long long      pct;
 
+		for (j = 0; j < 6 && i + j < nr; j++) {
+                        pct = ((unsigned long long) info[i+j]*100)/total;
+                        printf("%-6d %2d%% ", info[i+j], pct);
+                }

if pct is an "unsigned long long" we should use "%llu" to print it.

The following patch has been commited to the nfs-utils CVS and will be in the next release, which will probably be 1.1.0

NeilBrown

Index: ChangeLog
===================================================================
RCS file: /cvsroot/nfs/nfs-utils/ChangeLog,v
retrieving revision 1.202

ChangeLog +6 -0

diff -u -r1.202 ChangeLog
@@ -1,3 +1,9 @@
+2003-05-30 Michael Griffith <[email protected]>
+	NeilBrown <[email protected]>
+
+	utils/nfsstat/nfsstat.c(print_callstats): use unsigned
+	long long to avoid overflow when printing stats.
+	
 2003-05-30  NeilBrown <[email protected]>
 
 	* support/export/export.c, support/include/nfslib.h,

Index: utils/nfsstat/nfsstat.c
===================================================================
RCS file: /cvsroot/nfs/nfs-utils/utils/nfsstat/nfsstat.c,v retrieving revision 1.4

nfsstat/nfsstat.c +6 -4

diff -u -r1.4 nfsstat.c
@@ -307,7 +307,8 @@
 print_callstats(const char *hdr, const char **names,
 				 unsigned int *info, unsigned int nr)
 {
-	unsigned int	total;
+	unsigned long long	total;
+        unsigned long long      pct;
 	int		i, j;
 
 	fputs(hdr, stdout);

@@ -319,9 +320,10 @@
 		for (j = 0; j < 6 && i + j < nr; j++)
 			printf("%-11s", names[i+j]);
 		printf("\n");
-		for (j = 0; j < 6 && i + j < nr; j++)
-			printf("%-6d %2d%% ",
-				info[i+j], 100 * info[i+j] / total);
+		for (j = 0; j < 6 && i + j < nr; j++) {
+                        pct = ((unsigned long long) info[i+j]*100)/total;
+                        printf("%-6d %2llu%% ", info[i+j], pct);
+                }
 		printf("\n");
 	}
 	printf("\n");

-------------------------------------------------------
This SF.net email is sponsored by: eBay
Get office equipment for less on eBay!
http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Greg Lindahl

2003-06-02 07:10 UTC #2 lore raw

Quoted 6 lines · Neil Brown

Quoted 2 lines · On Thursday May 29, [email protected] wrote:
A small patch to handle when the RPC call counts are > 2^32/100.
Thanks.
However:

Quoted 6 lines

nfsstat/nfsstat.c line 326

Quoted 6 lines

nfsstat/nfsstat.c +5 -0

+        unsigned long long      pct;
 
+		for (j = 0; j < 6 && i + j < nr; j++) {
+                        pct = ((unsigned long long) info[i+j]*100)/total;
+                        printf("%-6d %2d%% ", info[i+j], pct);
+                }

It feels like overkill to use long long for this when you can get just as good of behavior with:

	unsigned long toto100;

	if (total >= 100)
	   toto100 = total / 100;
	else
	   toto100 = 1;

	for (j = 0; j < 6 && i + j < nr; j++)
              printf("%-6d %2d%% ", info[i+j], info[i+j]/toto100);

In both pieces of code, if total overflows you get the wrong answer. So you didn't get much of a benefit from the long long.

If you care about getting the %s reasonable when total is small, you can move the if() inside the for().

Doesn't gcc inline code on archs without real 64-bit ops? Yech.

greg