2009-03-31 21:02:47

by Greg Banks

[permalink] [raw]
Subject: [patch 01/29] knfsd: Add infrastructure for measuring RPC service times.

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 <[email protected]>
---

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;
+};
+

/*
* This is the RPC server thread function prototype
@@ -419,6 +429,8 @@ int svc_register(const struct svc_se
void svc_wake_up(struct svc_serv *);
void svc_reserve(struct svc_rqst *rqstp, int space);
struct svc_pool * svc_pool_for_cpu(struct svc_serv *serv, int cpu);
+void svc_time_mark(struct svc_time *);
+int svc_time_elapsed(const struct svc_time *, struct timespec *);
char * svc_print_addr(struct svc_rqst *, char *, size_t);

#define RPC_MAX_ADDRBUFLEN (63U)
Index: bfields/net/sunrpc/svc.c
===================================================================
--- bfields.orig/net/sunrpc/svc.c
+++ bfields/net/sunrpc/svc.c
@@ -1232,3 +1232,28 @@ u32 svc_max_payload(const struct svc_rqs
return max;
}
EXPORT_SYMBOL_GPL(svc_max_payload);
+
+
+void
+svc_time_mark(struct svc_time *st)
+{
+ getnstimeofday(&st->st_spec);
+}
+EXPORT_SYMBOL(svc_time_mark);
+
+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


2009-04-25 02:13:18

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [patch 01/29] knfsd: Add infrastructure for measuring RPC service times.

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 <[email protected]>
> ---
>
> 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

2009-04-25 02:14:39

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [patch 01/29] knfsd: Add infrastructure for measuring RPC service times.

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 <[email protected]>
> > ---
> >
> > 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

2009-04-25 02:52:45

by Greg Banks

[permalink] [raw]
Subject: Re: [patch 01/29] knfsd: Add infrastructure for measuring RPC service times.

On Sat, Apr 25, 2009 at 12:13 PM, J. Bruce Fields <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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.)

To be quite frank, no. They're historical; they were very necessary
in the original SLES10-based version of these patches because that
ancient kernel was missing some important time infrastructure bits.
In that code, the structure and functions did some very hairy and
arch-specific things involving jiffies and ITC/TSC registers. I was
being lazy by not eliding them during forward porting, and I'll do so
in the next version.

--
Greg.