Skip to content
  • Nicholas Piggin's avatar
    lockdep: Only trace IRQ edges · 044d0d6d
    Nicholas Piggin authored
    Problem:
    
      raw_local_irq_save(); // software state on
      local_irq_save(); // software state off
      ...
      local_irq_restore(); // software state still off, because we don't enable IRQs
      raw_local_irq_restore(); // software state still off, *whoopsie*
    
    existing instances:
    
     - lock_acquire()
         raw_local_irq_save()
         __lock_acquire()
           arch_spin_lock(&graph_lock)
             pv_wait() := kvm_wait() (same or worse for Xen/HyperV)
               local_irq_save()
    
     - trace_clock_global()
         raw_local_irq_save()
         arch_spin_lock()
           pv_wait() := kvm_wait()
    	 local_irq_save()
    
     - apic_retrigger_irq()
         raw_local_irq_save()
         apic->send_IPI() := default_send_IPI_single_phys()
           local_irq_save()
    
    Possible solutions:
    
     A) make it work by enabling the tracing inside raw_*()
     B) make it work by keeping tracing disabled inside raw_*()
     C) call it broken and clean it up now
    
    Now, given that the only reason to use the raw_* variant is because you don't
    want tracing. Therefore A) seems like a weird option (although it can be done).
    C) is tempting, but OTOH it ends up converting a _lot_ of code to raw just
    because there is one raw user, this strips the validation/tracing off for all
    the other users.
    
    So we pick B) and declare any code that ends up doing:
    
    	raw_local_irq_save()
    	local_irq_save()
    	lockdep_assert_irqs_disabled();
    
    broken. AFAICT this problem has existed forever, the only reason it came
    up is because commit: 859d069e
    
     ("lockdep: Prepare for NMI IRQ
    state tracking") changed IRQ tracing vs lockdep recursion and the
    first instance is fairly common, the other cases hardly ever happen.
    
    Signed-off-by: default avatarNicholas Piggin <npiggin@gmail.com>
    [rewrote changelog]
    Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
    Reviewed-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
    Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Acked-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
    Tested-by: default avatarMarco Elver <elver@google.com>
    Link: https://lkml.kernel.org/r/20200723105615.1268126-1-npiggin@gmail.com
    044d0d6d