• Jörn Engel's avatar
    target/iscsi: don't corrupt bh_count in iscsit_stop_time2retain_timer() · 574780fd
    Jörn Engel authored
    Here is a fun one.  Bug seems to have been introduced by commit 140854cb,
    almost two years ago.  I have no idea why we only started seeing it now,
    but we did.
    
    Rough callgraph:
    core_tpg_set_initiator_node_queue_depth()
    `-> spin_lock_irqsave(&tpg->session_lock, flags);
    `-> lio_tpg_shutdown_session()
        `-> iscsit_stop_time2retain_timer()
            `-> spin_unlock_bh(&se_tpg->session_lock);
            `-> spin_lock_bh(&se_tpg->session_lock);
    `-> spin_unlock_irqrestore(&tpg->session_lock, flags);
    
    core_tpg_set_initiator_node_queue_depth() used to call spin_lock_bh(),
    but 140854cb changed that to spin_lock_irqsave().  However,
    lio_tpg_shutdown_session() still claims to be called with spin_lock_bh()
    held, as does iscsit_stop_time2retain_timer():
     *      Called with spin_lock_bh(&struct se_portal_group->session_lock) held
    
    Stale documentation is mostly annoying, but in this case the dropping
    the lock with the _bh variant is plain wrong.  It is also wrong to drop
    locks two functions below the lock-holder, but I will ignore that bit
    for now.
    
    After some more locking and unlocking we eventually hit this backtrace:
    ------------[ cut here ]------------
    WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0xe8/0x100()
    Pid: 24645, comm: lio_helper.py Tainted: G           O 3.6.11+
    Call Trace:
     [<ffffffff8103e5ff>] warn_slowpath_common+0x7f/0xc0
     [<ffffffffa040ae37>] ? iscsit_inc_conn_usage_count+0x37/0x50 [iscsi_target_mod]
     [<ffffffff8103e65a>] warn_slowpath_null+0x1a/0x20
     [<ffffffff810472f8>] local_bh_enable_ip+0xe8/0x100
     [<ffffffff815b8365>] _raw_spin_unlock_bh+0x15/0x20
     [<ffffffffa040ae37>] iscsit_inc_conn_usage_count+0x37/0x50 [iscsi_target_mod]
     [<ffffffffa041149a>] iscsit_stop_session+0xfa/0x1c0 [iscsi_target_mod]
     [<ffffffffa0417fab>] lio_tpg_shutdown_session+0x7b/0x90 [iscsi_target_mod]
     [<ffffffffa033ede4>] core_tpg_set_initiator_node_queue_depth+0xe4/0x290 [target_core_mod]
     [<ffffffffa0409032>] iscsit_tpg_set_initiator_node_queue_depth+0x12/0x20 [iscsi_target_mod]
     [<ffffffffa0415c29>] lio_target_nacl_store_cmdsn_depth+0xa9/0x180 [iscsi_target_mod]
     [<ffffffffa0331b49>] target_fabric_nacl_base_attr_store+0x39/0x40 [target_core_mod]
     [<ffffffff811b857d>] configfs_write_file+0xbd/0x120
     [<ffffffff81148f36>] vfs_write+0xc6/0x180
     [<ffffffff81149251>] sys_write+0x51/0x90
     [<ffffffff815c0969>] system_call_fastpath+0x16/0x1b
    ---[ end trace 3747632b9b164652 ]---
    
    As a pure band-aid, this patch drops the _bh.
    Signed-off-by: default avatarJoern Engel <joern@logfs.org>
    Cc: stable <stable@vger.kernel.org>
    Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
    574780fd
iscsi_target_erl0.c 28.7 KB