[PATCHSET] Time: Generic Timeofday Subsystem (v B17)

[PATCHSET] Time: Generic Timeofday Subsystem (v B17)

[john stultz]

All,
	Yes, B17 already. I didn't bother to announce the B16 release, since it
was just a sync w/ the patches included into -mm. This announcement is
mainly for upstream users of my patch (-kthrt and -rt trees), or anyone
who wants to play with the patches outside of -mm. Anyway here it is:

	This patchset provides a generic timekeeping subsystem that is
independent of the timer interrupt. This allows for robust and correct
behavior in cases of late or lost ticks, avoids interpolation errors,
reduces duplication in arch specific code, and allows or assists future
changes such as high-res timers, dynamic ticks, or realtime preemption.
Additionally, it provides finer nanosecond resolution values to the
clock_gettime functions.

	The patch set provides the minimal NTP changes, the clocksource
abstraction, the core timekeeping code as well as the code to convert
i386. I have started on converting more arches, but for now I'm focusing
on code for i386 and x86-64.

Changes since the B16 release:
o Avoids clocksource churn
o Blacklist silent cpufreq changing thinkpad
o Sync up w/ patches in 2.6.16-rc1-mm2

Outstanding issues:
o TSC cpufreq caused stalls at bootup (working on this one)

Still on my TODO list:
o Squish any bugs that pop up from testing in -mm and -rt
o Finer grained ntp adjustment accounting (Suggested by Roman)
o A few spots could use some optimization (Again, suggested by Roman)
o Clean and split up x86-64 patch
o powerpc/ppc, s390, arm, ia64, alpha, sparc, sparc64 work


The patchset applies against the current 2.6.16-rc1-git.

The complete patchset can be found here:
	http://sr71.net/~jstultz/tod/


I'd like to thank the following people who have contributed ideas,
criticism, testing and code that has helped shape this work: 
	George Anzinger, Nish Aravamudan, Max Asbock, Serge Belyshev, Dominik
Brodowski, Thomas Gleixner, Darren Hart, Christoph Lameter, Matt Mackal,
Keith Mannthey, Ingo Molnar, Martin Schwidefsky, Frank Sorenson, Ulrich
Windl, Jonathan Woithe, Darrick Wong, Roman Zippel and any others whom
I've accidentally left off this list.

As always, feedback, suggestions and bug-reports are always appreciated.

thanks 
-john

Re: [PATCHSET] Time: Generic Timeofday Subsystem (v B17)

[Paul Mundt]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, Jan 20, 2006 at 04:00:26PM -0800, john stultz wrote:
> The patchset applies against the current 2.6.16-rc1-git.
> 
> The complete patchset can be found here:
> 	http://sr71.net/~jstultz/tod/
> 
> As always, feedback, suggestions and bug-reports are always appreciated.
> 
Patches weren't explicitly mentioned, but I'm assuming they're also
welcome ;-)

At first glance the register/unregister interface is a bit
unconventional, but I have a few trivial patches to get those fixed up,
which I'll send to you separately.

It looks like struct clocksource will also need suspend/resume ops,
since it's defining its own sysclass (so there's no overlap with the
timer sysclass that several other architectures setup to deal with this).
I haven't done that yet, but if there's interest, I'll hack something up.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCHSET] Time: Generic Timeofday Subsystem (v B17)

[john stultz]

On Mon, 2006-01-23 at 01:39 +0200, Paul Mundt wrote:
> On Fri, Jan 20, 2006 at 04:00:26PM -0800, john stultz wrote:
> > The patchset applies against the current 2.6.16-rc1-git.
> > 
> > The complete patchset can be found here:
> > 	http://sr71.net/~jstultz/tod/
> > 
> > As always, feedback, suggestions and bug-reports are always appreciated.
> > 
> Patches weren't explicitly mentioned, but I'm assuming they're also
> welcome ;-)
> 
> At first glance the register/unregister interface is a bit
> unconventional, but I have a few trivial patches to get those fixed up,
> which I'll send to you separately.

Very Cool!

> It looks like struct clocksource will also need suspend/resume ops,
> since it's defining its own sysclass (so there's no overlap with the
> timer sysclass that several other architectures setup to deal with this).
> I haven't done that yet, but if there's interest, I'll hack something up.

Hmmm. While we do have suspend and resume functions for the timekeeping
core, I'm not sure if suspend and resume are necessary for each struct
clocksource since state changes are only allowed inside the callback
hook.  That probably should be better documented. :)

Some specific hardware that might provide a clocksource interface may
need suspend/resume hooks (like the PIT, for example), but in my mind
that would be done independently from the clocksource code.

But maybe I'm not quite grasping what you're suggesting, so feel free to
clarify with maybe an example of its use and I'll re-think it.

Thanks for the code review and patches! I really appreciate it!
-john

Re: [PATCHSET] Time: Generic Timeofday Subsystem (v B17)

[Paul Mundt]

On Mon, Jan 23, 2006 at 12:10:36PM -0800, john stultz wrote:
> On Mon, 2006-01-23 at 01:39 +0200, Paul Mundt wrote:
> > On Fri, Jan 20, 2006 at 04:00:26PM -0800, john stultz wrote:
> > > The patchset applies against the current 2.6.16-rc1-git.
> > > 
> > > The complete patchset can be found here:
> > > 	http://sr71.net/~jstultz/tod/
> > > 
> > > As always, feedback, suggestions and bug-reports are always appreciated.
> > > 
> > Patches weren't explicitly mentioned, but I'm assuming they're also
> > welcome ;-)
> > 
> > At first glance the register/unregister interface is a bit
> > unconventional, but I have a few trivial patches to get those fixed up,
> > which I'll send to you separately.
> 
> Very Cool!
> 
Here it is again hopefully without the formatting damage.. quite trivial.

Currently register_clocksource() is a bit unconventional in that it's
currently void, which has to be a first for kernel register_xxx()
routines. Additionally, register_clocksource() _can_ fail, and
essentially every caller of it has an int return type anyways, so it
doesn't make much sense to lie about it.

The other issue is that the clocksource documentation is wrong, the
example won't compile, defines everything as non-static, and the
initcall is never flagged as __init.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 Documentation/timekeeping.txt |   15 ++++++---------
 arch/i386/kernel/hpet.c       |    4 +---
 arch/i386/kernel/i8253.c      |    4 +---
 arch/i386/kernel/tsc.c        |    2 +-
 arch/x86_64/kernel/time.c     |    6 ++----
 drivers/clocksource/acpi_pm.c |    4 +---
 drivers/clocksource/cyclone.c |    4 +---
 include/linux/clocksource.h   |    2 +-
 kernel/time/clocksource.c     |    8 ++++++--
 kernel/time/jiffies.c         |    4 +---
 10 files changed, 21 insertions(+), 32 deletions(-)

8fa73a3ae06f803d393ec91fc3b199504c447d6d
diff --git a/Documentation/timekeeping.txt b/Documentation/timekeeping.txt
index 255fd56..9a4edf2 100644
--- a/Documentation/timekeeping.txt
+++ b/Documentation/timekeeping.txt
@@ -306,27 +306,24 @@ So lets start out an empty cool-counter.
 
 static __iomem *cool_ptr = COOL_READ_PTR;
 
-struct clocksource clocksource_cool
-{
+static struct clocksource clocksource_cool = {
 	.name = "cool",
 	.rating = 200,		/* its a pretty decent clock */
 	.mask = 0xFFFFFFFF,	/* 32 bits */
 	.mult = 0,			/*to be computed */
 	.shift = 10,
-}
-
+};
 
 Now let's write the read function:
 
-cycle_t cool_counter_read(void)
+static cycle_t cool_counter_read(void)
 {
-	cycle_t ret = readl(cool_ptr);
-	return ret;
+	return (cycle_t)readl(cool_ptr);
 }
 
 Finally, lets write the init function:
 
-void cool_counter_init(void)
+static int __init cool_counter_init(void)
 {
 	__iomem *ptr = COOL_START_PTR;
 	u32 val;
@@ -342,7 +339,7 @@ void cool_counter_init(void)
 					clocksource_cool.shift);
 
 	/* register the clocksource */
-	register_clocksource(&clocksource_cool);
+	return register_clocksource(&clocksource_cool);
 }
 module_init(cool_counter_init);
 
diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c
index a620d15..61d8cd2 100644
--- a/arch/i386/kernel/hpet.c
+++ b/arch/i386/kernel/hpet.c
@@ -61,9 +61,7 @@ static int __init init_hpet_clocksource(
 	do_div(tmp, FSEC_PER_NSEC);
 	clocksource_hpet.mult = (u32)tmp;
 
-	register_clocksource(&clocksource_hpet);
-
-	return 0;
+	return register_clocksource(&clocksource_hpet);
 }
 
 module_init(init_hpet_clocksource);
diff --git a/arch/i386/kernel/i8253.c b/arch/i386/kernel/i8253.c
index 65917f9..cce0eb6 100644
--- a/arch/i386/kernel/i8253.c
+++ b/arch/i386/kernel/i8253.c
@@ -84,8 +84,6 @@ static int __init init_pit_clocksource(v
 		return 0;
 
 	clocksource_pit.mult = clocksource_hz2mult(CLOCK_TICK_RATE, 20);
-	register_clocksource(&clocksource_pit);
-
-	return 0;
+	return register_clocksource(&clocksource_pit);
 }
 module_init(init_pit_clocksource);
diff --git a/arch/i386/kernel/tsc.c b/arch/i386/kernel/tsc.c
index 36e1676..00ed75b 100644
--- a/arch/i386/kernel/tsc.c
+++ b/arch/i386/kernel/tsc.c
@@ -472,7 +472,7 @@ static int __init init_tsc_clocksource(v
 		/* lower the rating if we already know its unstable: */
 		if (check_tsc_unstable())
 			clocksource_tsc.rating = 50;
-		register_clocksource(&clocksource_tsc);
+		return register_clocksource(&clocksource_tsc);
 	}
 
 	return 0;
diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index e5d75d1..2ab7505 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -1122,7 +1122,7 @@ static int __init init_tsc_clocksource(v
 		current_tsc_khz = tsc_khz;
 		clocksource_tsc.mult = clocksource_khz2mult(current_tsc_khz,
 							clocksource_tsc.shift);
-		register_clocksource(&clocksource_tsc);
+		return register_clocksource(&clocksource_tsc);
 	}
 	return 0;
 }
@@ -1191,9 +1191,7 @@ static int __init init_hpet_clocksource(
 	do_div(tmp, FSEC_PER_NSEC);
 	clocksource_hpet.mult = (u32)tmp;
 
-	register_clocksource(&clocksource_hpet);
-
-	return 0;
+	return register_clocksource(&clocksource_hpet);
 }
 
 module_init(init_hpet_clocksource);
diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
index e5bc091..7aeef22 100644
--- a/drivers/clocksource/acpi_pm.c
+++ b/drivers/clocksource/acpi_pm.c
@@ -115,9 +115,7 @@ pm_good:
 		clocksource_acpi_pm.rating = 110;
 	}
 
-	register_clocksource(&clocksource_acpi_pm);
-
-	return 0;
+	return register_clocksource(&clocksource_acpi_pm);
 }
 
 module_init(init_acpi_pm_clocksource);
diff --git a/drivers/clocksource/cyclone.c b/drivers/clocksource/cyclone.c
index 168e78b..c873263 100644
--- a/drivers/clocksource/cyclone.c
+++ b/drivers/clocksource/cyclone.c
@@ -113,9 +113,7 @@ static int __init init_cyclone_clocksour
 	clocksource_cyclone.mult = clocksource_hz2mult(CYCLONE_TIMER_FREQ,
 						clocksource_cyclone.shift);
 
-	register_clocksource(&clocksource_cyclone);
-
-	return 0;
+	return register_clocksource(&clocksource_cyclone);
 }
 
 module_init(init_cyclone_clocksource);
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 5a3d6c3..f6dde25 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -297,7 +297,7 @@ static inline nsec_t cyc2ns_fixed_rem(st
 }
 
 /* used to install a new clocksource */
-void register_clocksource(struct clocksource*);
+int register_clocksource(struct clocksource*);
 void reselect_clocksource(void);
 struct clocksource* get_next_clocksource(void);
 
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index f606f1a..3f00051 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -139,22 +139,26 @@ static int is_registered_source(struct c
  * register_clocksource - Used to install new clocksources
  * @t:		clocksource to be registered
  */
-void register_clocksource(struct clocksource *c)
+int register_clocksource(struct clocksource *c)
 {
+	int ret = 0;
+
 	spin_lock(&clocksource_lock);
 #ifdef CONFIG_PARANOID_GENERIC_TIME
 	printk("register_clocksource: Registering %s\n", c->name);
 #endif
 	/* check if clocksource is already registered */
-	if (is_registered_source(c)) {
+	if (unlikely(is_registered_source(c))) {
 		printk("register_clocksource: Cannot register %s. Already registered!",
 		       c->name);
+		ret = -EBUSY;
 	} else {
 		list_add(&c->list, &clocksource_list);
 		/* select next clocksource */
 		next_clocksource = select_clocksource();
 	}
 	spin_unlock(&clocksource_lock);
+	return ret;
 }
 
 EXPORT_SYMBOL(register_clocksource);
diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index 4f0bdd4..3a8ea42 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -67,9 +67,7 @@ struct clocksource clocksource_jiffies =
 
 static int __init init_jiffies_clocksource(void)
 {
-	register_clocksource(&clocksource_jiffies);
-
-	return 0;
+	return register_clocksource(&clocksource_jiffies);
 }
 
 module_init(init_jiffies_clocksource);