History

tracing: Force arch_local_irq_* notrace for paravirt

When running ktest.pl randconfig tests, I would sometimes trigger a lockdep annotation bug (possible reason: unannotated irqs-on). This triggering happened right after function tracer self test was executed. After doing a config bisect I found that this was caused with having function tracer, paravirt guest, prove locking, and rcu torture all enabled. The rcu torture just enhanced the likelyhood of triggering the bug. Prove locking was needed, since it was the thing that was bugging. Function tracer would trace and disable interrupts in all sorts of funny places. paravirt guest would turn arch_local_irq_* into functions that would be traced. Besides the fact that tracing arch_local_irq_* is just a bad idea, this is what is happening. The bug happened simply in the local_irq_restore() code: if (raw_irqs_disabled_flags(flags)) { \ raw_local_irq_restore(flags); \ trace_hardirqs_off(); \ } else { \ trace_hardirqs_on(); \ raw_local_irq_restore(flags); \ } \ The raw_local_irq_restore() was defined as arch_local_irq_restore(). Now imagine, we are about to enable interrupts. We go into the else case and call trace_hardirqs_on() which tells lockdep that we are enabling interrupts, so it sets the current->hardirqs_enabled = 1. Then we call raw_local_irq_restore() which calls arch_local_irq_restore() which gets traced! Now in the function tracer we disable interrupts with local_irq_save(). This is fine, but flags is stored that we have interrupts disabled. When the function tracer calls local_irq_restore() it does it, but this time with flags set as disabled, so we go into the if () path. This keeps interrupts disabled and calls trace_hardirqs_off() which sets current->hardirqs_enabled = 0. When the tracer is finished and proceeds with the original code, we enable interrupts but leave current->hardirqs_enabled as 0. Which now breaks lockdeps internal processing.

Steven Rostedt committed 16 years ago

Commit b5908541 parent 13b9b6e1 file · +5 −5Browse files

Unified · Split

arch/x86/include/asm/paravirt.h+5−5
@@ -824,27 +824,27 @@ static __always_inline void arch_spin_unlock(struct arch_spinlock *lock) · 823 unmodified lines
824824 #define __PV_IS_CALLEE_SAVE(func) \
825825 ((struct paravirt_callee_save) { func })
826826
827−static inline unsigned long arch_local_save_flags(void)
827+static inline notrace unsigned long arch_local_save_flags(void)
828828 {
829829 return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl);
830830 }
831831
832−static inline void arch_local_irq_restore(unsigned long f)
832+static inline notrace void arch_local_irq_restore(unsigned long f)
833833 {
834834 PVOP_VCALLEE1(pv_irq_ops.restore_fl, f);
835835 }
836836
837−static inline void arch_local_irq_disable(void)
837+static inline notrace void arch_local_irq_disable(void)
838838 {
839839 PVOP_VCALLEE0(pv_irq_ops.irq_disable);
840840 }
841841
842−static inline void arch_local_irq_enable(void)
842+static inline notrace void arch_local_irq_enable(void)
843843 {
844844 PVOP_VCALLEE0(pv_irq_ops.irq_enable);
845845 }
846846
847−static inline unsigned long arch_local_irq_save(void)
847+static inline notrace unsigned long arch_local_irq_save(void)
848848 {
849849 unsigned long f;
850850