From b8f8c3cf0a4ac0632ec3f0e15e9dc0c29de917af Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 18 Jul 2008 17:27:28 +0200 Subject: nohz: prevent tick stop outside of the idle loop Jack Ren and Eric Miao tracked down the following long standing problem in the NOHZ code: scheduler switch to idle task enable interrupts Window starts here ----> interrupt happens (does not set NEED_RESCHED) irq_exit() stops the tick ----> interrupt happens (does set NEED_RESCHED) return from schedule() cpu_idle(): preempt_disable(); Window ends here The interrupts can happen at any point inside the race window. The first interrupt stops the tick, the second one causes the scheduler to rerun and switch away from idle again and we end up with the tick disabled. The fact that it needs two interrupts where the first one does not set NEED_RESCHED and the second one does made the bug obscure and extremly hard to reproduce and analyse. Kudos to Jack and Eric. Solution: Limit the NOHZ functionality to the idle loop to make sure that we can not run into such a situation ever again. cpu_idle() { preempt_disable(); while(1) { tick_nohz_stop_sched_tick(1); <- tell NOHZ code that we are in the idle loop while (!need_resched()) halt(); tick_nohz_restart_sched_tick(); <- disables NOHZ mode preempt_enable_no_resched(); schedule(); preempt_disable(); } } In hindsight we should have done this forever, but ... /me grabs a large brown paperbag. Debugged-by: Jack Ren , Debugged-by: eric miao Signed-off-by: Thomas Gleixner --- arch/sh/kernel/process_32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/sh/kernel/process_32.c') diff --git a/arch/sh/kernel/process_32.c b/arch/sh/kernel/process_32.c index b98e37a1f54..921892c351d 100644 --- a/arch/sh/kernel/process_32.c +++ b/arch/sh/kernel/process_32.c @@ -86,7 +86,7 @@ void cpu_idle(void) if (!idle) idle = default_idle; - tick_nohz_stop_sched_tick(); + tick_nohz_stop_sched_tick(1); while (!need_resched()) idle(); tick_nohz_restart_sched_tick(); -- cgit v1.2.3-70-g09d2 From 4c1cfab1e0f9a41246cfdcca78f3700fb67f0a5c Mon Sep 17 00:00:00 2001 From: Adrian Bunk Date: Wed, 18 Jun 2008 03:36:50 +0300 Subject: sh/kernel/ cleanups This patch contains the following cleanups: - make the following needlessly global code static: - cf-enabler.c: cf_init() - cpu/clock.c: __clk_enable() - cpu/clock.c: __clk_disable() - process_32.c: default_idle() - time_32.c: struct clocksource_sh - timers/timer-tmu.c: struct tmu_timer_ops - remove the following unused functions (no CONFIG_BLK_DEV_FD on sh): - process_{32,64}.c: disable_hlt() - process_{32,64}.c: enable_hlt() Signed-off-by: Adrian Bunk Signed-off-by: Paul Mundt --- arch/sh/kernel/cf-enabler.c | 2 +- arch/sh/kernel/cpu/clock.c | 6 ++---- arch/sh/kernel/process_32.c | 14 +------------- arch/sh/kernel/process_64.c | 10 ---------- arch/sh/kernel/time_32.c | 2 +- arch/sh/kernel/timers/timer-tmu.c | 2 +- include/asm-sh/clock.h | 3 --- include/asm-sh/system.h | 8 -------- include/asm-sh/timer.h | 1 - 9 files changed, 6 insertions(+), 42 deletions(-) (limited to 'arch/sh/kernel/process_32.c') diff --git a/arch/sh/kernel/cf-enabler.c b/arch/sh/kernel/cf-enabler.c index 01ff4d05aab..d3d9f320423 100644 --- a/arch/sh/kernel/cf-enabler.c +++ b/arch/sh/kernel/cf-enabler.c @@ -157,7 +157,7 @@ static int __init cf_init_se(void) } #endif -int __init cf_init(void) +static int __init cf_init(void) { if (mach_is_se() || mach_is_7722se() || mach_is_7721se()) return cf_init_se(); diff --git a/arch/sh/kernel/cpu/clock.c b/arch/sh/kernel/cpu/clock.c index b5f1e23ed57..73334c689e9 100644 --- a/arch/sh/kernel/cpu/clock.c +++ b/arch/sh/kernel/cpu/clock.c @@ -88,7 +88,7 @@ static void propagate_rate(struct clk *clk) } } -int __clk_enable(struct clk *clk) +static int __clk_enable(struct clk *clk) { /* * See if this is the first time we're enabling the clock, some @@ -111,7 +111,6 @@ int __clk_enable(struct clk *clk) return 0; } -EXPORT_SYMBOL_GPL(__clk_enable); int clk_enable(struct clk *clk) { @@ -131,7 +130,7 @@ static void clk_kref_release(struct kref *kref) /* Nothing to do */ } -void __clk_disable(struct clk *clk) +static void __clk_disable(struct clk *clk) { int count = kref_put(&clk->kref, clk_kref_release); @@ -143,7 +142,6 @@ void __clk_disable(struct clk *clk) clk->ops->disable(clk); } } -EXPORT_SYMBOL_GPL(__clk_disable); void clk_disable(struct clk *clk) { diff --git a/arch/sh/kernel/process_32.c b/arch/sh/kernel/process_32.c index 921892c351d..3326a45749d 100644 --- a/arch/sh/kernel/process_32.c +++ b/arch/sh/kernel/process_32.c @@ -34,18 +34,6 @@ void (*pm_idle)(void); void (*pm_power_off)(void); EXPORT_SYMBOL(pm_power_off); -void disable_hlt(void) -{ - hlt_counter++; -} -EXPORT_SYMBOL(disable_hlt); - -void enable_hlt(void) -{ - hlt_counter--; -} -EXPORT_SYMBOL(enable_hlt); - static int __init nohlt_setup(char *__unused) { hlt_counter = 1; @@ -60,7 +48,7 @@ static int __init hlt_setup(char *__unused) } __setup("hlt", hlt_setup); -void default_idle(void) +static void default_idle(void) { if (!hlt_counter) { clear_thread_flag(TIF_POLLING_NRFLAG); diff --git a/arch/sh/kernel/process_64.c b/arch/sh/kernel/process_64.c index 0283d813307..b9dbd2d3b4a 100644 --- a/arch/sh/kernel/process_64.c +++ b/arch/sh/kernel/process_64.c @@ -36,16 +36,6 @@ static int hlt_counter = 1; #define HARD_IDLE_TIMEOUT (HZ / 3) -void disable_hlt(void) -{ - hlt_counter++; -} - -void enable_hlt(void) -{ - hlt_counter--; -} - static int __init nohlt_setup(char *__unused) { hlt_counter = 1; diff --git a/arch/sh/kernel/time_32.c b/arch/sh/kernel/time_32.c index 7281342c044..0758b5ee818 100644 --- a/arch/sh/kernel/time_32.c +++ b/arch/sh/kernel/time_32.c @@ -211,7 +211,7 @@ unsigned long sh_hpt_frequency = 0; #define NSEC_PER_CYC_SHIFT 10 -struct clocksource clocksource_sh = { +static struct clocksource clocksource_sh = { .name = "SuperH", .rating = 200, .mask = CLOCKSOURCE_MASK(32), diff --git a/arch/sh/kernel/timers/timer-tmu.c b/arch/sh/kernel/timers/timer-tmu.c index 8935570008d..1ca9ad49b54 100644 --- a/arch/sh/kernel/timers/timer-tmu.c +++ b/arch/sh/kernel/timers/timer-tmu.c @@ -209,7 +209,7 @@ static int tmu_timer_init(void) return 0; } -struct sys_timer_ops tmu_timer_ops = { +static struct sys_timer_ops tmu_timer_ops = { .init = tmu_timer_init, .start = tmu_timer_start, .stop = tmu_timer_stop, diff --git a/include/asm-sh/clock.h b/include/asm-sh/clock.h index b550a27a704..608fda55c0e 100644 --- a/include/asm-sh/clock.h +++ b/include/asm-sh/clock.h @@ -41,9 +41,6 @@ void arch_init_clk_ops(struct clk_ops **, int type); /* arch/sh/kernel/cpu/clock.c */ int clk_init(void); -int __clk_enable(struct clk *); -void __clk_disable(struct clk *); - void clk_recalc_rate(struct clk *); int clk_register(struct clk *); diff --git a/include/asm-sh/system.h b/include/asm-sh/system.h index e65b6b822cb..056d68cd210 100644 --- a/include/asm-sh/system.h +++ b/include/asm-sh/system.h @@ -148,14 +148,6 @@ extern unsigned long cached_to_uncached; extern struct dentry *sh_debugfs_root; -/* XXX - * disable hlt during certain critical i/o operations - */ -#define HAVE_DISABLE_HLT -void disable_hlt(void); -void enable_hlt(void); - -void default_idle(void); void per_cpu_trap_init(void); asmlinkage void break_point_trap(void); diff --git a/include/asm-sh/timer.h b/include/asm-sh/timer.h index 701ba84c704..327f7eb8976 100644 --- a/include/asm-sh/timer.h +++ b/include/asm-sh/timer.h @@ -40,6 +40,5 @@ struct sys_timer *get_sys_timer(void); /* arch/sh/kernel/time.c */ void handle_timer_tick(void); extern unsigned long sh_hpt_frequency; -extern struct clocksource clocksource_sh; #endif /* __ASM_SH_TIMER_H */ -- cgit v1.2.3-70-g09d2