On Wed, 20 Feb 2013, Frederic Weisbecker wrote:
> 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.
That's not a fix. That's an hack.
> A saner long term solution will be to remove
> __ARCH_IRQ_EXIT_IRQS_DISABLED.
We really want to enforce that interrupt disabled condition for
calling irq_exit(). So why make this exclusive to tick_nohz_irq_exit()?
Thanks,
tglx
Index: linux-2.6/kernel/softirq.c
===================================================================
--- linux-2.6.orig/kernel/softirq.c
+++ linux-2.6/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);
- }
}
/*
@@ -341,6 +333,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
+ BUG_ON(!irqs_disabled();
+#endif
+
account_irq_exit_time(current);
trace_hardirq_exit();
sub_preempt_count(IRQ_EXIT_OFFSET);
@@ -354,6 +354,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
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
