• Erik Hugne's avatar
    tipc: remove interface state mirroring in bearer · 512137ee
    Erik Hugne authored
    struct 'tipc_bearer' is a generic representation of the underlying
    media type, and exists in a one-to-one relationship to each interface
    TIPC is using. The struct contains a 'blocked' flag that mirrors the
    operational and execution state of the represented interface, and is
    updated through notification calls from the latter. The users of
    tipc_bearer are checking this flag before each attempt to send a
    packet via the interface.
    This state mirroring serves no purpose in the current code base. TIPC
    links will not discover a media failure any faster through this
    mechanism, and in reality the flag only adds overhead at packet
    sending and reception.
    Furthermore, the fact that the flag needs to be protected by a spinlock
    aggregated into tipc_bearer has turned out to cause a serious and
    completely unnecessary deadlock problem.
    CPU0                                    CPU1
    ----                                    ----
    Time 0: bearer_disable()                link_timeout()
    Time 1:   spin_lock_bh(&b_ptr->lock)      tipc_link_push_queue()
    Time 2:   tipc_link_delete()                tipc_bearer_blocked(b_ptr)
    Time 3:     k_cancel_timer(&req->timer)       spin_lock_bh(&b_ptr->lock)
    Time 4:       del_timer_sync(&req->timer)
    I.e., del_timer_sync() on CPU0 never returns, because the timer handler
    on CPU1 is waiting for the bearer lock.
    We eliminate the 'blocked' flag from struct tipc_bearer, along with all
    tests on this flag. This not only resolves the deadlock, but also
    simplifies and speeds up the data path execution of TIPC. It also fits
    well into our ongoing effort to make the locking policy simpler and
    more manageable.
    An effect of this change is that we can get rid of functions such as
    tipc_bearer_blocked(), tipc_continue() and tipc_block_bearer().
    We replace the latter with a new function, tipc_reset_bearer(), which
    resets all links associated to the bearer immediately after an
    interface goes down.
    A user might notice one slight change in link behaviour after this
    change. When an interface goes down, (e.g. through a NETDEV_DOWN
    event) all attached links will be reset immediately, instead of
    leaving it to each link to detect the failure through a timer-driven
    mechanism. We consider this an improvement, and see no obvious risks
    with the new behavior.
    Signed-off-by: default avatarErik Hugne <erik.hugne@ericsson.com>
    Reviewed-by: Ying Xue's avatarYing Xue <ying.xue@windriver.com>
    Reviewed-by: default avatarPaul Gortmaker <Paul.Gortmaker@windriver.com>
    Signed-off-by: default avatarJon Maloy <jon.maloy@ericsson.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>