From af8772926f019b7bddd7477b8de5f3b0f12bad21 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 8 Jul 2012 15:58:49 -0400 Subject: target: replace the processing thread with a TMR work queue The last functionality of the target processing thread is offloading possibly long running task management requests from the submitter context. To keep TMR semantics the same we need a single threaded ordered queue, which can be provided by a per-device workqueue with the right flags. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/target/target_core_device.c') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 5ad972856a8d..bcef6771bafc 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -715,7 +715,7 @@ void se_release_device_for_hba(struct se_device *dev) se_dev_stop(dev); if (dev->dev_ptr) { - kthread_stop(dev->process_thread); + destroy_workqueue(dev->tmr_wq); if (dev->transport->free_device) dev->transport->free_device(dev->dev_ptr); } -- cgit v1.2.3 From 2dca673b46c586d8e5dd0c8e3a582b51a1d7907d Mon Sep 17 00:00:00 2001 From: Andy Grover Date: Thu, 12 Jul 2012 17:34:55 -0700 Subject: target: Remove hba param from core_dev_add_lun Only used in a debugprint, and function signature is cleaner now. Signed-off-by: Andy Grover Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 3 +-- drivers/target/target_core_fabric_configfs.c | 3 +-- drivers/target/target_core_internal.h | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) (limited to 'drivers/target/target_core_device.c') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index bcef6771bafc..415c71ecdff0 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1273,7 +1273,6 @@ int se_dev_set_block_size(struct se_device *dev, u32 block_size) struct se_lun *core_dev_add_lun( struct se_portal_group *tpg, - struct se_hba *hba, struct se_device *dev, u32 lun) { @@ -1298,7 +1297,7 @@ struct se_lun *core_dev_add_lun( pr_debug("%s_TPG[%u]_LUN[%u] - Activated %s Logical Unit from" " CORE HBA: %u\n", tpg->se_tpg_tfo->get_fabric_name(), tpg->se_tpg_tfo->tpg_get_tag(tpg), lun_p->unpacked_lun, - tpg->se_tpg_tfo->get_fabric_name(), hba->hba_id); + tpg->se_tpg_tfo->get_fabric_name(), dev->se_hba->hba_id); /* * Update LUN maps for dynamically added initiators when * generate_node_acl is enabled. diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 405cc98eaed6..ea479e54f5fd 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -764,8 +764,7 @@ static int target_fabric_port_link( goto out; } - lun_p = core_dev_add_lun(se_tpg, dev->se_hba, dev, - lun->unpacked_lun); + lun_p = core_dev_add_lun(se_tpg, dev, lun->unpacked_lun); if (IS_ERR(lun_p)) { pr_err("core_dev_add_lun() failed\n"); ret = PTR_ERR(lun_p); diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index 88f69e0ba515..d41a9bb89ce1 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -45,8 +45,7 @@ int se_dev_set_max_sectors(struct se_device *, u32); int se_dev_set_fabric_max_sectors(struct se_device *, u32); int se_dev_set_optimal_sectors(struct se_device *, u32); int se_dev_set_block_size(struct se_device *, u32); -struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_hba *, - struct se_device *, u32); +struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_device *, u32); int core_dev_del_lun(struct se_portal_group *, u32); struct se_lun *core_get_lun_from_tpg(struct se_portal_group *, u32); struct se_lun_acl *core_dev_init_initiator_node_lun_acl(struct se_portal_group *, -- cgit v1.2.3 From e80ac6c4cc073d54daef8ba4ced88347d130d314 Mon Sep 17 00:00:00 2001 From: Andy Grover Date: Thu, 12 Jul 2012 17:34:58 -0700 Subject: target: refactor core_update_device_list_for_node() Code was almost entirely divided based on value of bool param "enable". Split it into two functions. Signed-off-by: Andy Grover Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 166 +++++++++++++++++++--------------- drivers/target/target_core_internal.h | 6 +- drivers/target/target_core_tpg.c | 8 +- 3 files changed, 103 insertions(+), 77 deletions(-) (limited to 'drivers/target/target_core_device.c') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 415c71ecdff0..0a1c907a531b 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -300,8 +300,8 @@ int core_free_device_list_for_node( lun = deve->se_lun; spin_unlock_irq(&nacl->device_list_lock); - core_update_device_list_for_node(lun, NULL, deve->mapped_lun, - TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg, 0); + core_disable_device_list_for_node(lun, NULL, deve->mapped_lun, + TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg); spin_lock_irq(&nacl->device_list_lock); } spin_unlock_irq(&nacl->device_list_lock); @@ -342,72 +342,46 @@ void core_update_device_list_access( spin_unlock_irq(&nacl->device_list_lock); } -/* core_update_device_list_for_node(): +/* core_enable_device_list_for_node(): * * */ -int core_update_device_list_for_node( +int core_enable_device_list_for_node( struct se_lun *lun, struct se_lun_acl *lun_acl, u32 mapped_lun, u32 lun_access, struct se_node_acl *nacl, - struct se_portal_group *tpg, - int enable) + struct se_portal_group *tpg) { struct se_port *port = lun->lun_sep; - struct se_dev_entry *deve = nacl->device_list[mapped_lun]; - int trans = 0; - /* - * If the MappedLUN entry is being disabled, the entry in - * port->sep_alua_list must be removed now before clearing the - * struct se_dev_entry pointers below as logic in - * core_alua_do_transition_tg_pt() depends on these being present. - */ - if (!enable) { - /* - * deve->se_lun_acl will be NULL for demo-mode created LUNs - * that have not been explicitly concerted to MappedLUNs -> - * struct se_lun_acl, but we remove deve->alua_port_list from - * port->sep_alua_list. This also means that active UAs and - * NodeACL context specific PR metadata for demo-mode - * MappedLUN *deve will be released below.. - */ - spin_lock_bh(&port->sep_alua_lock); - list_del(&deve->alua_port_list); - spin_unlock_bh(&port->sep_alua_lock); - } + struct se_dev_entry *deve; spin_lock_irq(&nacl->device_list_lock); - if (enable) { - /* - * Check if the call is handling demo mode -> explict LUN ACL - * transition. This transition must be for the same struct se_lun - * + mapped_lun that was setup in demo mode.. - */ - if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) { - if (deve->se_lun_acl != NULL) { - pr_err("struct se_dev_entry->se_lun_acl" - " already set for demo mode -> explict" - " LUN ACL transition\n"); - spin_unlock_irq(&nacl->device_list_lock); - return -EINVAL; - } - if (deve->se_lun != lun) { - pr_err("struct se_dev_entry->se_lun does" - " match passed struct se_lun for demo mode" - " -> explict LUN ACL transition\n"); - spin_unlock_irq(&nacl->device_list_lock); - return -EINVAL; - } - deve->se_lun_acl = lun_acl; - trans = 1; - } else { - deve->se_lun = lun; - deve->se_lun_acl = lun_acl; - deve->mapped_lun = mapped_lun; - deve->lun_flags |= TRANSPORT_LUNFLAGS_INITIATOR_ACCESS; + + deve = nacl->device_list[mapped_lun]; + + /* + * Check if the call is handling demo mode -> explict LUN ACL + * transition. This transition must be for the same struct se_lun + * + mapped_lun that was setup in demo mode.. + */ + if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) { + if (deve->se_lun_acl != NULL) { + pr_err("struct se_dev_entry->se_lun_acl" + " already set for demo mode -> explict" + " LUN ACL transition\n"); + spin_unlock_irq(&nacl->device_list_lock); + return -EINVAL; + } + if (deve->se_lun != lun) { + pr_err("struct se_dev_entry->se_lun does" + " match passed struct se_lun for demo mode" + " -> explict LUN ACL transition\n"); + spin_unlock_irq(&nacl->device_list_lock); + return -EINVAL; } + deve->se_lun_acl = lun_acl; if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) { deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY; @@ -417,20 +391,70 @@ int core_update_device_list_for_node( deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; } - if (trans) { - spin_unlock_irq(&nacl->device_list_lock); - return 0; - } - deve->creation_time = get_jiffies_64(); - deve->attach_count++; spin_unlock_irq(&nacl->device_list_lock); + return 0; + } - spin_lock_bh(&port->sep_alua_lock); - list_add_tail(&deve->alua_port_list, &port->sep_alua_list); - spin_unlock_bh(&port->sep_alua_lock); + deve->se_lun = lun; + deve->se_lun_acl = lun_acl; + deve->mapped_lun = mapped_lun; + deve->lun_flags |= TRANSPORT_LUNFLAGS_INITIATOR_ACCESS; - return 0; + if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) { + deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY; + deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; + } else { + deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE; + deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; } + + deve->creation_time = get_jiffies_64(); + deve->attach_count++; + spin_unlock_irq(&nacl->device_list_lock); + + spin_lock_bh(&port->sep_alua_lock); + list_add_tail(&deve->alua_port_list, &port->sep_alua_list); + spin_unlock_bh(&port->sep_alua_lock); + + return 0; +} + +/* core_disable_device_list_for_node(): + * + * + */ +int core_disable_device_list_for_node( + struct se_lun *lun, + struct se_lun_acl *lun_acl, + u32 mapped_lun, + u32 lun_access, + struct se_node_acl *nacl, + 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]; + + /* + * If the MappedLUN entry is being disabled, the entry in + * port->sep_alua_list must be removed now before clearing the + * struct se_dev_entry pointers below as logic in + * core_alua_do_transition_tg_pt() depends on these being present. + * + * deve->se_lun_acl will be NULL for demo-mode created LUNs + * that have not been explicitly converted to MappedLUNs -> + * struct se_lun_acl, but we remove deve->alua_port_list from + * port->sep_alua_list. This also means that active UAs and + * NodeACL context specific PR metadata for demo-mode + * MappedLUN *deve will be released below.. + */ + 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. @@ -475,9 +499,9 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg) continue; spin_unlock_irq(&nacl->device_list_lock); - core_update_device_list_for_node(lun, NULL, + core_disable_device_list_for_node(lun, NULL, deve->mapped_lun, TRANSPORT_LUNFLAGS_NO_ACCESS, - nacl, tpg, 0); + nacl, tpg); spin_lock_irq(&nacl->device_list_lock); } @@ -1469,8 +1493,8 @@ int core_dev_add_initiator_node_lun_acl( lacl->se_lun = lun; - if (core_update_device_list_for_node(lun, lacl, lacl->mapped_lun, - lun_access, nacl, tpg, 1) < 0) + if (core_enable_device_list_for_node(lun, lacl, lacl->mapped_lun, + lun_access, nacl, tpg) < 0) return -EINVAL; spin_lock(&lun->lun_acl_lock); @@ -1513,8 +1537,8 @@ int core_dev_del_initiator_node_lun_acl( smp_mb__after_atomic_dec(); spin_unlock(&lun->lun_acl_lock); - core_update_device_list_for_node(lun, NULL, lacl->mapped_lun, - TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg, 0); + core_disable_device_list_for_node(lun, NULL, lacl->mapped_lun, + TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg); lacl->se_lun = NULL; diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index d41a9bb89ce1..0fd428225d11 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -10,8 +10,10 @@ int core_free_device_list_for_node(struct se_node_acl *, struct se_portal_group *); void core_dec_lacl_count(struct se_node_acl *, struct se_cmd *); void core_update_device_list_access(u32, u32, struct se_node_acl *); -int core_update_device_list_for_node(struct se_lun *, struct se_lun_acl *, - u32, u32, struct se_node_acl *, struct se_portal_group *, int); +int core_enable_device_list_for_node(struct se_lun *, struct se_lun_acl *, + u32, u32, struct se_node_acl *, struct se_portal_group *); +int core_disable_device_list_for_node(struct se_lun *, struct se_lun_acl *, + u32, u32, struct se_node_acl *, struct se_portal_group *); void core_clear_lun_from_tpg(struct se_lun *, struct se_portal_group *); int core_dev_export(struct se_device *, struct se_portal_group *, struct se_lun *); diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index 5ca340cff111..b8628a5014b9 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -77,8 +77,8 @@ static void core_clear_initiator_node_from_tpg( lun = deve->se_lun; spin_unlock_irq(&nacl->device_list_lock); - core_update_device_list_for_node(lun, NULL, deve->mapped_lun, - TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg, 0); + core_disable_device_list_for_node(lun, NULL, deve->mapped_lun, + TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg); spin_lock_irq(&nacl->device_list_lock); } @@ -172,8 +172,8 @@ void core_tpg_add_node_to_devs( (lun_access == TRANSPORT_LUNFLAGS_READ_WRITE) ? "READ-WRITE" : "READ-ONLY"); - core_update_device_list_for_node(lun, NULL, lun->unpacked_lun, - lun_access, acl, tpg, 1); + core_enable_device_list_for_node(lun, NULL, lun->unpacked_lun, + lun_access, acl, tpg); spin_lock(&tpg->tpg_lun_lock); } spin_unlock(&tpg->tpg_lun_lock); -- cgit v1.2.3 From 77d4c7450646e6bcba3ff6de17aa4b98c34fe849 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Sat, 14 Jul 2012 15:11:41 -0700 Subject: 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(-) (limited to 'drivers/target/target_core_device.c') 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 -- cgit v1.2.3 From c8045372dd2d4784bc4cb32ddc27f4b88ff2140c Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Mon, 16 Jul 2012 15:17:12 -0700 Subject: target: Make unnecessarily global se_dev_align_max_sectors() static Signed-off-by: Roland Dreier Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/target/target_core_device.c') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 0048d0a0b0c1..cf2c66f3c116 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -841,7 +841,7 @@ int se_dev_check_shutdown(struct se_device *dev) return ret; } -u32 se_dev_align_max_sectors(u32 max_sectors, u32 block_size) +static u32 se_dev_align_max_sectors(u32 max_sectors, u32 block_size) { u32 tmp, aligned_max_sectors; /* -- cgit v1.2.3