Since it's likely that this interface would get used during bootup
I moved all the clocksource registration into the postcore initcall.
This also eliminated some clocksource shuffling during bootup.
Signed-Off-By: Daniel Walker <[email protected]>
---
arch/i386/kernel/hpet.c | 2 +-
arch/i386/kernel/i8253.c | 2 +-
arch/i386/kernel/tsc.c | 2 +-
drivers/clocksource/acpi_pm.c | 2 +-
drivers/clocksource/cyclone.c | 2 +-
drivers/clocksource/scx200_hrt.c | 2 +-
kernel/time/clocksource.c | 15 +--------------
kernel/time/jiffies.c | 2 +-
8 files changed, 8 insertions(+), 21 deletions(-)
Index: linux-2.6.17/arch/i386/kernel/hpet.c
===================================================================
--- linux-2.6.17.orig/arch/i386/kernel/hpet.c
+++ linux-2.6.17/arch/i386/kernel/hpet.c
@@ -64,4 +64,4 @@ static int __init init_hpet_clocksource(
return clocksource_register(&clocksource_hpet);
}
-module_init(init_hpet_clocksource);
+postcore_initcall(init_hpet_clocksource);
Index: linux-2.6.17/arch/i386/kernel/i8253.c
===================================================================
--- linux-2.6.17.orig/arch/i386/kernel/i8253.c
+++ linux-2.6.17/arch/i386/kernel/i8253.c
@@ -115,4 +115,4 @@ static int __init init_pit_clocksource(v
clocksource_pit.mult = clocksource_hz2mult(CLOCK_TICK_RATE, 20);
return clocksource_register(&clocksource_pit);
}
-module_init(init_pit_clocksource);
+postcore_initcall(init_pit_clocksource);
Index: linux-2.6.17/arch/i386/kernel/tsc.c
===================================================================
--- linux-2.6.17.orig/arch/i386/kernel/tsc.c
+++ linux-2.6.17/arch/i386/kernel/tsc.c
@@ -475,4 +475,4 @@ static int __init init_tsc_clocksource(v
return 0;
}
-module_init(init_tsc_clocksource);
+postcore_initcall(init_tsc_clocksource);
Index: linux-2.6.17/drivers/clocksource/acpi_pm.c
===================================================================
--- linux-2.6.17.orig/drivers/clocksource/acpi_pm.c
+++ linux-2.6.17/drivers/clocksource/acpi_pm.c
@@ -174,4 +174,4 @@ pm_good:
return clocksource_register(&clocksource_acpi_pm);
}
-module_init(init_acpi_pm_clocksource);
+postcore_initcall(init_acpi_pm_clocksource);
Index: linux-2.6.17/drivers/clocksource/cyclone.c
===================================================================
--- linux-2.6.17.orig/drivers/clocksource/cyclone.c
+++ linux-2.6.17/drivers/clocksource/cyclone.c
@@ -116,4 +116,4 @@ static int __init init_cyclone_clocksour
return clocksource_register(&clocksource_cyclone);
}
-module_init(init_cyclone_clocksource);
+postcore_initcall(init_cyclone_clocksource);
Index: linux-2.6.17/drivers/clocksource/scx200_hrt.c
===================================================================
--- linux-2.6.17.orig/drivers/clocksource/scx200_hrt.c
+++ linux-2.6.17/drivers/clocksource/scx200_hrt.c
@@ -94,7 +94,7 @@ static int __init init_hrt_clocksource(v
return clocksource_register(&cs_hrt);
}
-module_init(init_hrt_clocksource);
+postcore_initcall(init_hrt_clocksource);
MODULE_AUTHOR("Jim Cromie <[email protected]>");
MODULE_DESCRIPTION("clocksource on SCx200 HiRes Timer");
Index: linux-2.6.17/kernel/time/clocksource.c
===================================================================
--- linux-2.6.17.orig/kernel/time/clocksource.c
+++ linux-2.6.17/kernel/time/clocksource.c
@@ -50,19 +50,6 @@ static struct clocksource *next_clocksou
static LIST_HEAD(clocksource_list);
static DEFINE_SPINLOCK(clocksource_lock);
static char override_name[32];
-static int finished_booting;
-
-/* clocksource_done_booting - Called near the end of bootup
- *
- * Hack to avoid lots of clocksource churn at boot time
- */
-static int __init clocksource_done_booting(void)
-{
- finished_booting = 1;
- return 0;
-}
-
-late_initcall(clocksource_done_booting);
/**
* clocksource_get_next - Returns the selected clocksource
@@ -73,7 +60,7 @@ struct clocksource *clocksource_get_next
unsigned long flags;
spin_lock_irqsave(&clocksource_lock, flags);
- if (next_clocksource && finished_booting) {
+ if (next_clocksource) {
curr_clocksource = next_clocksource;
next_clocksource = NULL;
}
Index: linux-2.6.17/kernel/time/jiffies.c
===================================================================
--- linux-2.6.17.orig/kernel/time/jiffies.c
+++ linux-2.6.17/kernel/time/jiffies.c
@@ -70,4 +70,4 @@ static int __init init_jiffies_clocksour
return clocksource_register(&clocksource_jiffies);
}
-module_init(init_jiffies_clocksource);
+postcore_initcall(init_jiffies_clocksource);
--
Daniel Walker <[email protected]> writes:
> Index: linux-2.6.17/drivers/clocksource/acpi_pm.c
> ===================================================================
> --- linux-2.6.17.orig/drivers/clocksource/acpi_pm.c
> +++ linux-2.6.17/drivers/clocksource/acpi_pm.c
> @@ -174,4 +174,4 @@ pm_good:
> return clocksource_register(&clocksource_acpi_pm);
> }
>
> -module_init(init_acpi_pm_clocksource);
> +postcore_initcall(init_acpi_pm_clocksource);
Current code is assumeing DECLARE_PCI_FIXUP_EARLY() is called before
init_acpi_pm_clocksource().
We'll need to change it.
--
OGAWA Hirofumi <[email protected]>
On Sun, 2006-10-08 at 00:40 +0900, OGAWA Hirofumi wrote:
> Daniel Walker <[email protected]> writes:
>
> > Index: linux-2.6.17/drivers/clocksource/acpi_pm.c
> > ===================================================================
> > --- linux-2.6.17.orig/drivers/clocksource/acpi_pm.c
> > +++ linux-2.6.17/drivers/clocksource/acpi_pm.c
> > @@ -174,4 +174,4 @@ pm_good:
> > return clocksource_register(&clocksource_acpi_pm);
> > }
> >
> > -module_init(init_acpi_pm_clocksource);
> > +postcore_initcall(init_acpi_pm_clocksource);
>
> Current code is assumeing DECLARE_PCI_FIXUP_EARLY() is called before
> init_acpi_pm_clocksource().
>
> We'll need to change it.
We can add a call to clocksource_rating_change() inside
acpi_pm_need_workaround(), are there deeper dependencies?
Daniel
Daniel Walker <[email protected]> writes:
> On Sun, 2006-10-08 at 00:40 +0900, OGAWA Hirofumi wrote:
>> Daniel Walker <[email protected]> writes:
>>
>> > Index: linux-2.6.17/drivers/clocksource/acpi_pm.c
>> > ===================================================================
>> > --- linux-2.6.17.orig/drivers/clocksource/acpi_pm.c
>> > +++ linux-2.6.17/drivers/clocksource/acpi_pm.c
>> > @@ -174,4 +174,4 @@ pm_good:
>> > return clocksource_register(&clocksource_acpi_pm);
>> > }
>> >
>> > -module_init(init_acpi_pm_clocksource);
>> > +postcore_initcall(init_acpi_pm_clocksource);
>>
>> Current code is assumeing DECLARE_PCI_FIXUP_EARLY() is called before
>> init_acpi_pm_clocksource().
>>
>> We'll need to change it.
>
> We can add a call to clocksource_rating_change() inside
> acpi_pm_need_workaround(), are there deeper dependencies?
There is no deeper dependencies. If it's meaning
clocksource_reselect() in current git, it sounds good to me.
--
OGAWA Hirofumi <[email protected]>
OGAWA Hirofumi <[email protected]> writes:
> Daniel Walker <[email protected]> writes:
>
>> On Sun, 2006-10-08 at 00:40 +0900, OGAWA Hirofumi wrote:
>>> Daniel Walker <[email protected]> writes:
>>>
>>> > Index: linux-2.6.17/drivers/clocksource/acpi_pm.c
>>> > ===================================================================
>>> > --- linux-2.6.17.orig/drivers/clocksource/acpi_pm.c
>>> > +++ linux-2.6.17/drivers/clocksource/acpi_pm.c
>>> > @@ -174,4 +174,4 @@ pm_good:
>>> > return clocksource_register(&clocksource_acpi_pm);
>>> > }
>>> >
>>> > -module_init(init_acpi_pm_clocksource);
>>> > +postcore_initcall(init_acpi_pm_clocksource);
>>>
>>> Current code is assumeing DECLARE_PCI_FIXUP_EARLY() is called before
>>> init_acpi_pm_clocksource().
>>>
>>> We'll need to change it.
>>
>> We can add a call to clocksource_rating_change() inside
>> acpi_pm_need_workaround(), are there deeper dependencies?
>
> There is no deeper dependencies. If it's meaning
> clocksource_reselect() in current git, it sounds good to me.
Ah, I was forgetting why I didn't before. If it's a buggy pmtmr, we'll
get corrupted time until re-selecting the clocksource.
If anybody doesn't care this will be good with it. If not, we would
need to back to old one. IIRC, John did it.
--
OGAWA Hirofumi <[email protected]>
On Sun, 2006-10-08 at 03:41 +0900, OGAWA Hirofumi wrote:
> >>>
> >>> We'll need to change it.
> >>
> >> We can add a call to clocksource_rating_change() inside
> >> acpi_pm_need_workaround(), are there deeper dependencies?
> >
> > There is no deeper dependencies. If it's meaning
> > clocksource_reselect() in current git, it sounds good to me.
>
> Ah, I was forgetting why I didn't before. If it's a buggy pmtmr, we'll
> get corrupted time until re-selecting the clocksource.
>
> If anybody doesn't care this will be good with it. If not, we would
> need to back to old one. IIRC, John did it.
We could just reverse it, use the verified read function until we know
it's a good PM timer, then switch to the faster read function.
Daniel
Daniel Walker <[email protected]> writes:
> On Sun, 2006-10-08 at 03:41 +0900, OGAWA Hirofumi wrote:
>
>> >>>
>> >>> We'll need to change it.
>> >>
>> >> We can add a call to clocksource_rating_change() inside
>> >> acpi_pm_need_workaround(), are there deeper dependencies?
>> >
>> > There is no deeper dependencies. If it's meaning
>> > clocksource_reselect() in current git, it sounds good to me.
>>
>> Ah, I was forgetting why I didn't before. If it's a buggy pmtmr, we'll
>> get corrupted time until re-selecting the clocksource.
>>
>> If anybody doesn't care this will be good with it. If not, we would
>> need to back to old one. IIRC, John did it.
>
> We could just reverse it, use the verified read function until we know
> it's a good PM timer, then switch to the faster read function.
Yes, we did it in old timer code.
--
OGAWA Hirofumi <[email protected]>
On Fri, 2006-10-06 at 11:54 -0700, Daniel Walker wrote:
> plain text document attachment (clocksource_init_call.patch)
> Since it's likely that this interface would get used during bootup
> I moved all the clocksource registration into the postcore initcall.
So this is still somewhat of an open question: While timekeeping_init
runs quite early, and the timekeeping subsystem and its interface is
usable early in the boot process, currently not all the clocksources are
available as early. This is by design, as there may be clocksource
driver modules loaded later on in the boot process, so we don't want to
require it early.
So, the question becomes: Do we want to start using arch specific
clocksources as early as possible, with the potential that we'll replace
it when a better one shows up later? It would allow for finer grained
timekeeping early in boot, which sounds nice, but I'm not sure how great
the real need is for that.
> This also eliminated some clocksource shuffling during bootup.
Actually, I'm not sure I see this. Which shuffling does it avoid?
I suspect it might actually cause more shuffling, as some clocksources
(well, just the TSC, really.. its such a pain...) are not disqualified
until later because we don't know if the system will enter C3, or change
cpufreq, etc.. By waiting longer, we increase the chance that those
disqualifying actions will occur before we install it.
thanks
-john
On Mon, 2006-10-09 at 11:50 -0700, john stultz wrote:
> On Fri, 2006-10-06 at 11:54 -0700, Daniel Walker wrote:
> > plain text document attachment (clocksource_init_call.patch)
> > Since it's likely that this interface would get used during bootup
> > I moved all the clocksource registration into the postcore initcall.
>
> So this is still somewhat of an open question: While timekeeping_init
> runs quite early, and the timekeeping subsystem and its interface is
> usable early in the boot process, currently not all the clocksources are
> available as early. This is by design, as there may be clocksource
> driver modules loaded later on in the boot process, so we don't want to
> require it early.
>
> So, the question becomes: Do we want to start using arch specific
> clocksources as early as possible, with the potential that we'll replace
> it when a better one shows up later? It would allow for finer grained
> timekeeping early in boot, which sounds nice, but I'm not sure how great
> the real need is for that.
My main motivation is that I'm assuming other uses of the interface will
exist. Then anything that uses the interface after postcore will avoid
switching clocks later.
If I for instance, just return clocksource_jiffies until the system is
fully booted then any thing that got a clock early, even during part of
device_initcall, would end up switching to a new clock when boot up
finished. I think that was acceptable when just timekeeping might have
been using the interface, but I don't know that it would scale well from
1 to 2 to 5 users of the interface. Then you would have several clock
switches happened after boot up .
> > This also eliminated some clocksource shuffling during bootup.
>
> Actually, I'm not sure I see this. Which shuffling does it avoid?
The shuffling that you commented about in kernel/time/clocksource.c
which was the reason for the "finished_booting" flag, and related code.
> I suspect it might actually cause more shuffling, as some clocksources
> (well, just the TSC, really.. its such a pain...) are not disqualified
> until later because we don't know if the system will enter C3, or change
> cpufreq, etc.. By waiting longer, we increase the chance that those
> disqualifying actions will occur before we install it.
This is something I've struggled with, but it's still and issue with the
current code to a lesser degree. Like cpufreq isn't likely to be used
during bootup, but acpi sleep states might be..
The current code can be classified as avoiding shuffle, but not
eliminating it. Like with the acpi_pm timer, I was thinking I'd just put
it back into device_initcall and assume shuffling might happen in rare
cases.
Daniel
On Mon, 2006-10-09 at 12:24 -0700, Daniel Walker wrote:
> > So, the question becomes: Do we want to start using arch specific
> > clocksources as early as possible, with the potential that we'll replace
> > it when a better one shows up later? It would allow for finer grained
> > timekeeping early in boot, which sounds nice, but I'm not sure how great
> > the real need is for that.
>
> My main motivation is that I'm assuming other uses of the interface will
> exist. Then anything that uses the interface after postcore will avoid
> switching clocks later.
What does it matter, when clocks switch? That's a one time event.
The boot code is different and there is nothing which requires the
availability of that interface in the early boot process.
> If I for instance, just return clocksource_jiffies until the system is
> fully booted then any thing that got a clock early, even during part of
> device_initcall, would end up switching to a new clock when boot up
> finished. I think that was acceptable when just timekeeping might have
> been using the interface, but I don't know that it would scale well from
> 1 to 2 to 5 users of the interface. Then you would have several clock
> switches happened after boot up .
What are the 5 users of that interface ?
There is one existing user, one artifical you try to create for
sched_clock and some handwaving about instrumentation. Even when we have
5 users, then the switching of clocks does not matter anything. This
happens the same way, when I load the highest rated clock source as a
module during init.
> > I suspect it might actually cause more shuffling, as some clocksources
> > (well, just the TSC, really.. its such a pain...) are not disqualified
> > until later because we don't know if the system will enter C3, or change
> > cpufreq, etc.. By waiting longer, we increase the chance that those
> > disqualifying actions will occur before we install it.
>
> This is something I've struggled with, but it's still and issue with the
> current code to a lesser degree. Like cpufreq isn't likely to be used
> during bootup, but acpi sleep states might be..
Lesser degree ? You have an issue, which was not there before you made
the changes. This is simply regression and the ignoring of that
regression is wilful breakage.
> The current code can be classified as avoiding shuffle, but not
> eliminating it. Like with the acpi_pm timer, I was thinking I'd just put
> it back into device_initcall and assume shuffling might happen in rare
> cases.
Great. We install TSC in early boot and pmtimer in late boot. Now we
have the fun of unsynced TSCs and time going backwards again, until we
get pmtimer loaded. I have explained it to you already, but that is the
reason why that stuff was moved into late boot.
Also you have not yet once given an explanation other than the "magic
churn", why this is necessary. There is no churn. The clocksource layer
is designed to allow the replacement of clock sources and it does not
hurt at all. It does also not matter if there are 1 ore 100 users.
tglx