From 77d4c7450646e6bcba3ff6de17aa4b98c34fe849 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Sat, 14 Jul 2012 15:11:41 -0700 Subject: [PATCH] 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] [] ? warn_slowpath_common+0x78/0x8c [ 126.612737] [] ? local_bh_enable_ip+0x41/0x8c [ 126.619433] [] ? core_disable_device_list_for_node+0x70/0xe3 [target_core_mod] [ 126.629323] [] ? core_clear_lun_from_tpg+0x88/0xeb [target_core_mod] [ 126.638244] [] ? core_tpg_post_dellun+0x17/0x48 [target_core_mod] [ 126.646873] [] ? core_dev_del_lun+0x26/0x8c [target_core_mod] [ 126.655114] [] ? dput+0x27/0x154 [ 126.660549] [] ? target_fabric_port_unlink+0x3b/0x41 [target_core_mod] [ 126.669661] [] ? configfs_unlink+0xfc/0x14a [configfs] [ 126.677224] [] ? vfs_unlink+0x58/0xb7 [ 126.683141] [] ? do_unlinkat+0xbb/0x142 [ 126.689253] [] ? page_fault+0x25/0x30 [ 126.695170] [] ? 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 Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 0a1c907a531b..0048d0a0b0c1 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -432,11 +432,7 @@ int core_disable_device_list_for_node( struct se_portal_group *tpg) { struct se_port *port = lun->lun_sep; - struct se_dev_entry *deve; - - spin_lock_irq(&nacl->device_list_lock); - - deve = nacl->device_list[mapped_lun]; + struct se_dev_entry *deve = nacl->device_list[mapped_lun]; /* * If the MappedLUN entry is being disabled, the entry in @@ -454,14 +450,13 @@ int core_disable_device_list_for_node( spin_lock_bh(&port->sep_alua_lock); list_del(&deve->alua_port_list); spin_unlock_bh(&port->sep_alua_lock); - /* * Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE * PR operation to complete. */ - spin_unlock_irq(&nacl->device_list_lock); while (atomic_read(&deve->pr_ref_count) != 0) cpu_relax(); + spin_lock_irq(&nacl->device_list_lock); /* * Disable struct se_dev_entry LUN ACL mapping -- 2.30.2