2009-07-21 19:21:51

by Martin Schwidefsky

[permalink] [raw]
Subject: [RFC][patch 4/5] clocksource_read/clocksource_read_raw inline functions

From: Martin Schwidefsky <[email protected]>

Add clocksource_read / clocksource_read_raw inline functions and use
them for getnstimeofday, ktime_get, ktime_get_ts and getrawmonotonic.

Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: john stultz <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
include/linux/clocksource.h | 45 +++++++++++++++++++++++++++++++++++++++++++
kernel/time/timekeeping.c | 46 +++-----------------------------------------
2 files changed, 49 insertions(+), 42 deletions(-)

Index: linux-2.6/include/linux/clocksource.h
===================================================================
--- linux-2.6.orig/include/linux/clocksource.h
+++ linux-2.6/include/linux/clocksource.h
@@ -284,6 +284,51 @@ static inline s64 cyc2ns(struct clocksou
return ret;
}

+/**
+ * clocksource_read: - Read nanosecond delta from clocksource.
+ * @cs: pointer to clocksource being read
+ *
+ * Read from the clock source and return the clock value converted
+ * to nanoseconds.
+ */
+static inline s64 clocksource_read(struct clocksource *cs)
+{
+ cycle_t cycle_now, cycle_delta;
+
+ /* read clocksource: */
+ cycle_now = cs->read(cs);
+
+ /* calculate the delta since the last update_wall_time: */
+ cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;
+
+ /* convert to nanoseconds: */
+ return cyc2ns(cs, cycle_delta);
+}
+
+/**
+ * clocksource_read_raw: - Read raw nanosecond delta from clocksource.
+ * @cs: pointer to clocksource being read
+ *
+ * Read from the clock source and return the clock value converted
+ * to nanoseconds.
+ */
+static inline s64 clocksource_read_raw(struct clocksource *cs,
+ struct timespec *ts)
+{
+ cycle_t cycle_now, cycle_delta;
+
+ *ts = clock->raw_time;
+
+ /* read clocksource: */
+ cycle_now = cs->read(cs);
+
+ /* calculate the delta since the last update_wall_time: */
+ cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;
+
+ /* convert to nanoseconds: */
+ return ((s64) cycle_delta * clock->mult_orig) >> clock->shift;
+}
+
/* used to install a new clocksource */
extern void __init clocksource_init(void);
extern int clocksource_register(struct clocksource*);
Index: linux-2.6/kernel/time/timekeeping.c
===================================================================
--- linux-2.6.orig/kernel/time/timekeeping.c
+++ linux-2.6/kernel/time/timekeeping.c
@@ -66,7 +66,6 @@ void update_xtime_cache(u64 nsec)
*/
void getnstimeofday(struct timespec *ts)
{
- cycle_t cycle_now, cycle_delta;
unsigned long seq;
s64 nsecs;

@@ -76,15 +75,7 @@ void getnstimeofday(struct timespec *ts)
seq = read_seqbegin(&xtime_lock);

*ts = xtime;
-
- /* read clocksource: */
- cycle_now = clock->read(clock);
-
- /* calculate the delta since the last update_wall_time: */
- cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
-
- /* convert to nanoseconds: */
- nsecs = cyc2ns(clock, cycle_delta);
+ nsecs = clocksource_read(clock);

/* If arch requires, add in gettimeoffset() */
nsecs += arch_gettimeoffset();
@@ -98,7 +89,6 @@ EXPORT_SYMBOL(getnstimeofday);

ktime_t ktime_get(void)
{
- cycle_t cycle_now, cycle_delta;
unsigned int seq;
s64 secs, nsecs;

@@ -108,15 +98,7 @@ ktime_t ktime_get(void)
seq = read_seqbegin(&xtime_lock);
secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
-
- /* read clocksource: */
- cycle_now = clock->read(clock);
-
- /* calculate the delta since the last update_wall_time: */
- cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
-
- /* convert to nanoseconds: */
- nsecs += cyc2ns(clock, cycle_delta);
+ nsecs += clocksource_read(clock);

} while (read_seqretry(&xtime_lock, seq));
/*
@@ -137,7 +119,6 @@ EXPORT_SYMBOL_GPL(ktime_get);
*/
void ktime_get_ts(struct timespec *ts)
{
- cycle_t cycle_now, cycle_delta;
struct timespec tomono;
unsigned int seq;
s64 nsecs;
@@ -148,15 +129,7 @@ void ktime_get_ts(struct timespec *ts)
seq = read_seqbegin(&xtime_lock);
*ts = xtime;
tomono = wall_to_monotonic;
-
- /* read clocksource: */
- cycle_now = clock->read(clock);
-
- /* calculate the delta since the last update_wall_time: */
- cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
-
- /* convert to nanoseconds: */
- nsecs = cyc2ns(clock, cycle_delta);
+ nsecs = clocksource_read(clock);

} while (read_seqretry(&xtime_lock, seq));

@@ -290,21 +263,10 @@ void getrawmonotonic(struct timespec *ts
{
unsigned long seq;
s64 nsecs;
- cycle_t cycle_now, cycle_delta;

do {
seq = read_seqbegin(&xtime_lock);
-
- /* read clocksource: */
- cycle_now = clock->read(clock);
-
- /* calculate the delta since the last update_wall_time: */
- cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
-
- /* convert to nanoseconds: */
- nsecs = ((s64)cycle_delta * clock->mult_orig) >> clock->shift;
-
- *ts = clock->raw_time;
+ nsecs = clocksource_read_raw(clock, ts);

} while (read_seqretry(&xtime_lock, seq));


--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.


2009-07-21 22:01:19

by john stultz

[permalink] [raw]
Subject: Re: [RFC][patch 4/5] clocksource_read/clocksource_read_raw inline functions

On Tue, 2009-07-21 at 21:17 +0200, Martin Schwidefsky wrote:
> plain text document attachment (clocksource-read-ns.diff)
> From: Martin Schwidefsky <[email protected]>
>
> Add clocksource_read / clocksource_read_raw inline functions and use
> them for getnstimeofday, ktime_get, ktime_get_ts and getrawmonotonic.
>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: john stultz <[email protected]>
> Signed-off-by: Martin Schwidefsky <[email protected]>
> ---
> include/linux/clocksource.h | 45 +++++++++++++++++++++++++++++++++++++++++++
> kernel/time/timekeeping.c | 46 +++-----------------------------------------
> 2 files changed, 49 insertions(+), 42 deletions(-)
>
> Index: linux-2.6/include/linux/clocksource.h
> ===================================================================
> --- linux-2.6.orig/include/linux/clocksource.h
> +++ linux-2.6/include/linux/clocksource.h
> @@ -284,6 +284,51 @@ static inline s64 cyc2ns(struct clocksou
> return ret;
> }
>
> +/**
> + * clocksource_read: - Read nanosecond delta from clocksource.
> + * @cs: pointer to clocksource being read
> + *
> + * Read from the clock source and return the clock value converted
> + * to nanoseconds.
> + */
> +static inline s64 clocksource_read(struct clocksource *cs)
> +{
> + cycle_t cycle_now, cycle_delta;
> +
> + /* read clocksource: */
> + cycle_now = cs->read(cs);
> +
> + /* calculate the delta since the last update_wall_time: */
> + cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;
> +
> + /* convert to nanoseconds: */
> + return cyc2ns(cs, cycle_delta);
> +}

Oof. So you took out clocksource_read() only to replace with a different
function with the same name? If this move is necessary, could we call
these clocksource_get_ns()/clocksource_get_raw_ns() to avoid the
confusion?

thanks
-john

2009-07-22 07:29:40

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 4/5] clocksource_read/clocksource_read_raw inline functions

On Tue, 21 Jul 2009 15:01:14 -0700
john stultz <[email protected]> wrote:

> On Tue, 2009-07-21 at 21:17 +0200, Martin Schwidefsky wrote:
> > plain text document attachment (clocksource-read-ns.diff)
> > From: Martin Schwidefsky <[email protected]>
> >
> > Add clocksource_read / clocksource_read_raw inline functions and use
> > them for getnstimeofday, ktime_get, ktime_get_ts and getrawmonotonic.
> >
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: john stultz <[email protected]>
> > Signed-off-by: Martin Schwidefsky <[email protected]>
> > ---
> > include/linux/clocksource.h | 45 +++++++++++++++++++++++++++++++++++++++++++
> > kernel/time/timekeeping.c | 46 +++-----------------------------------------
> > 2 files changed, 49 insertions(+), 42 deletions(-)
> >
> > Index: linux-2.6/include/linux/clocksource.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/clocksource.h
> > +++ linux-2.6/include/linux/clocksource.h
> > @@ -284,6 +284,51 @@ static inline s64 cyc2ns(struct clocksou
> > return ret;
> > }
> >
> > +/**
> > + * clocksource_read: - Read nanosecond delta from clocksource.
> > + * @cs: pointer to clocksource being read
> > + *
> > + * Read from the clock source and return the clock value converted
> > + * to nanoseconds.
> > + */
> > +static inline s64 clocksource_read(struct clocksource *cs)
> > +{
> > + cycle_t cycle_now, cycle_delta;
> > +
> > + /* read clocksource: */
> > + cycle_now = cs->read(cs);
> > +
> > + /* calculate the delta since the last update_wall_time: */
> > + cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;
> > +
> > + /* convert to nanoseconds: */
> > + return cyc2ns(cs, cycle_delta);
> > +}
>
> Oof. So you took out clocksource_read() only to replace with a different
> function with the same name? If this move is necessary, could we call
> these clocksource_get_ns()/clocksource_get_raw_ns() to avoid the
> confusion?

The code that uses the old clocksource_read is accessing other fields
from the struct clocksource. So the old inline function seems
redundant to me. The new one do more and returns a value where you
don't need anything else from the clocksource (namely nanoseconds).
To reuse the name adds a little confusion while you look at the patches,
the end result should be pretty clear, no? But if you prefer to add the
_ns suffix that is fine with me.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.