| Message ID | 1374189690-10810-6-git-send-email-sboyd@codeaurora.org (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
Hi Stephen, On Fri, Jul 19, 2013 at 12:21:18AM +0100, Stephen Boyd wrote: > Register with the generic sched_clock framework now that it > supports 64 bits. This fixes two problems with the current > sched_clock support for machines using the architected timers. > First off, we don't subtract the start value from subsequent > sched_clock calls so we can potentially start off with > sched_clock returning gigantic numbers. Second, there is no > support for suspend/resume handling so problems such as discussed > in 6a4dae5 (ARM: 7565/1: sched: stop sched_clock() during > suspend, 2012-10-23) can happen without this patch. Finally, it > allows us to move the sched_clock setup into drivers clocksource > out of the arch ports. Sorry, this one slipped through the cracks. > diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c > index 221f07b..1791f12 100644 > --- a/arch/arm/kernel/arch_timer.c > +++ b/arch/arm/kernel/arch_timer.c > @@ -11,7 +11,6 @@ > #include <linux/init.h> > #include <linux/types.h> > #include <linux/errno.h> > -#include <linux/sched_clock.h> > > #include <asm/delay.h> > > @@ -22,13 +21,6 @@ static unsigned long arch_timer_read_counter_long(void) > return arch_timer_read_counter(); > } > > -static u32 sched_clock_mult __read_mostly; > - > -static unsigned long long notrace arch_timer_sched_clock(void) > -{ > - return arch_timer_read_counter() * sched_clock_mult; > -} > - > static struct delay_timer arch_delay_timer; > > static void __init arch_timer_delay_timer_register(void) > @@ -48,11 +40,5 @@ int __init arch_timer_arch_init(void) > > arch_timer_delay_timer_register(); > > - /* Cache the sched_clock multiplier to save a divide in the hot path. */ > - sched_clock_mult = NSEC_PER_SEC / arch_timer_rate; > - sched_clock_func = arch_timer_sched_clock; > - pr_info("sched_clock: ARM arch timer >56 bits at %ukHz, resolution %uns\n", > - arch_timer_rate / 1000, sched_clock_mult); > - > return 0; > } > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 9737e97..e32b471 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -14,6 +14,7 @@ config ARM64 > select GENERIC_IOMAP > select GENERIC_IRQ_PROBE > select GENERIC_IRQ_SHOW > + select GENERIC_SCHED_CLOCK > select GENERIC_SMP_IDLE_THREAD > select GENERIC_TIME_VSYSCALL > select HARDIRQS_SW_RESEND > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c > index 03dc371..29c39d5 100644 > --- a/arch/arm64/kernel/time.c > +++ b/arch/arm64/kernel/time.c > @@ -61,13 +61,6 @@ unsigned long profile_pc(struct pt_regs *regs) > EXPORT_SYMBOL(profile_pc); > #endif > > -static u64 sched_clock_mult __read_mostly; > - > -unsigned long long notrace sched_clock(void) > -{ > - return arch_timer_read_counter() * sched_clock_mult; > -} > - > void __init time_init(void) > { > u32 arch_timer_rate; > @@ -78,9 +71,6 @@ void __init time_init(void) > if (!arch_timer_rate) > panic("Unable to initialise architected timer.\n"); > > - /* Cache the sched_clock multiplier to save a divide in the hot path. */ > - sched_clock_mult = NSEC_PER_SEC / arch_timer_rate; > - > /* Calibrate the delay loop directly */ > lpj_fine = arch_timer_rate / HZ; > } > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index 053d846..2facf14 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -17,6 +17,7 @@ > #include <linux/interrupt.h> > #include <linux/of_irq.h> > #include <linux/io.h> > +#include <linux/sched_clock.h> > > #include <asm/arch_timer.h> > #include <asm/virt.h> > @@ -281,6 +282,9 @@ static int __init arch_timer_register(void) > timecounter_init(&timecounter, &cyclecounter, > arch_counter_get_cntvct()); > > + /* 56 bits minimum, so we assume worst case rollover */ > + sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate); We've got the same assumption elsewhere in the kernel (e.g. vdso) but at some point we should probably deal with >56 bits in case somebody makes a high-frequency timer with more significant bits. However, I think this patch is going in the right direction: Acked-by: Will Deacon <will.deacon@arm.com> Will -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stephen Boyd <sboyd@codeaurora.org> writes: > Register with the generic sched_clock framework now that it > supports 64 bits. This fixes two problems with the current > sched_clock support for machines using the architected timers. > First off, we don't subtract the start value from subsequent > sched_clock calls so we can potentially start off with > sched_clock returning gigantic numbers. Second, there is no > support for suspend/resume handling so problems such as discussed > in 6a4dae5 (ARM: 7565/1: sched: stop sched_clock() during > suspend, 2012-10-23) can happen without this patch. Finally, it > allows us to move the sched_clock setup into drivers clocksource > out of the arch ports. > > Cc: Christopher Covington <cov@codeaurora.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> A boot failure on Exynos5/Arndale showed up in next-20131014[1], and a subsequent bisect has fingered this patch as the culprit. I haven't had a chance to debug this any further, but wanted to share in case someone else can debug. The console log is below, but don't think there is much useful there as it shows nothing after the 'Starting kernel ...' from u-boot. Kevin [1] http://lists.linaro.org/pipermail/kernel-build-reports/2013-October/000651.html Connected to arndale console [channel connected] (~$quit to exit) (user:khilman) is already connected ~$hardreset Command(arndale console)> hardreset (user:khilman) Reboot arndale Reboot: arndale ; phidget 2 0 : off, sleep, on U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250 CPU: Exynos5250@1000MHz Board: for ARNDALE5250 I2C: ready DRAM: 2 GiB WARNING: Caches not enabled Checking Boot Mode ... SDMMC MMC: EXYNOS DWMMC: 0, EXYNOS DWMMC: 1, EXYNOS DWMMC: 2 In: serial Out: serial Err: serial Net: No ethernet found. (Re)start USB... USB0: USB EHCI 1.00 scanning bus 0 for devices... 4 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 1 Ethernet Device(s) found Hit any key to stop autoboot: 3 0 ARNDALE5250 # version version U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250 arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.2-1ubuntu1) 4.7.2 GNU ld (GNU Binutils for Ubuntu) 2.22.90.20120919 ARNDALE5250 # setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4 setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4 ARNDALE5250 # setenv netargs 'setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}::::192.168.1.254:none' setenv netargs 'setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}::::192.168.1.254:none' ARNDALE5250 # if test -n ${initenv}; then run initenv; fi if test -n ${initenv}; then run initenv; fi ARNDALE5250 # if test -n ${preboot}; then run preboot; fi if test -n ${preboot}; then run preboot; fi (Re)start USB... USB0: USB EHCI 1.00 scanning bus 0 for devices... 4 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 1 Ethernet Device(s) found ARNDALE5250 #setenv autoload no; setenv autoboot no setenv autoload no; setenv autoboot no ARNDALE5250 # dhcp dhcp Waiting for Ethernet connection... done. BOOTP broadcast 1 DHCP client bound to address 192.168.1.171 ARNDALE5250 # setenv serverip 192.168.1.2 setenv serverip 192.168.1.2 ARNDALE5250 # if test -n ${netargs}; then run netargs; fi if test -n ${netargs}; then run netargs; fi ARNDALE5250 # tftp 0x40800000 tmp/arndale-lNb7OG/zImage tftp 0x40800000 tmp/arndale-lNb7OG/zImage Waiting for Ethernet connection... done. Using asx0 device TFTP from server 192.168.1.2; our IP address is 192.168.1.171 Filename 'tmp/arndale-lNb7OG/zImage'. Load address: 0x40800000 Loading: *################################################################# ################################################################# ####################################################### done Bytes transferred = 2709912 (295998 hex) ARNDALE5250 # tftp 0x407c0000 tmp/arndale-lNb7OG/exynos5250-arndale.dtb tftp 0x407c0000 tmp/arndale-lNb7OG/exynos5250-arndale.dtb Waiting for Ethernet connection... done. Using asx0 device TFTP from server 192.168.1.2; our IP address is 192.168.1.171 Filename 'tmp/arndale-lNb7OG/exynos5250-arndale.dtb'. Load address: 0x407c0000 Loading: *### done Bytes transferred = 32949 (80b5 hex) ARNDALE5250 # printenv bootargs printenv bootargs bootargs=console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4 ip=192.168.1.171:192.168.1.2:192.168.1.254:255.255.255.0::::192.168.1.254:none ARNDALE5250 # bootz 0x40800000 - 0x407c0000 bootz 0x40800000 - 0x407c0000 ## Flattened Device Tree blob at 407c0000 Booting using the fdt blob at 0x407c0000 Using Device Tree in place at 407c0000, end 407cb0b4 Starting kernel ... ~$off # PYBOOT: Exception: kernel: ERROR: did not start booting. # PYBOOT: Time: 33.50 seconds. # PYBOOT: Result: FAIL -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c index 221f07b..1791f12 100644 --- a/arch/arm/kernel/arch_timer.c +++ b/arch/arm/kernel/arch_timer.c @@ -11,7 +11,6 @@ #include <linux/init.h> #include <linux/types.h> #include <linux/errno.h> -#include <linux/sched_clock.h> #include <asm/delay.h> @@ -22,13 +21,6 @@ static unsigned long arch_timer_read_counter_long(void) return arch_timer_read_counter(); } -static u32 sched_clock_mult __read_mostly; - -static unsigned long long notrace arch_timer_sched_clock(void) -{ - return arch_timer_read_counter() * sched_clock_mult; -} - static struct delay_timer arch_delay_timer; static void __init arch_timer_delay_timer_register(void) @@ -48,11 +40,5 @@ int __init arch_timer_arch_init(void) arch_timer_delay_timer_register(); - /* Cache the sched_clock multiplier to save a divide in the hot path. */ - sched_clock_mult = NSEC_PER_SEC / arch_timer_rate; - sched_clock_func = arch_timer_sched_clock; - pr_info("sched_clock: ARM arch timer >56 bits at %ukHz, resolution %uns\n", - arch_timer_rate / 1000, sched_clock_mult); - return 0; } diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 9737e97..e32b471 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -14,6 +14,7 @@ config ARM64 select GENERIC_IOMAP select GENERIC_IRQ_PROBE select GENERIC_IRQ_SHOW + select GENERIC_SCHED_CLOCK select GENERIC_SMP_IDLE_THREAD select GENERIC_TIME_VSYSCALL select HARDIRQS_SW_RESEND diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c index 03dc371..29c39d5 100644 --- a/arch/arm64/kernel/time.c +++ b/arch/arm64/kernel/time.c @@ -61,13 +61,6 @@ unsigned long profile_pc(struct pt_regs *regs) EXPORT_SYMBOL(profile_pc); #endif -static u64 sched_clock_mult __read_mostly; - -unsigned long long notrace sched_clock(void) -{ - return arch_timer_read_counter() * sched_clock_mult; -} - void __init time_init(void) { u32 arch_timer_rate; @@ -78,9 +71,6 @@ void __init time_init(void) if (!arch_timer_rate) panic("Unable to initialise architected timer.\n"); - /* Cache the sched_clock multiplier to save a divide in the hot path. */ - sched_clock_mult = NSEC_PER_SEC / arch_timer_rate; - /* Calibrate the delay loop directly */ lpj_fine = arch_timer_rate / HZ; } diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 053d846..2facf14 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -17,6 +17,7 @@ #include <linux/interrupt.h> #include <linux/of_irq.h> #include <linux/io.h> +#include <linux/sched_clock.h> #include <asm/arch_timer.h> #include <asm/virt.h> @@ -281,6 +282,9 @@ static int __init arch_timer_register(void) timecounter_init(&timecounter, &cyclecounter, arch_counter_get_cntvct()); + /* 56 bits minimum, so we assume worst case rollover */ + sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate); + if (arch_timer_use_virtual) { ppi = arch_timer_ppi[VIRT_PPI]; err = request_percpu_irq(ppi, arch_timer_handler_virt,
Register with the generic sched_clock framework now that it supports 64 bits. This fixes two problems with the current sched_clock support for machines using the architected timers. First off, we don't subtract the start value from subsequent sched_clock calls so we can potentially start off with sched_clock returning gigantic numbers. Second, there is no support for suspend/resume handling so problems such as discussed in 6a4dae5 (ARM: 7565/1: sched: stop sched_clock() during suspend, 2012-10-23) can happen without this patch. Finally, it allows us to move the sched_clock setup into drivers clocksource out of the arch ports. Cc: Christopher Covington <cov@codeaurora.org> Cc: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- arch/arm/kernel/arch_timer.c | 14 -------------- arch/arm64/Kconfig | 1 + arch/arm64/kernel/time.c | 10 ---------- drivers/clocksource/arm_arch_timer.c | 4 ++++ 4 files changed, 5 insertions(+), 24 deletions(-)