2008-07-11 20:55:46

by Uwe Kleine-König

[permalink] [raw]
Subject: [PATCH, RFC] use hrtimer in sched_clock

This should make it unnecessary to overwrite sched_clock for a higher
precision.
With this patch I get sub-jiffie timing with CONFIG_PRINTK_TIME=y.

Signed-off-by: Uwe Kleine-König <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
Hello,

I tested the patch on arch-arm/mach-ns9xxx and it seem seams to work.
But I admit I didn't test it deeply and I didn't measure if there is any
overhead.

Best regards
Uwe

kernel/sched_clock.c | 6 +++++-
kernel/time/timekeeping.c | 16 +++++++++-------
2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/kernel/sched_clock.c b/kernel/sched_clock.c
index ce05271..12daf55 100644
--- a/kernel/sched_clock.c
+++ b/kernel/sched_clock.c
@@ -242,5 +242,9 @@ EXPORT_SYMBOL_GPL(sched_clock_idle_wakeup_event);
*/
unsigned long long __attribute__((weak)) sched_clock(void)
{
- return (unsigned long long)jiffies * (NSEC_PER_SEC / HZ);
+ struct timespec ts;
+
+ ktime_get_ts(&ts);
+
+ return (long long)ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
}
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e91c29f..49412f2 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -67,16 +67,18 @@ struct clocksource *clock;
static inline s64 __get_nsec_offset(void)
{
cycle_t cycle_now, cycle_delta;
- s64 ns_offset;
+ s64 ns_offset = 0;

- /* read clocksource: */
- cycle_now = clocksource_read(clock);
+ if (likely(clock)) {
+ /* read clocksource: */
+ cycle_now = clocksource_read(clock);

- /* calculate the delta since the last update_wall_time: */
- cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
+ /* calculate the delta since the last update_wall_time: */
+ cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;

- /* convert to nanoseconds: */
- ns_offset = cyc2ns(clock, cycle_delta);
+ /* convert to nanoseconds: */
+ ns_offset = cyc2ns(clock, cycle_delta);
+ }

return ns_offset;
}
--
1.5.6.2


2008-07-14 09:30:37

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH, RFC] use hrtimer in sched_clock

On Fri, 11 Jul 2008, Uwe Kleine-König wrote:
> This should make it unnecessary to overwrite sched_clock for a higher
> precision.
> With this patch I get sub-jiffie timing with CONFIG_PRINTK_TIME=y.
>
> Signed-off-by: Uwe Kleine-König <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> ---
> Hello,
>
> I tested the patch on arch-arm/mach-ns9xxx and it seem seams to work.
> But I admit I didn't test it deeply and I didn't measure if there is any
> overhead.

There is lots of overhead and your approach is simply wrong.

> unsigned long long __attribute__((weak)) sched_clock(void)

The correct way to solve it is to override sched_clock() with a high
resolution implementation for your hardware platform. sched_clock is a
weak function to provide a default implementation based on jiffies.

If you do a grep -r sched_clock arch/arm you'll find a couple of
examples how to override sched_clock().

Thanks,

tglx

2008-07-14 09:49:04

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH, RFC] use hrtimer in sched_clock

Hello Thomas,

Thomas Gleixner wrote:
> On Fri, 11 Jul 2008, Uwe Kleine-K?nig wrote:
> > This should make it unnecessary to overwrite sched_clock for a higher
> > precision.
> > With this patch I get sub-jiffie timing with CONFIG_PRINTK_TIME=y.
> >
> > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > ---
> > Hello,
> >
> > I tested the patch on arch-arm/mach-ns9xxx and it seem seams to work.
> > But I admit I didn't test it deeply and I didn't measure if there is any
> > overhead.
>
> There is lots of overhead and your approach is simply wrong.
I expected to have some overhead, but can you please elaborate on
"simply wrong"?
I will try to measure the actual overhead but cannot promise to find
time for that in the near future.

> > unsigned long long __attribute__((weak)) sched_clock(void)
>
> The correct way to solve it is to override sched_clock() with a high
> resolution implementation for your hardware platform. sched_clock is a
> weak function to provide a default implementation based on jiffies.
>
> If you do a grep -r sched_clock arch/arm you'll find a couple of
> examples how to override sched_clock().
I new that and my purpose was to make this unneeded. Would be nice,
wouldn't it?

Thanks and best regards,
Uwe

--
Uwe Kleine-K?nig, Software Engineer
Digi International GmbH Branch Breisach, K?ferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

2008-07-14 10:04:06

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH, RFC] use hrtimer in sched_clock

On Mon, 14 Jul 2008, Uwe Kleine-K?nig wrote:
> > > I tested the patch on arch-arm/mach-ns9xxx and it seem seams to work.
> > > But I admit I didn't test it deeply and I didn't measure if there is any
> > > overhead.
> >
> > There is lots of overhead and your approach is simply wrong.
> I expected to have some overhead, but can you please elaborate on
> "simply wrong"?

It's wrong because it has a locking problem vs. xtime lock. Try
printk() in a code region which has xtime_lock write locked.

> > If you do a grep -r sched_clock arch/arm you'll find a couple of
> > examples how to override sched_clock().
> I new that and my purpose was to make this unneeded. Would be nice,
> wouldn't it?

It would be nice, but it's simply not possible as each hardware
platform has a different hardware which is used for that. Also your
approach enforces that the hardware which is used for sched_clock is
the same as the one which is used for timekeeping. That's not
necessarily a good idea as we can use a less accurate and less
reliable hardware for sched_clock() than for timekeeping and we want
to do that when the sched_clock() hardware is faster to access.

Thanks,

tglx