• Herbert Xu's avatar
    [NET]: Drop tx lock in dev_watchdog_up · d7811e62
    Herbert Xu authored
    Fix lockdep warning with GRE, iptables and Speedtouch ADSL, PPP over ATM.
    
    On Sat, Sep 02, 2006 at 08:39:28PM +0000, Krzysztof Halasa wrote:
    > 
    > =======================================================
    > [ INFO: possible circular locking dependency detected ]
    > -------------------------------------------------------
    > swapper/0 is trying to acquire lock:
    >  (&dev->queue_lock){-+..}, at: [<c02c8c46>] dev_queue_xmit+0x56/0x290
    > 
    > but task is already holding lock:
    >  (&dev->_xmit_lock){-+..}, at: [<c02c8e14>] dev_queue_xmit+0x224/0x290
    > 
    > which lock already depends on the new lock.
    
    This turns out to be a genuine bug.  The queue lock and xmit lock are
    intentionally taken out of order.  Two things are supposed to prevent
    dead-locks from occuring:
    
    1) When we hold the queue_lock we're supposed to only do try_lock on the
    tx_lock.
    
    2) We always drop the queue_lock after taking the tx_lock and before doing
    anything else.
    
    > 
    > the existing dependency chain (in reverse order) is:
    > 
    > -> #1 (&dev->_xmit_lock){-+..}:
    >        [<c012e7b6>] lock_acquire+0x76/0xa0
    >        [<c0336241>] _spin_lock_bh+0x31/0x40
    >        [<c02d25a9>] dev_activate+0x69/0x120
    
    This path obviously breaks assumption 1) and therefore can lead to ABBA
    dead-locks.
    
    I've looked at the history and there seems to be no reason for the lock
    to be held at all in dev_watchdog_up.  The lock appeared in day one and
    even there it was unnecessary.  In fact, people added __dev_watchdog_up
    precisely in order to get around the tx lock there.
    
    The function dev_watchdog_up is already serialised by rtnl_lock since
    its only caller dev_activate is always called under it.
    
    So here is a simple patch to remove the tx lock from dev_watchdog_up.
    In 2.6.19 we can eliminate the unnecessary __dev_watchdog_up and
    replace it with dev_watchdog_up.
    Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    d7811e62
sch_generic.c 15.1 KB