Skip to content
  • Milton Miller's avatar
    powerpc/irq: Protect irq_radix_revmap_lookup against irq_free_virt · 9b788251
    Milton Miller authored
    The radix-tree code uses call_rcu when freeing internal elements.
    We must protect against the elements being freed while we traverse
    the tree, even if the returned pointer will still be valid.
    
    While preparing a patch to expand the context in which
    irq_radix_revmap_lookup will be called, I realized that the
    radix tree was not locked.
    
    When asked
    
        For a normal call_rcu usage, is it allowed to read the structure in
        irq_enter / irq_exit, without additional rcu_read_lock?  Could an
        element freed with call_rcu advance with the cpu still between
        irq_enter/irq_exit (and irq_disabled())?
    
    Paul McKenney replied:
    
        Absolutely illegal to do so. OK for call_rcu_sched(), but a
        flaming bug for call_rcu().
    
        And thank you very much for finding this!!!
    
    Further analysis:
    
    In the current CONFIG_TREE_RCU implementation. CONFIG_TREE_PREEMPT_RCU
    (and CONFIG_TINY_PREEMPT_RCU) uses explicit counters.
    
    These counters are reflected from per-CPU to global in the
    scheduling-clock-interrupt handler, so disabling irq does prevent the
    grace period from completing. But there are real-time implementations
    (such as the one use by the Concurrent guys) where disabling irq
    does -not- prevent the grace period from completing.
    
    While an alternative fix would be to switch radix-tree to rcu_sched, I
    don't want to audit the other users of radix trees (nor put alternative
    freeing in the library).  The normal overhead for rcu_read_lock and
    unlock are a local counter increment and decrement.
    
    This does not show up in the rcu lockdep because in 2.6.34 commit
    2676a58c
    
     (radix-tree: Disable RCU lockdep checking in radix tree)
    deemed it too hard to pass the condition of the protecting lock
    to the library.
    
    Signed-off-by: default avatarMilton Miller <miltonm@bga.com>
    Reviewed-by: default avatarPaul E. McKenney <paulmck@linux.vnet.ibm.com>
    Signed-off-by: default avatarBenjamin Herrenschmidt <benh@kernel.crashing.org>
    9b788251