From e5ab012c3271990e8457055c25cafddc1ae8aa6b Mon Sep 17 00:00:00 2001 From: Frederic Weisbecker Date: Wed, 20 Feb 2013 16:15:36 +0100 Subject: [PATCH 1/6] nohz: Make tick_nohz_irq_exit() irq safe As it stands, irq_exit() may or may not be called with irqs disabled, depending on __ARCH_IRQ_EXIT_IRQS_DISABLED that the arch can define. It makes tick_nohz_irq_exit() unsafe. For example two interrupts can race in tick_nohz_stop_sched_tick(): the inner most one computes the expiring time on top of the timer list, then it's interrupted right before reprogramming the clock. The new interrupt enqueues a new timer list timer, it reprogram the clock to take it into account and it exits. The CPUs resumes the inner most interrupt and performs the clock reprogramming without considering the new timer list timer. This regression has been introduced by: 280f06774afedf849f0b34248ed6aff57d0f6908 ("nohz: Separate out irq exit and idle loop dyntick logic") Let's fix it right now with the appropriate protections. A saner long term solution will be to remove __ARCH_IRQ_EXIT_IRQS_DISABLED and mandate that irq_exit() is called with interrupts disabled. Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Linus Torvalds Cc: #v3.2+ Link: http://lkml.kernel.org/r/1361373336-11337-1-git-send-email-fweisbec@gmail.com Signed-off-by: Thomas Gleixner --- kernel/time/tick-sched.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 314b9ee07edf..520592ab6aa4 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -565,14 +565,19 @@ void tick_nohz_idle_enter(void) */ void tick_nohz_irq_exit(void) { + unsigned long flags; struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched); if (!ts->inidle) return; - /* Cancel the timer because CPU already waken up from the C-states*/ + local_irq_save(flags); + + /* Cancel the timer because CPU already waken up from the C-states */ menu_hrtimer_cancel(); __tick_nohz_idle_enter(ts); + + local_irq_restore(flags); } /** From 74eed0163d0def3fce27228d9ccf3d36e207b286 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 20 Feb 2013 22:00:48 +0100 Subject: [PATCH 2/6] irq: Ensure irq_exit() code runs with interrupts disabled We had already a few problems with code called from irq_exit() when interrupted from a nesting interrupt. This can happen on architectures which do not define __ARCH_IRQ_EXIT_IRQS_DISABLED. __ARCH_IRQ_EXIT_IRQS_DISABLED should go away and we want to make it mandatory to call irq_exit() with interrupts disabled. As a temporary protection disable interrupts for those architectures which do not define __ARCH_IRQ_EXIT_IRQS_DISABLED and add a WARN_ONCE when an architecture which defines __ARCH_IRQ_EXIT_IRQS_DISABLED calls irq_exit() with interrupts enabled. Signed-off-by: Thomas Gleixner Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Linus Torvalds Link: http://lkml.kernel.org/r/alpine.LFD.2.02.1302202155320.22263@ionos --- kernel/softirq.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kernel/softirq.c b/kernel/softirq.c index f5cc25f147a6..f2a934673008 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -341,6 +341,14 @@ static inline void invoke_softirq(void) */ void irq_exit(void) { +#ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED + unsigned long flags; + + local_irq_save(flags); +#else + WARN_ON_ONCE(!irqs_disabled()); +#endif + account_irq_exit_time(current); trace_hardirq_exit(); sub_preempt_count(IRQ_EXIT_OFFSET); @@ -354,6 +362,9 @@ void irq_exit(void) #endif rcu_irq_exit(); sched_preempt_enable_no_resched(); +#ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED + local_irq_restore(flags); +#endif } /* From facd8b80c67a3cf64a467c4a2ac5fb31f2e6745b Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Thu, 21 Feb 2013 18:17:42 +0100 Subject: [PATCH 3/6] irq: Sanitize invoke_softirq With the irq protection in irq_exit, we can remove the #ifdeffery and the bh_disable/enable dance in invoke_softirq() Signed-off-by: Thomas Gleixner Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Linus Torvalds Link: http://lkml.kernel.org/r/alpine.LFD.2.02.1302202155320.22263@ionos --- kernel/softirq.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index f2a934673008..24a921bcf04f 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -322,18 +322,10 @@ void irq_enter(void) static inline void invoke_softirq(void) { - if (!force_irqthreads) { -#ifdef __ARCH_IRQ_EXIT_IRQS_DISABLED + if (!force_irqthreads) __do_softirq(); -#else - do_softirq(); -#endif - } else { - __local_bh_disable((unsigned long)__builtin_return_address(0), - SOFTIRQ_OFFSET); + else wakeup_softirqd(); - __local_bh_enable(SOFTIRQ_OFFSET); - } } /* From af7bdbafe3812af406ce07631effd2b96aae2dba Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Thu, 21 Feb 2013 18:21:30 +0100 Subject: [PATCH 4/6] Revert "nohz: Make tick_nohz_irq_exit() irq safe" This reverts commit 351429b2e62b6545bb10c756686393f29ba268a1. The extra local_irq_save() is not longer needed as the call site now always calls with interrupts disabled. Signed-off-by: Thomas Gleixner Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Linus Torvalds --- kernel/time/tick-sched.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 520592ab6aa4..314b9ee07edf 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -565,19 +565,14 @@ void tick_nohz_idle_enter(void) */ void tick_nohz_irq_exit(void) { - unsigned long flags; struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched); if (!ts->inidle) return; - local_irq_save(flags); - - /* Cancel the timer because CPU already waken up from the C-states */ + /* Cancel the timer because CPU already waken up from the C-states*/ menu_hrtimer_cancel(); __tick_nohz_idle_enter(ts); - - local_irq_restore(flags); } /** From 4d4c4e24cf48400a24d33feffc7cca4f4e8cabe1 Mon Sep 17 00:00:00 2001 From: Frederic Weisbecker Date: Fri, 22 Feb 2013 00:05:07 +0100 Subject: [PATCH 5/6] irq: Remove IRQ_EXIT_OFFSET workaround The IRQ_EXIT_OFFSET trick was used to make sure the irq doesn't get preempted after we substract the HARDIRQ_OFFSET until we are entirely done with any code in irq_exit(). This workaround was necessary because some archs may call irq_exit() with irqs enabled and there is still some code in the end of this function that is not covered by the HARDIRQ_OFFSET but want to stay non-preemptible. Now that irq are always disabled in irq_exit(), the whole code is guaranteed not to be preempted. We can thus remove this hack. Signed-off-by: Frederic Weisbecker Cc: Linus Torvalds Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Paul E. McKenney --- include/linux/hardirq.h | 2 -- kernel/softirq.c | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h index 29eb805ea4a6..c1d6555d2567 100644 --- a/include/linux/hardirq.h +++ b/include/linux/hardirq.h @@ -118,10 +118,8 @@ #ifdef CONFIG_PREEMPT_COUNT # define preemptible() (preempt_count() == 0 && !irqs_disabled()) -# define IRQ_EXIT_OFFSET (HARDIRQ_OFFSET-1) #else # define preemptible() 0 -# define IRQ_EXIT_OFFSET HARDIRQ_OFFSET #endif #if defined(CONFIG_SMP) || defined(CONFIG_GENERIC_HARDIRQS) diff --git a/kernel/softirq.c b/kernel/softirq.c index 24a921bcf04f..f42ff97e1f8f 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -343,7 +343,7 @@ void irq_exit(void) account_irq_exit_time(current); trace_hardirq_exit(); - sub_preempt_count(IRQ_EXIT_OFFSET); + sub_preempt_count(HARDIRQ_OFFSET); if (!in_interrupt() && local_softirq_pending()) invoke_softirq(); @@ -353,7 +353,6 @@ void irq_exit(void) tick_nohz_irq_exit(); #endif rcu_irq_exit(); - sched_preempt_enable_no_resched(); #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED local_irq_restore(flags); #endif From 4cd5d1115c2f752ca94a0eb461b36d88fb37ed1e Mon Sep 17 00:00:00 2001 From: Frederic Weisbecker Date: Thu, 28 Feb 2013 20:00:43 +0100 Subject: [PATCH 6/6] irq: Don't re-enable interrupts at the end of irq_exit Commit 74eed0163d0def3fce27228d9ccf3d36e207b286 "irq: Ensure irq_exit() code runs with interrupts disabled" restore interrupts flags in the end of irq_exit() for archs that don't define __ARCH_IRQ_EXIT_IRQS_DISABLED. However always returning from irq_exit() with interrupts disabled should not be a problem for these archs. Prior to this commit this was already happening anytime we processed pending softirqs anyway. Suggested-by: Linus Torvalds Signed-off-by: Frederic Weisbecker Cc: Linus Torvalds Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Paul E. McKenney --- kernel/softirq.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index f42ff97e1f8f..dce38fac4f32 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -334,9 +334,7 @@ static inline void invoke_softirq(void) void irq_exit(void) { #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED - unsigned long flags; - - local_irq_save(flags); + local_irq_disable(); #else WARN_ON_ONCE(!irqs_disabled()); #endif @@ -353,9 +351,6 @@ void irq_exit(void) tick_nohz_irq_exit(); #endif rcu_irq_exit(); -#ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED - local_irq_restore(flags); -#endif } /*