From: "J. Bruce Fields" <bfields@fieldses.org>
Subject: Re: [patch 01/29] knfsd: Add infrastructure for measuring RPC
	service times.
Date: Fri, 24 Apr 2009 22:14:38 -0400
Message-ID: <20090425021438.GB24770@fieldses.org>
References: <20090331202800.739621000@sgi.com> <20090331202937.980076000@sgi.com> <20090425021315.GA24770@fieldses.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Linux NFS ML <linux-nfs@vger.kernel.org>
To: Greg Banks <gnb-xTcybq6BZ68@public.gmane.org>
In-Reply-To: <20090425021315.GA24770@fieldses.org>
Sender: linux-nfs-owner@vger.kernel.org

Whoops--correcting Greg's email address.

On Fri, Apr 24, 2009 at 10:13:15PM -0400, bfields wrote:
> On Wed, Apr 01, 2009 at 07:28:01AM +1100, Greg Banks wrote:
> > Two new functions; svc_time_mark() remembers the current time
> > in a struct svc_time; svc_time_elapsed() calculates and returns
> > the time since a svc_time was marked.
> > 
> > Signed-off-by: Greg Banks <gnb@sgi.com>
> > ---
> > 
> >  include/linux/sunrpc/svc.h |   12 ++++++++++++
> >  net/sunrpc/svc.c           |   25 +++++++++++++++++++++++++
> >  2 files changed, 37 insertions(+)
> > 
> > Index: bfields/include/linux/sunrpc/svc.h
> > ===================================================================
> > --- bfields.orig/include/linux/sunrpc/svc.h
> > +++ bfields/include/linux/sunrpc/svc.h
> > @@ -18,6 +18,16 @@
> >  #include <linux/sunrpc/svcauth.h>
> >  #include <linux/wait.h>
> >  #include <linux/mm.h>
> > +#include <linux/time.h>
> > +
> > +/*
> > + * Structure used to implement a fast lockless elapsed time measure.
> > + */
> > +struct svc_time
> > +{
> > +	struct timespec	st_spec;
> > +};
> 
> Are struct svc_time, ...
> 
> > +void
> > +svc_time_mark(struct svc_time *st)
> > +{
> > +	getnstimeofday(&st->st_spec);
> > +}
> > +EXPORT_SYMBOL(svc_time_mark);
> 
> ... and this function really necessary?  If you're not too attached to
> them: it would seem simpler just to use struct timespec and
> getnstimeofday directly.  (Well, at least simpler to read for someone
> familiar with the kernel but not with th nfs code.)
> 
> --b.
> 
> > +
> > +int
> > +svc_time_elapsed(const struct svc_time *mark, struct timespec *ts)
> > +{
> > +	struct svc_time now;
> > +
> > +	svc_time_mark(&now);
> > +
> > +	if (now.st_spec.tv_sec < mark->st_spec.tv_sec)
> > +		return -EINVAL;	/* time going backwards */
> > +
> > +	*ts = timespec_sub(now.st_spec, mark->st_spec);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(svc_time_elapsed);
> > +
> > 
> > --
> > Greg