Linux NFS development
 help / color / mirror / Atom feed
* [PATCH 0/2] nfsstat: Patch resend
@ 2009-04-03 19:06 Kevin Constantine
       [not found] ` <1238785592-18869-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Kevin Constantine @ 2009-04-03 19:06 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs, Kevin.Constantine-P5ys19MLBK/QT0dZR+AlfA

Just wanted to re-send the patches so that the right versions get looked at.  This leaves out an erroneous printing of newlines when there's no nfs traffic that was in the original patchset.

-kevin

[PATCH 1/2] nfsstat: Print diff stats every N seconds
[PATCH 2/2] nfsstat: Add --list flag

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsstat: Add --list flag
@ 2009-03-17  4:18 Greg Banks
  2009-03-17  7:46 ` [PATCH 1/2] nfsstat: Print diff stats every N seconds Kevin Constantine
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Banks @ 2009-03-17  4:18 UTC (permalink / raw)
  To: Kevin Constantine; +Cc: linux-nfs, steved, Kevin Constantine

Kevin Constantine wrote:
> nfsstat.c: Adds the --list flag to print information in a list format instead of the standard multi-column format
> nfsstat.man: Updates the manpage to include the --list flag.
>
> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
> ---
>  utils/nfsstat/nfsstat.c   |   87 ++++++++++++++++++++++++++++++--------------
>  utils/nfsstat/nfsstat.man |    3 ++
>  2 files changed, 62 insertions(+), 28 deletions(-)
>
> diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
> index c25580c..51aea8d 100644
> --- a/utils/nfsstat/nfsstat.c
> +++ b/utils/nfsstat/nfsstat.c
> @@ -176,7 +176,7 @@ static void		print_callstats(const char *, const char **,
> @@ -257,6 +257,7 @@ static struct option longopts[] =
> @@ -269,6 +270,7 @@ main(int argc, char **argv)
> @@ -290,7 +292,7 @@ main(int argc, char **argv)
> @@ -348,6 +350,9 @@ main(int argc, char **argv)
Looks good.
> @@ -428,15 +433,24 @@ main(int argc, char **argv)
>  				get_stats(NFSCLTSTAT, clientinfo_tmp, &opt_clt, opt_srv, 0);
>  				diff_stats(clientinfo_tmp, clientinfo, 0);
>  			}
> -			print_stats_list(opt_prt);
> +			if (opt_list) {
> +				print_stats_list(opt_prt);
> +			} else {
> +				print_server_stats(opt_srv, opt_prt);
> +				print_client_stats(opt_clt, opt_prt);
> +			}		
>  			fflush(stdout);
>  
>  			update_old_counters(clientinfo_tmp, clientinfo);
>  			sleep(sleep_time);
>  		}	
>  	} else {
> -		print_server_stats(opt_srv, opt_prt);
> -		print_client_stats(opt_clt, opt_prt);
> +		if (opt_list) {
> +			print_stats_list(opt_prt);
> +		} else {
> +			print_server_stats(opt_srv, opt_prt);
> +			print_client_stats(opt_clt, opt_prt);
> +		}
>  	}
>  
>   
The logic is good here.  The code might have been marginally cleaner if
those 6 lines which are now duplicated were pushed into a function.

>  	return 0;
> @@ -568,29 +582,31 @@ print_stats_list(int opt_prt)
> @@ -648,19 +664,34 @@ print_callstats(const char *hdr, const char **names,
>  static void
>  print_callstats_list(const char *hdr, const char **names,
>  		 	unsigned int *srvinfo, unsigned int *cltinfo, 
> -			unsigned int nr)
> +			unsigned int nr, unsigned int nfsvers)
>  {
>  	unsigned long long	srvtotal, clttotal;
>  	int			i;
>  
> -	for (i = 0, srvtotal = 0, clttotal = 0; i < nr; i++) {
> -		srvtotal += srvinfo[i];
> -		clttotal += cltinfo[i];
> -	}
> -	printf("%13s %8s %8s\n", hdr, "Server", "Client");
> -	printf("%12s: %8llu %8llu \n", "total", srvtotal, clttotal);
> -	for (i = 0; i < nr; i++) {
> -			printf("%12s: %8u %8u \n", names[i], srvinfo[i], cltinfo[i]);
> +	if (nfsvers < 4) {
> +		for (i = 0, srvtotal = 0, clttotal = 0; i < nr; i++) {
> +			srvtotal += srvinfo[i];
> +			clttotal += cltinfo[i];
> +		}
> +		printf("%13s %8s %8s\n", hdr, "Server", "Client");
> +		printf("%12s: %8llu %8llu \n", "total", srvtotal, clttotal);
> +		for (i = 0; i < nr; i++) {
> +				printf("%12s: %8u %8u \n", names[i], srvinfo[i], cltinfo[i]);
> +		}
> +	} else {
> +		/* nfsv4's client and server calls aren't named the same, so we'll print them out
> +		*	separately.  Everything will be passed in as srvinfo.  Ignore cltinfo which
> +		*	will be passed in as 0
> +		*/
> +		for (i = 0, srvtotal = 0; i < nr; i++)
> +			srvtotal += srvinfo[i];
> +		printf("%13s %8s\n", hdr, "Calls");
> +		printf("%12s: %8llu \n", "total", srvtotal);
> +		for (i = 0; i < nr; i++) {
> +				printf("%12s: %8u \n", names[i], srvinfo[i]);
> +		}
> +		printf("\n");
>  	}
>   

Maybe you can smooth out this difference, and make the code simpler and
cleaner, by making the output always be exactly two columns, one for the
label and one for the value.  Then your earlier quoted example would be
something like

       nfs v3 client total:   213
        nfs v3 client null:     0
     nfs v3 client getattr:    23
     nfs v3 client setattr:     9
      nfs v3 client lookup:     8
      nfs v3 client access:     1
    nfs v3 client readlink:     0
        nfs v3 client read:     7
       nfs v3 client write:    23
      nfs v3 client create:     8
       nfs v3 client mkdir:     0
     nfs v3 client symlink:     0
       nfs v3 client mknod:     0
      nfs v3 client remove:     7
       nfs v3 client rmdir:     0
      nfs v3 client rename:     1
        nfs v3 client link:     0
     nfs v3 client readdir:     0
 nfs v3 client readdirplus:     0
      nfs v3 client fsstat:   126
      nfs v3 client fsinfo:     0
    nfs v3 client pathconf:     0
      nfs v3 client commit:     0 
       nfs v3 server total:     0
        nfs v3 server null:     0
     nfs v3 server getattr:     0
     nfs v3 server setattr:     0
      nfs v3 server lookup:     0
      nfs v3 server access:     0
    nfs v3 server readlink:     0
        nfs v3 server read:     0
       nfs v3 server write:     0
      nfs v3 server create:     0
       nfs v3 server mkdir:     0
     nfs v3 server symlink:     0
       nfs v3 server mknod:     0
      nfs v3 server remove:     0
       nfs v3 server rmdir:     0
      nfs v3 server rename:     0
        nfs v3 server link:     0
     nfs v3 server readdir:     0
 nfs v3 server readdirplus:     0
      nfs v3 server fsstat:     0
      nfs v3 server fsinfo:     0
    nfs v3 server pathconf:     0
      nfs v3 server commit:     0


Instead of

nfs v3 stats:   Server   Client
       total:        0      213
        null:        0        0
     getattr:        0       23
     setattr:        0        9
      lookup:        0        8
      access:        0        1
    readlink:        0        0
        read:        0        7
       write:        0       23
      create:        0        8
       mkdir:        0        0
     symlink:        0        0
       mknod:        0        0
      remove:        0        7
       rmdir:        0        0
      rename:        0        1
        link:        0        0
     readdir:        0        0
 readdirplus:        0        0
      fsstat:        0      126
      fsinfo:        0        0
    pathconf:        0        0
      commit:        0        0 


This would also help with your stated objective of being to grep the output.
>  		
>  }
> diff --git a/utils/nfsstat/nfsstat.man b/utils/nfsstat/nfsstat.man
> index 461b2c0..52215a9 100644
> --- a/utils/nfsstat/nfsstat.man
> +++ b/utils/nfsstat/nfsstat.man
> @@ -72,6 +72,9 @@ Display all of the above facilities.
>
>   
Looks good.

-- 
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-04-04 11:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-03 19:06 [PATCH 0/2] nfsstat: Patch resend Kevin Constantine
     [not found] ` <1238785592-18869-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-04-03 19:06   ` [PATCH 1/2] nfsstat: Print diff stats every N seconds Kevin Constantine
     [not found]     ` <1238785592-18869-2-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-04-03 19:06       ` [PATCH 2/2] nfsstat: Add --list flag Kevin Constantine
     [not found]         ` <1238785592-18869-3-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-04-03 23:49           ` Greg Banks
     [not found]             ` <ac442c870904031649o5880e9fcpeb3d6420c5de9a9c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-04-04 11:55               ` Steve Dickson
2009-04-03 23:48       ` [PATCH 1/2] nfsstat: Print diff stats every N seconds Greg Banks
     [not found]         ` <ac442c870904031648v5b40e29avf8c1327c07b4f7ec-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-04-04 11:55           ` Steve Dickson
2009-04-04 10:42       ` Steve Dickson
  -- strict thread matches above, loose matches on Subject: below --
2009-03-17  4:18 [PATCH 2/2] nfsstat: Add --list flag Greg Banks
2009-03-17  7:46 ` [PATCH 1/2] nfsstat: Print diff stats every N seconds Kevin Constantine
     [not found]   ` <1237275989-16421-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-03-18  2:43     ` Greg Banks

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox