* [PATCH] clocksource: Load the ACPI PM clocksource asynchronously @ 2012-01-31 4:51 Arjan van de Ven 2012-02-02 2:40 ` john stultz 2012-04-08 5:58 ` Michael Witten 0 siblings, 2 replies; 6+ messages in thread From: Arjan van de Ven @ 2012-01-31 4:51 UTC (permalink / raw) To: linux-kernel; +Cc: John Stultz, Thomas Gleixner, Len Brown, arjanvandeven >From ef4b0d23878277dfd75828f13353e605aae2937f Mon Sep 17 00:00:00 2001 From: Arjan van de Ven <arjan@linux.intel.com> Date: Mon, 30 Jan 2012 20:23:30 -0800 Subject: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously The ACPI clocksource takes quite some time to initialize, and this increases the boot time of the kernel for a double digit percentage. This while almost all modern systems will be using the HPET already anyway. This patch turns the clocksource loading into an asynchronous operation; which means it won't hold up the boot while still becoming available normally. To make this work well, an udelay() had to be turned into an usleep_range() so that on UP systems, we yield the CPU to regular boot tasks instead of spinning. CC: John Stultz <johnstul@us.ibm.com> CC: Thomas Gleixner <tglx@linutronix.de> CC: Len Brown <lenb@kernel.org> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com> --- drivers/clocksource/acpi_pm.c | 24 ++++++++++++++++-------- 1 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 6b5cf02..82e8820 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -23,6 +23,7 @@ #include <linux/init.h> #include <linux/pci.h> #include <linux/delay.h> +#include <linux/async.h> #include <asm/io.h> /* @@ -179,17 +180,15 @@ static int verify_pmtmr_rate(void) /* Number of reads we try to get two different values */ #define ACPI_PM_READ_CHECKS 10000 -static int __init init_acpi_pm_clocksource(void) +static void __init acpi_pm_clocksource_async(void *unused, async_cookie_t cookie) { cycle_t value1, value2; unsigned int i, j = 0; - if (!pmtmr_ioport) - return -ENODEV; /* "verify" this timing source: */ for (j = 0; j < ACPI_PM_MONOTONICITY_CHECKS; j++) { - udelay(100 * j); + usleep_range(100 * j, 100 * j + 100); value1 = clocksource_acpi_pm.read(&clocksource_acpi_pm); for (i = 0; i < ACPI_PM_READ_CHECKS; i++) { value2 = clocksource_acpi_pm.read(&clocksource_acpi_pm); @@ -203,25 +202,34 @@ static int __init init_acpi_pm_clocksource(void) " 0x%#llx, 0x%#llx - aborting.\n", value1, value2); pmtmr_ioport = 0; - return -EINVAL; + return; } if (i == ACPI_PM_READ_CHECKS) { printk(KERN_INFO "PM-Timer failed consistency check " " (0x%#llx) - aborting.\n", value1); pmtmr_ioport = 0; - return -ENODEV; + return; } } if (verify_pmtmr_rate() != 0){ pmtmr_ioport = 0; - return -ENODEV; + return; } - return clocksource_register_hz(&clocksource_acpi_pm, + clocksource_register_hz(&clocksource_acpi_pm, PMTMR_TICKS_PER_SEC); } +static int __init init_acpi_pm_clocksource(void) +{ + if (!pmtmr_ioport) + return -ENODEV; + + async_schedule(acpi_pm_clocksource_async, NULL); + return 0; +} + /* We use fs_initcall because we want the PCI fixups to have run * but we still need to load before device_initcall */ -- 1.7.6.4 -- Arjan van de Ven Intel Open Source Technology Centre For development, discussion and tips for power savings, visit http://www.lesswatts.org ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously 2012-01-31 4:51 [PATCH] clocksource: Load the ACPI PM clocksource asynchronously Arjan van de Ven @ 2012-02-02 2:40 ` john stultz 2012-04-08 5:58 ` Michael Witten 1 sibling, 0 replies; 6+ messages in thread From: john stultz @ 2012-02-02 2:40 UTC (permalink / raw) To: Arjan van de Ven; +Cc: linux-kernel, Thomas Gleixner, Len Brown, arjanvandeven On Mon, 2012-01-30 at 20:51 -0800, Arjan van de Ven wrote: > From ef4b0d23878277dfd75828f13353e605aae2937f Mon Sep 17 00:00:00 2001 > From: Arjan van de Ven <arjan@linux.intel.com> > Date: Mon, 30 Jan 2012 20:23:30 -0800 > Subject: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously > > The ACPI clocksource takes quite some time to initialize, > and this increases the boot time of the kernel for a > double digit percentage. This while almost all modern > systems will be using the HPET already anyway. > > This patch turns the clocksource loading into an asynchronous > operation; which means it won't hold up the boot while > still becoming available normally. > > To make this work well, an udelay() had to be turned into an > usleep_range() so that on UP systems, we yield the CPU to > regular boot tasks instead of spinning. > > CC: John Stultz <johnstul@us.ibm.com> > CC: Thomas Gleixner <tglx@linutronix.de> > CC: Len Brown <lenb@kernel.org> > > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com> Looks good to me. I've gone ahead and queued it thanks -john ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously 2012-01-31 4:51 [PATCH] clocksource: Load the ACPI PM clocksource asynchronously Arjan van de Ven 2012-02-02 2:40 ` john stultz @ 2012-04-08 5:58 ` Michael Witten [not found] ` <CADyApD2UZebJd_nSJHAWkjz=PT4_DZfnSr38geqUb1kvJzKi-g@mail.gmail.com> 1 sibling, 1 reply; 6+ messages in thread From: Michael Witten @ 2012-04-08 5:58 UTC (permalink / raw) To: Arjan van de Ven Cc: John Stultz, Thomas Gleixner, Len Brown, Arjan van de Ven, linux-kernel On Mon, 30 Jan 2012 20:51:20 -0800, Arjan van de Ven wrote: > ... > > The ACPI clocksource takes quite some time to initialize, > and this increases the boot time of the kernel for a > double digit percentage. This while almost all modern > systems will be using the HPET already anyway. > > This patch turns the clocksource loading into an asynchronous > operation; which means it won't hold up the boot while > still becoming available normally. > > To make this work well, an udelay() had to be turned into an > usleep_range() so that on UP systems, we yield the CPU to > regular boot tasks instead of spinning. > > ... This patch became the following commit: commit b519508298e0292e1771eecf14aaf67755adc39d Author: Arjan van de Ven <arjan@linux.intel.com> AuthorDate: Mon Jan 30 20:23:30 2012 -0800 Commit: John Stultz <john.stultz@linaro.org> CommitDate: Wed Feb 1 18:39:46 2012 -0800 to which I bisected as the culprit for very strange load balance behavior on my machine. With this patch in place, my CPU is constantly being pegged at 100% (and my CPU monitor sometimes registers NaN%), regardless of the active governor and under conditions when my computer would normally be idling quite placidly. Reverting this commit does indeed remove the problem from previously problematic builds. Sincerely, Michael Witten Some probably useless information: * Dell Latitude D810 (from about 2005/2006) * BIOS Version A05 * /proc/cpuinfo: processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 13 model name : Intel(R) Pentium(R) M processor 2.13GHz stepping : 8 microcode : 0x20 cpu MHz : 800.000 cache size : 2048 KB fdiv_bug : no hlt_bug : no f00f_bug : no coma_bug : no fpu : yes fpu_exception : yes cpuid level : 2 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov clflush dts acpi mmx fxsr sse sse2 ss tm pbe nx bts est tm2 bogomips : 1596.47 clflush size : 64 cache_alignment : 64 address sizes : 32 bits physical, 32 bits virtual power management: ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CADyApD2UZebJd_nSJHAWkjz=PT4_DZfnSr38geqUb1kvJzKi-g@mail.gmail.com>]
* Re: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously [not found] ` <CADyApD2UZebJd_nSJHAWkjz=PT4_DZfnSr38geqUb1kvJzKi-g@mail.gmail.com> @ 2012-04-11 20:04 ` Michael Witten 2012-04-11 21:13 ` Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Michael Witten @ 2012-04-11 20:04 UTC (permalink / raw) To: Arjan van de Ven Cc: Arjan van de Ven, John Stultz, Thomas Gleixner, Len Brown, linux-kernel On Sun, 8 Apr 2012 19:20:22 -0700, Arjan van de Ven wrote: >> to which I bisected as the culprit for very strange load balance >> behavior on my machine. >> >> With this patch in place, my CPU is constantly being pegged at 100% >> (and my CPU monitor sometimes registers NaN%), regardless of the >> active governor and under conditions when my computer would normally >> be idling quite placidly. >> >> Reverting this commit does indeed remove the problem from previously >> problematic builds. >> >> > any chance of a dmesg? (or even better, a diff of the dmesg from > abefore and fter) Doh! I should have thought of that. The dmesg output for a kernel built from your patch's commit yields this interesting line: PM-Timer running at invalid rate: 113% of normal - aborting. That certainly sounds unsavory! Here is some other, probably uninteresting context: --- good-dmesg.txt +++ bad-dmesg.txt [...] Initializing cgroup subsys cpu -Linux version 3.3.0-rc1-good-12d6d41276def096cb3f7dc36f438db9ed6a0a8d+ [...] +Linux version 3.3.0-rc1-bad-b519508298e0292e1771eecf14aaf67755adc39d+ [...] [...] Fast TSC calibration using PIT -Detected 798.093 MHz processor. +Detected 798.071 MHz processor. -Calibrating delay loop (skipped), value calculated using timer frequency.. 1596.18 BogoMIPS (lpj=798093) +Calibrating delay loop (skipped), value calculated using timer frequency.. 1596.14 BogoMIPS (lpj=798071) pid_max: default: 32768 minimum: 301 Mount-cache hash table entries: 512 Initializing cgroup subsys cpuacct [...] system 00:0c: Plug and Play ACPI device, IDs PNP0c01 (active) pnp: PnP ACPI: found 13 devices ACPI: ACPI bus type pnp unregistered -Switching to clocksource acpi_pm [...] +PM-Timer running at invalid rate: 113% of normal - aborting. Sincerely, Michael Witten P.S. I'm terribly sorry for the delayed reply; Gmail somehow managed to throw away this thread entirely (actually, I probably threw it away, but I'm content to blame my tools; after all, they can't deny it!). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clocksource: Load the ACPI PM clocksource asynchronously 2012-04-11 20:04 ` Michael Witten @ 2012-04-11 21:13 ` Thomas Gleixner 2012-04-11 22:15 ` [tip:timers/urgent] Revert "clocksource: Load the ACPI PM clocksource asynchronously" tip-bot for Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Thomas Gleixner @ 2012-04-11 21:13 UTC (permalink / raw) To: Michael Witten Cc: Arjan van de Ven, Arjan van de Ven, John Stultz, Len Brown, linux-kernel On Wed, 11 Apr 2012, Michael Witten wrote: > On Sun, 8 Apr 2012 19:20:22 -0700, Arjan van de Ven wrote: > > >> to which I bisected as the culprit for very strange load balance > >> behavior on my machine. > >> > >> With this patch in place, my CPU is constantly being pegged at 100% > >> (and my CPU monitor sometimes registers NaN%), regardless of the > >> active governor and under conditions when my computer would normally > >> be idling quite placidly. > >> > >> Reverting this commit does indeed remove the problem from previously > >> problematic builds. > >> > >> > > any chance of a dmesg? (or even better, a diff of the dmesg from > > abefore and fter) > > Doh! I should have thought of that. > > The dmesg output for a kernel built from your patch's commit yields > this interesting line: > > PM-Timer running at invalid rate: 113% of normal - aborting. Duh. The original code does this with interrupts disabled and not from the context of some random worker thread. Aside of that, this will blow up if something else touches the CTC channel 2, which is possible as this code runs async to other init stuff. Arjan, did you ever verify that patch extensivly on hardware which relies on the PM-Timer? I doubt it. :( There is no other choice than reverting it. Thanks, tglx ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:timers/urgent] Revert "clocksource: Load the ACPI PM clocksource asynchronously" 2012-04-11 21:13 ` Thomas Gleixner @ 2012-04-11 22:15 ` tip-bot for Thomas Gleixner 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Thomas Gleixner @ 2012-04-11 22:15 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, arjanvandeven, hpa, mingo, johnstul, arjan, mfwitten, lenb, tglx Commit-ID: d48fc63f6f3f485ed5aa9cf019d8e8e3a7d10263 Gitweb: http://git.kernel.org/tip/d48fc63f6f3f485ed5aa9cf019d8e8e3a7d10263 Author: Thomas Gleixner <tglx@linutronix.de> AuthorDate: Wed, 11 Apr 2012 23:49:16 +0200 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 12 Apr 2012 00:05:05 +0200 Revert "clocksource: Load the ACPI PM clocksource asynchronously" This reverts commit b519508298e0292e1771eecf14aaf67755adc39d. The reason for this revert is that making the frequency verification preemptible and interruptible is not working reliably. Michaels machine failed to use PM-timer with the message: PM-Timer running at invalid rate: 113% of normal - aborting. That's not a surprise as the frequency verification does rely on interrupts being disabled. With a async scheduled thread there is no guarantee to achieve the same result. Also some driver might fiddle with the CTC channel 2 during the verification period, which makes the result even more random and unpredictable. This can be solved by using the same mechanism as we use in the deferred TSC validation code, but that only will work if we verified a working HPET _BEFORE_ trying to do the PM-Timer lazy validation. So for now reverting is the safe option. Bisected-by: Michael Witten <mfwitten@gmail.com> Cc: Arjan van de Ven <arjanvandeven@gmail.com> Cc: Arjan van de Ven <arjan@infradead.org> Cc: John Stultz <johnstul@us.ibm.com> Cc: Len Brown <lenb@kernel.org> LKML-Reference: <alpine.LFD.2.02.1204112303270.2542@ionos> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/clocksource/acpi_pm.c | 24 ++++++++---------------- 1 files changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 82e8820..6b5cf02 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -23,7 +23,6 @@ #include <linux/init.h> #include <linux/pci.h> #include <linux/delay.h> -#include <linux/async.h> #include <asm/io.h> /* @@ -180,15 +179,17 @@ static int verify_pmtmr_rate(void) /* Number of reads we try to get two different values */ #define ACPI_PM_READ_CHECKS 10000 -static void __init acpi_pm_clocksource_async(void *unused, async_cookie_t cookie) +static int __init init_acpi_pm_clocksource(void) { cycle_t value1, value2; unsigned int i, j = 0; + if (!pmtmr_ioport) + return -ENODEV; /* "verify" this timing source: */ for (j = 0; j < ACPI_PM_MONOTONICITY_CHECKS; j++) { - usleep_range(100 * j, 100 * j + 100); + udelay(100 * j); value1 = clocksource_acpi_pm.read(&clocksource_acpi_pm); for (i = 0; i < ACPI_PM_READ_CHECKS; i++) { value2 = clocksource_acpi_pm.read(&clocksource_acpi_pm); @@ -202,34 +203,25 @@ static void __init acpi_pm_clocksource_async(void *unused, async_cookie_t cookie " 0x%#llx, 0x%#llx - aborting.\n", value1, value2); pmtmr_ioport = 0; - return; + return -EINVAL; } if (i == ACPI_PM_READ_CHECKS) { printk(KERN_INFO "PM-Timer failed consistency check " " (0x%#llx) - aborting.\n", value1); pmtmr_ioport = 0; - return; + return -ENODEV; } } if (verify_pmtmr_rate() != 0){ pmtmr_ioport = 0; - return; + return -ENODEV; } - clocksource_register_hz(&clocksource_acpi_pm, + return clocksource_register_hz(&clocksource_acpi_pm, PMTMR_TICKS_PER_SEC); } -static int __init init_acpi_pm_clocksource(void) -{ - if (!pmtmr_ioport) - return -ENODEV; - - async_schedule(acpi_pm_clocksource_async, NULL); - return 0; -} - /* We use fs_initcall because we want the PCI fixups to have run * but we still need to load before device_initcall */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-11 22:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-31 4:51 [PATCH] clocksource: Load the ACPI PM clocksource asynchronously Arjan van de Ven
2012-02-02 2:40 ` john stultz
2012-04-08 5:58 ` Michael Witten
[not found] ` <CADyApD2UZebJd_nSJHAWkjz=PT4_DZfnSr38geqUb1kvJzKi-g@mail.gmail.com>
2012-04-11 20:04 ` Michael Witten
2012-04-11 21:13 ` Thomas Gleixner
2012-04-11 22:15 ` [tip:timers/urgent] Revert "clocksource: Load the ACPI PM clocksource asynchronously" tip-bot for Thomas Gleixner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).