Commit 77d4c745 authored by Nicholas Bellinger's avatar Nicholas Bellinger

target: Make core_disable_device_list_for_node use pre-refactoring lock ordering

So after kicking around commit 547ac4c9c90 around a bit more, a tcm_qla2xxx LUN
unlink OP has generated the following warning:

[   50.386625] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
[   70.572988] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...
[  126.527531] ------------[ cut here ]------------
[  126.532677] WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0x41/0x8c()
[  126.540433] Hardware name: S5520HC
[  126.544248] Modules linked in: tcm_vhost ib_srpt ib_cm ib_sa ib_mad ib_core tcm_qla2xxx tcm_loop tcm_fc libfc iscsi_target_mod target_core_pscsi target_core_file target_core_iblock target_core_mod configfs ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi dm_round_robin dm_multipath scsi_dh loop i2c_i801 kvm_intel kvm crc32c_intel i2c_core microcode joydev button iomemory_vsl(O) pcspkr ext3 jbd uhci_hcd lpfc ata_piix libata ehci_hcd qla2xxx mlx4_core scsi_transport_fc scsi_tgt igb [last unloaded: scsi_wait_scan]
[  126.595567] Pid: 3283, comm: unlink Tainted: G           O 3.5.0-rc2+ #33
[  126.603128] Call Trace:
[  126.605853]  [<ffffffff81026b91>] ? warn_slowpath_common+0x78/0x8c
[  126.612737]  [<ffffffff8102c342>] ? local_bh_enable_ip+0x41/0x8c
[  126.619433]  [<ffffffffa03582a2>] ? core_disable_device_list_for_node+0x70/0xe3 [target_core_mod]
[  126.629323]  [<ffffffffa035849f>] ? core_clear_lun_from_tpg+0x88/0xeb [target_core_mod]
[  126.638244]  [<ffffffffa0362ec1>] ? core_tpg_post_dellun+0x17/0x48 [target_core_mod]
[  126.646873]  [<ffffffffa03575ee>] ? core_dev_del_lun+0x26/0x8c [target_core_mod]
[  126.655114]  [<ffffffff810bcbd1>] ? dput+0x27/0x154
[  126.660549]  [<ffffffffa0359aa0>] ? target_fabric_port_unlink+0x3b/0x41 [target_core_mod]
[  126.669661]  [<ffffffffa034a698>] ? configfs_unlink+0xfc/0x14a [configfs]
[  126.677224]  [<ffffffff810b5979>] ? vfs_unlink+0x58/0xb7
[  126.683141]  [<ffffffff810b6ef3>] ? do_unlinkat+0xbb/0x142
[  126.689253]  [<ffffffff81330c75>] ? page_fault+0x25/0x30
[  126.695170]  [<ffffffff81335df9>] ? system_call_fastpath+0x16/0x1b
[  126.702053] ---[ end trace 2f8e5b0a9ec797ef ]---
[  126.756336] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
[  146.942414] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...

So this warning triggered because device_list disable logic is now
holding nacl->device_list_lock w/ spin_lock_irqsave before obtaining
port->sep_alua_lock with only spin_lock_bh..

The original disable logic obtains *deve ahead of dropping the entry
from deve->alua_port_list and then obtains ->device_list_lock to do the
remaining work.  Also, I'm pretty sure this particular warning is being
generated by a demo-mode session in tcm_qla2xxx, and not by explicit
NodeACL MappedLUNs.  The Initiator MappedLUNs are already protected by a
seperate configfs symlink reference back se_lun->lun_group, and the
demo-mode se_node_acl (and associated ->device_list[]) is released
during se_portal_group->tpg_group shutdown.

The following patch drops the extra functional change to disable logic
in commit 547ac4c9c90

Cc: Andy Grover <agrover@redhat.com>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent e80ac6c4
...@@ -432,11 +432,7 @@ int core_disable_device_list_for_node( ...@@ -432,11 +432,7 @@ int core_disable_device_list_for_node(
struct se_portal_group *tpg) struct se_portal_group *tpg)
{ {
struct se_port *port = lun->lun_sep; struct se_port *port = lun->lun_sep;
struct se_dev_entry *deve; struct se_dev_entry *deve = nacl->device_list[mapped_lun];
spin_lock_irq(&nacl->device_list_lock);
deve = nacl->device_list[mapped_lun];
/* /*
* If the MappedLUN entry is being disabled, the entry in * If the MappedLUN entry is being disabled, the entry in
...@@ -454,14 +450,13 @@ int core_disable_device_list_for_node( ...@@ -454,14 +450,13 @@ int core_disable_device_list_for_node(
spin_lock_bh(&port->sep_alua_lock); spin_lock_bh(&port->sep_alua_lock);
list_del(&deve->alua_port_list); list_del(&deve->alua_port_list);
spin_unlock_bh(&port->sep_alua_lock); spin_unlock_bh(&port->sep_alua_lock);
/* /*
* Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE * Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE
* PR operation to complete. * PR operation to complete.
*/ */
spin_unlock_irq(&nacl->device_list_lock);
while (atomic_read(&deve->pr_ref_count) != 0) while (atomic_read(&deve->pr_ref_count) != 0)
cpu_relax(); cpu_relax();
spin_lock_irq(&nacl->device_list_lock); spin_lock_irq(&nacl->device_list_lock);
/* /*
* Disable struct se_dev_entry LUN ACL mapping * Disable struct se_dev_entry LUN ACL mapping
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment