From: Nicholas Bellinger <[email protected]>
Hi all,
Here is -v2 series for converting LIO target se_node_acl + se_lun
mapping tables from fixed size arrays to dynamic RCU hlist_heads.
This turns fast-path I/O into a lock-less RCU reader using existing
percpu based se_lun->lun_ref logic, and converts the RCU updater
path to allow for an arbitrary number of LUNs for both types of
mappings within target-core.
This series also squashes a number of previous se_node_acl RCU
related changes into a single commit (#1) for easier review,
and to avoid potential bisect issues.
There have been a number of changes since -v1, including:
- Mirror port->sep_rtpi in lun->lun_rtpi for RCU
- Drop unnecessary synchronize_rcu() usage
- Convert call_rcu() to kfree_rcu() usage
- Move hlist_del_rcu head of rcu_assign_pointer in se_dev_entry
- Drop unnecessary lookup deve in target_fabric_mappedlun_unlink()
- Add target_lun_is_rdonly helper
- Acquire lun_entry_mutex during core_disable_device_list_for_node
- Drop TRANSPORT_LUNFLAGS_*_ACCESS usage
- Pass se_dev_entry directly to core_disable_device_list_for_node
- Convert sbp-target se_lun usage to use ->login_lun
- Fix se_session dereference in spc_emulate_report_luns
- Fix testing for NULL instead of IS_ERR in fabric_make_lun()
- Convert BUG_ON to EINVAL for wrong dynamic -> explicit ACL conversion
- Add missing hlist_del_rcu when swapping orig with new
- Add HBA_FLAGS_INTERNAL_USE checks in add/remove lun
Please review.
--nab
Christoph Hellwig (1):
target/pr: cleanup core_scsi3_pr_seq_non_holder
Nicholas Bellinger (8):
target: Convert se_node_acl->device_list[] to RCU hlist
target/pr: Use atomic bitop for se_dev_entry->pr_reg reservation check
target/pr: Change alloc_registration to avoid pr_reg_tg_pt_lun
target: Convert se_portal_group->tpg_lun_list[] to RCU hlist
target: Convert se_tpg->acl_node_lock to ->acl_node_mutex
target: Convert core_tpg_deregister to use list splice
target: Drop unused se_lun->lun_acl_list
target: Only reset specific dynamic entries during lun_group creation
drivers/target/iscsi/iscsi_target_tpg.c | 2 -
drivers/target/sbp/sbp_target.c | 97 +++---
drivers/target/sbp/sbp_target.h | 2 +-
drivers/target/target_core_configfs.c | 6 +-
drivers/target/target_core_device.c | 452 +++++++++++----------------
drivers/target/target_core_fabric_configfs.c | 75 +++--
drivers/target/target_core_internal.h | 17 +-
drivers/target/target_core_pr.c | 217 +++++++------
drivers/target/target_core_pscsi.c | 7 +-
drivers/target/target_core_spc.c | 18 +-
drivers/target/target_core_stat.c | 180 +++++------
drivers/target/target_core_tpg.c | 269 ++++------------
drivers/target/target_core_transport.c | 20 +-
drivers/target/target_core_ua.c | 51 ++-
drivers/target/tcm_fc/tfc_conf.c | 4 +-
drivers/xen/xen-scsiback.c | 27 +-
include/target/target_core_backend.h | 2 +-
include/target/target_core_base.h | 38 +--
include/target/target_core_fabric.h | 1 -
19 files changed, 634 insertions(+), 851 deletions(-)
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch converts se_node_acl->device_list[] table for mappedluns
to modern RCU hlist_head usage in order to support an arbitrary number
of node_acl lun mappings.
It converts transport_lookup_*_lun() fast-path code to use RCU read path
primitives when looking up se_dev_entry. It adds a new hlist_head at
se_node_acl->lun_entry_hlist for this purpose.
For transport_lookup_cmd_lun() code, it works with existing per-cpu
se_lun->lun_ref when associating se_cmd with se_lun + se_device.
Also, go ahead and update core_create_device_list_for_node() +
core_free_device_list_for_node() to use ->lun_entry_hlist.
It also converts se_dev_entry->pr_ref_count access to use modern
struct kref counting, and updates core_disable_device_list_for_node()
to kref_put() and block on se_deve->pr_comp waiting for outstanding PR
references to drop.
So now that se_node_acl->lun_entry_hlist fast path access uses RCU
protected pointers, go ahead and convert remaining non-fast path
RCU updater code using ->lun_entry_lock to struct mutex to allow
callers to block while walking se_node_acl->lun_entry_hlist.
Finally drop the left-over core_clear_initiator_node_from_tpg() that
originally cleared lun_access during se_node_acl shutdown, as post
RCU conversion it now becomes duplicated logic.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_device.c | 326 +++++++++++++--------------
drivers/target/target_core_fabric_configfs.c | 43 ++--
drivers/target/target_core_internal.h | 8 +-
drivers/target/target_core_pr.c | 79 ++++---
drivers/target/target_core_pscsi.c | 7 +-
drivers/target/target_core_spc.c | 18 +-
drivers/target/target_core_stat.c | 180 +++++++--------
drivers/target/target_core_tpg.c | 75 +-----
drivers/target/target_core_ua.c | 51 +++--
include/target/target_core_backend.h | 2 +-
include/target/target_core_base.h | 19 +-
11 files changed, 386 insertions(+), 422 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 6e58976..cfe5cd3 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -59,17 +59,16 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
{
struct se_lun *se_lun = NULL;
struct se_session *se_sess = se_cmd->se_sess;
+ struct se_node_acl *nacl = se_sess->se_node_acl;
struct se_device *dev;
- unsigned long flags;
+ struct se_dev_entry *deve;
if (unpacked_lun >= TRANSPORT_MAX_LUNS_PER_TPG)
return TCM_NON_EXISTENT_LUN;
- spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
- se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
- if (se_cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
- struct se_dev_entry *deve = se_cmd->se_deve;
-
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, unpacked_lun);
+ if (deve) {
deve->total_cmds++;
if ((se_cmd->data_direction == DMA_TO_DEVICE) &&
@@ -78,7 +77,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
" Access for 0x%08x\n",
se_cmd->se_tfo->get_fabric_name(),
unpacked_lun);
- spin_unlock_irqrestore(&se_sess->se_node_acl->device_list_lock, flags);
+ rcu_read_unlock();
return TCM_WRITE_PROTECTED;
}
@@ -96,7 +95,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
percpu_ref_get(&se_lun->lun_ref);
se_cmd->lun_ref_active = true;
}
- spin_unlock_irqrestore(&se_sess->se_node_acl->device_list_lock, flags);
+ rcu_read_unlock();
if (!se_lun) {
/*
@@ -146,24 +145,23 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
struct se_dev_entry *deve;
struct se_lun *se_lun = NULL;
struct se_session *se_sess = se_cmd->se_sess;
+ struct se_node_acl *nacl = se_sess->se_node_acl;
struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
unsigned long flags;
if (unpacked_lun >= TRANSPORT_MAX_LUNS_PER_TPG)
return -ENODEV;
- spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
- se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
- deve = se_cmd->se_deve;
-
- if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, unpacked_lun);
+ if (deve) {
se_tmr->tmr_lun = deve->se_lun;
se_cmd->se_lun = deve->se_lun;
se_lun = deve->se_lun;
se_cmd->pr_res_key = deve->pr_res_key;
se_cmd->orig_fe_lun = unpacked_lun;
}
- spin_unlock_irqrestore(&se_sess->se_node_acl->device_list_lock, flags);
+ rcu_read_unlock();
if (!se_lun) {
pr_debug("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN"
@@ -185,9 +183,27 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
}
EXPORT_SYMBOL(transport_lookup_tmr_lun);
+bool target_lun_is_rdonly(struct se_cmd *cmd)
+{
+ struct se_session *se_sess = cmd->se_sess;
+ struct se_dev_entry *deve;
+ bool ret;
+
+ if (cmd->se_lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY)
+ return true;
+
+ rcu_read_lock();
+ deve = target_nacl_find_deve(se_sess->se_node_acl, cmd->orig_fe_lun);
+ ret = (deve && deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY);
+ rcu_read_unlock();
+
+ return ret;
+}
+EXPORT_SYMBOL(target_lun_is_rdonly);
+
/*
* This function is called from core_scsi3_emulate_pro_register_and_move()
- * and core_scsi3_decode_spec_i_port(), and will increment &deve->pr_ref_count
+ * and core_scsi3_decode_spec_i_port(), and will increment &deve->pr_kref
* when a matching rtpi is found.
*/
struct se_dev_entry *core_get_se_deve_from_rtpi(
@@ -196,17 +212,10 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
{
struct se_dev_entry *deve;
struct se_lun *lun;
- struct se_port *port;
struct se_portal_group *tpg = nacl->se_tpg;
- u32 i;
-
- spin_lock_irq(&nacl->device_list_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = nacl->device_list[i];
-
- if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
- continue;
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) {
lun = deve->se_lun;
if (!lun) {
pr_err("%s device entries device pointer is"
@@ -214,63 +223,29 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
tpg->se_tpg_tfo->get_fabric_name());
continue;
}
- port = lun->lun_sep;
- if (!port) {
- pr_err("%s device entries device pointer is"
- " NULL, but Initiator has access.\n",
- tpg->se_tpg_tfo->get_fabric_name());
- continue;
- }
- if (port->sep_rtpi != rtpi)
+ if (lun->lun_rtpi != rtpi)
continue;
- atomic_inc_mb(&deve->pr_ref_count);
- spin_unlock_irq(&nacl->device_list_lock);
+ kref_get(&deve->pr_kref);
+ rcu_read_unlock();
return deve;
}
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return NULL;
}
-int core_free_device_list_for_node(
+void core_free_device_list_for_node(
struct se_node_acl *nacl,
struct se_portal_group *tpg)
{
struct se_dev_entry *deve;
- struct se_lun *lun;
- u32 i;
-
- if (!nacl->device_list)
- return 0;
- spin_lock_irq(&nacl->device_list_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = nacl->device_list[i];
-
- if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
- continue;
-
- if (!deve->se_lun) {
- pr_err("%s device entries device pointer is"
- " NULL, but Initiator has access.\n",
- tpg->se_tpg_tfo->get_fabric_name());
- continue;
- }
- lun = deve->se_lun;
-
- spin_unlock_irq(&nacl->device_list_lock);
- 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);
-
- array_free(nacl->device_list, TRANSPORT_MAX_LUNS_PER_TPG);
- nacl->device_list = NULL;
-
- return 0;
+ mutex_lock(&nacl->lun_entry_mutex);
+ hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link)
+ core_disable_device_list_for_node(deve->se_lun, deve, nacl, tpg);
+ mutex_unlock(&nacl->lun_entry_mutex);
}
void core_update_device_list_access(
@@ -280,16 +255,40 @@ void core_update_device_list_access(
{
struct se_dev_entry *deve;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[mapped_lun];
- 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;
+ mutex_lock(&nacl->lun_entry_mutex);
+ deve = target_nacl_find_deve(nacl, mapped_lun);
+ if (deve) {
+ 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;
+ }
}
- spin_unlock_irq(&nacl->device_list_lock);
+ mutex_unlock(&nacl->lun_entry_mutex);
+}
+
+/*
+ * Called with rcu_read_lock or nacl->device_list_lock held.
+ */
+struct se_dev_entry *target_nacl_find_deve(struct se_node_acl *nacl, u32 mapped_lun)
+{
+ struct se_dev_entry *deve;
+
+ hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link)
+ if (deve->mapped_lun == mapped_lun)
+ return deve;
+
+ return NULL;
+}
+EXPORT_SYMBOL(target_nacl_find_deve);
+
+void target_pr_kref_release(struct kref *kref)
+{
+ struct se_dev_entry *deve = container_of(kref, struct se_dev_entry,
+ pr_kref);
+ complete(&deve->pr_comp);
}
/* core_enable_device_list_for_node():
@@ -305,85 +304,85 @@ int core_enable_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];
-
- /*
- * Check if the call is handling demo mode -> explicit 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 -> explicit"
- " 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"
- " -> explicit LUN ACL transition\n");
- spin_unlock_irq(&nacl->device_list_lock);
+ struct se_dev_entry *orig, *new;
+
+ new = kzalloc(sizeof(*new), GFP_KERNEL);
+ if (!new) {
+ pr_err("Unable to allocate se_dev_entry memory\n");
+ return -ENOMEM;
+ }
+
+ new->se_node_acl = nacl;
+ atomic_set(&new->ua_count, 0);
+ spin_lock_init(&new->ua_lock);
+ INIT_LIST_HEAD(&new->alua_port_list);
+ INIT_LIST_HEAD(&new->ua_list);
+
+ new->mapped_lun = mapped_lun;
+ kref_init(&new->pr_kref);
+ init_completion(&new->pr_comp);
+
+ if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE)
+ new->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
+ else
+ new->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
+
+ new->creation_time = get_jiffies_64();
+ new->attach_count++;
+
+ mutex_lock(&nacl->lun_entry_mutex);
+ orig = target_nacl_find_deve(nacl, mapped_lun);
+ if (orig && orig->se_lun) {
+ BUG_ON(orig->se_lun_acl != NULL);
+ if (orig->se_lun != lun) {
+ pr_err("Existing orig->se_lun doesn't match new lun"
+ " for dynamic -> explicit NodeACL conversion:"
+ " %s\n", nacl->initiatorname);
+ mutex_unlock(&nacl->lun_entry_mutex);
+ kfree(new);
return -EINVAL;
}
- deve->se_lun_acl = lun_acl;
- 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;
- }
+ rcu_assign_pointer(new->se_lun, lun);
+ rcu_assign_pointer(new->se_lun_acl, lun_acl);
+ hlist_del_rcu(&orig->link);
+ hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
+ mutex_unlock(&nacl->lun_entry_mutex);
- spin_unlock_irq(&nacl->device_list_lock);
- return 0;
- }
+ spin_lock_bh(&port->sep_alua_lock);
+ list_del(&orig->alua_port_list);
+ list_add_tail(&new->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;
+ kref_put(&orig->pr_kref, target_pr_kref_release);
+ wait_for_completion(&orig->pr_comp);
- 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;
+ kfree_rcu(orig, rcu_head);
+ return 0;
}
- deve->creation_time = get_jiffies_64();
- deve->attach_count++;
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_assign_pointer(new->se_lun, lun);
+ rcu_assign_pointer(new->se_lun_acl, lun_acl);
+ hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
+ mutex_unlock(&nacl->lun_entry_mutex);
spin_lock_bh(&port->sep_alua_lock);
- list_add_tail(&deve->alua_port_list, &port->sep_alua_list);
+ list_add_tail(&new->alua_port_list, &port->sep_alua_list);
spin_unlock_bh(&port->sep_alua_lock);
return 0;
}
-/* core_disable_device_list_for_node():
- *
- *
+/*
+ * Called with se_node_acl->lun_entry_mutex held.
*/
-int core_disable_device_list_for_node(
+void core_disable_device_list_for_node(
struct se_lun *lun,
- struct se_lun_acl *lun_acl,
- u32 mapped_lun,
- u32 lun_access,
+ struct se_dev_entry *orig,
struct se_node_acl *nacl,
struct se_portal_group *tpg)
{
struct se_port *port = lun->lun_sep;
- struct se_dev_entry *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
@@ -398,29 +397,30 @@ int core_disable_device_list_for_node(
* MappedLUN *deve will be released below..
*/
spin_lock_bh(&port->sep_alua_lock);
- list_del(&deve->alua_port_list);
+ list_del(&orig->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.
+ * Disable struct se_dev_entry LUN ACL mapping
*/
- while (atomic_read(&deve->pr_ref_count) != 0)
- cpu_relax();
+ core_scsi3_ua_release_all(orig);
- spin_lock_irq(&nacl->device_list_lock);
+ hlist_del_rcu(&orig->link);
+ rcu_assign_pointer(orig->se_lun, NULL);
+ rcu_assign_pointer(orig->se_lun_acl, NULL);
+ orig->lun_flags = 0;
+ orig->creation_time = 0;
+ orig->attach_count--;
+ mutex_unlock(&nacl->lun_entry_mutex);
+
+ kref_put(&orig->pr_kref, target_pr_kref_release);
/*
- * Disable struct se_dev_entry LUN ACL mapping
+ * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
+ * or REGISTER_AND_MOVE PR operation to complete.
*/
- core_scsi3_ua_release_all(deve);
- deve->se_lun = NULL;
- deve->se_lun_acl = NULL;
- deve->lun_flags = 0;
- deve->creation_time = 0;
- deve->attach_count--;
- spin_unlock_irq(&nacl->device_list_lock);
+ wait_for_completion(&orig->pr_comp);
+ kfree_rcu(orig, rcu_head);
core_scsi3_free_pr_reg_from_nacl(lun->lun_se_dev, nacl);
- return 0;
}
/* core_clear_lun_from_tpg():
@@ -431,26 +431,19 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
{
struct se_node_acl *nacl;
struct se_dev_entry *deve;
- u32 i;
spin_lock_irq(&tpg->acl_node_lock);
list_for_each_entry(nacl, &tpg->acl_node_list, acl_list) {
spin_unlock_irq(&tpg->acl_node_lock);
- spin_lock_irq(&nacl->device_list_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = nacl->device_list[i];
+ mutex_lock(&nacl->lun_entry_mutex);
+ hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) {
if (lun != deve->se_lun)
continue;
- spin_unlock_irq(&nacl->device_list_lock);
- core_disable_device_list_for_node(lun, NULL,
- deve->mapped_lun, TRANSPORT_LUNFLAGS_NO_ACCESS,
- nacl, tpg);
-
- spin_lock_irq(&nacl->device_list_lock);
+ core_disable_device_list_for_node(lun, deve, nacl, tpg);
}
- spin_unlock_irq(&nacl->device_list_lock);
+ mutex_unlock(&nacl->lun_entry_mutex);
spin_lock_irq(&tpg->acl_node_lock);
}
@@ -583,6 +576,7 @@ int core_dev_export(
return PTR_ERR(port);
lun->lun_se_dev = dev;
+ lun->lun_rtpi = port->sep_rtpi;
spin_lock(&hba->device_lock);
dev->export_count++;
@@ -1368,16 +1362,13 @@ int core_dev_add_initiator_node_lun_acl(
return 0;
}
-/* core_dev_del_initiator_node_lun_acl():
- *
- *
- */
int core_dev_del_initiator_node_lun_acl(
struct se_portal_group *tpg,
struct se_lun *lun,
struct se_lun_acl *lacl)
{
struct se_node_acl *nacl;
+ struct se_dev_entry *deve;
nacl = lacl->se_lun_nacl;
if (!nacl)
@@ -1388,8 +1379,11 @@ int core_dev_del_initiator_node_lun_acl(
atomic_dec_mb(&lun->lun_acl_count);
spin_unlock(&lun->lun_acl_lock);
- core_disable_device_list_for_node(lun, NULL, lacl->mapped_lun,
- TRANSPORT_LUNFLAGS_NO_ACCESS, nacl, tpg);
+ mutex_lock(&nacl->lun_entry_mutex);
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (deve)
+ core_disable_device_list_for_node(lun, deve, nacl, tpg);
+ mutex_unlock(&nacl->lun_entry_mutex);
lacl->se_lun = NULL;
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 6cb4828..0939a54 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -123,16 +123,16 @@ static int target_fabric_mappedlun_link(
* which be will write protected (READ-ONLY) when
* tpg_1/attrib/demo_mode_write_protect=1
*/
- spin_lock_irq(&lacl->se_lun_nacl->device_list_lock);
- deve = lacl->se_lun_nacl->device_list[lacl->mapped_lun];
- if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS)
+ rcu_read_lock();
+ deve = target_nacl_find_deve(lacl->se_lun_nacl, lacl->mapped_lun);
+ if (deve)
lun_access = deve->lun_flags;
else
lun_access =
(se_tpg->se_tpg_tfo->tpg_check_prod_mode_write_protect(
se_tpg)) ? TRANSPORT_LUNFLAGS_READ_ONLY :
TRANSPORT_LUNFLAGS_READ_WRITE;
- spin_unlock_irq(&lacl->se_lun_nacl->device_list_lock);
+ rcu_read_unlock();
/*
* Determine the actual mapped LUN value user wants..
*
@@ -149,23 +149,13 @@ static int target_fabric_mappedlun_unlink(
struct config_item *lun_acl_ci,
struct config_item *lun_ci)
{
- struct se_lun *lun;
struct se_lun_acl *lacl = container_of(to_config_group(lun_acl_ci),
struct se_lun_acl, se_lun_group);
- struct se_node_acl *nacl = lacl->se_lun_nacl;
- struct se_dev_entry *deve = nacl->device_list[lacl->mapped_lun];
- struct se_portal_group *se_tpg;
- /*
- * Determine if the underlying MappedLUN has already been released..
- */
- if (!deve->se_lun)
- return 0;
-
- lun = container_of(to_config_group(lun_ci), struct se_lun, lun_group);
- se_tpg = lun->lun_sep->sep_tpg;
+ struct se_lun *lun = container_of(to_config_group(lun_ci),
+ struct se_lun, lun_group);
+ struct se_portal_group *se_tpg = lun->lun_sep->sep_tpg;
- core_dev_del_initiator_node_lun_acl(se_tpg, lun, lacl);
- return 0;
+ return core_dev_del_initiator_node_lun_acl(se_tpg, lun, lacl);
}
CONFIGFS_EATTR_STRUCT(target_fabric_mappedlun, se_lun_acl);
@@ -181,14 +171,15 @@ static ssize_t target_fabric_mappedlun_show_write_protect(
{
struct se_node_acl *se_nacl = lacl->se_lun_nacl;
struct se_dev_entry *deve;
- ssize_t len;
-
- spin_lock_irq(&se_nacl->device_list_lock);
- deve = se_nacl->device_list[lacl->mapped_lun];
- len = sprintf(page, "%d\n",
- (deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY) ?
- 1 : 0);
- spin_unlock_irq(&se_nacl->device_list_lock);
+ ssize_t len = 0;
+
+ rcu_read_lock();
+ deve = target_nacl_find_deve(se_nacl, lacl->mapped_lun);
+ if (deve) {
+ len = sprintf(page, "%d\n",
+ (deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY) ? 1 : 0);
+ }
+ rcu_read_unlock();
return len;
}
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index d0344ad..a04a6e3 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -9,13 +9,15 @@ extern struct mutex g_device_mutex;
extern struct list_head g_device_list;
struct se_dev_entry *core_get_se_deve_from_rtpi(struct se_node_acl *, u16);
-int core_free_device_list_for_node(struct se_node_acl *,
+void target_pr_kref_release(struct kref *);
+void core_free_device_list_for_node(struct se_node_acl *,
struct se_portal_group *);
void core_update_device_list_access(u32, u32, struct se_node_acl *);
+struct se_dev_entry *target_nacl_find_deve(struct se_node_acl *, u32);
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_disable_device_list_for_node(struct se_lun *, struct se_dev_entry *,
+ 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_pr.c b/drivers/target/target_core_pr.c
index 8052d40..c0b593a 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -48,7 +48,7 @@ struct pr_transport_id_holder {
struct t10_pr_registration *dest_pr_reg;
struct se_portal_group *dest_tpg;
struct se_node_acl *dest_node_acl;
- struct se_dev_entry *dest_se_deve;
+ struct se_dev_entry __rcu *dest_se_deve;
struct list_head dest_list;
};
@@ -232,7 +232,7 @@ target_scsi2_reservation_release(struct se_cmd *cmd)
tpg = sess->se_tpg;
pr_debug("SCSI-2 Released reservation for %s LUN: %u ->"
" MAPPED LUN: %u for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
- cmd->se_lun->unpacked_lun, cmd->se_deve->mapped_lun,
+ cmd->se_lun->unpacked_lun, cmd->orig_fe_lun,
sess->se_node_acl->initiatorname);
out_unlock:
@@ -281,7 +281,7 @@ target_scsi2_reservation_reserve(struct se_cmd *cmd)
dev->dev_reserved_node_acl->initiatorname);
pr_err("Current attempt - LUN: %u -> MAPPED LUN: %u"
" from %s \n", cmd->se_lun->unpacked_lun,
- cmd->se_deve->mapped_lun,
+ cmd->orig_fe_lun,
sess->se_node_acl->initiatorname);
ret = TCM_RESERVATION_CONFLICT;
goto out_unlock;
@@ -295,7 +295,7 @@ target_scsi2_reservation_reserve(struct se_cmd *cmd)
}
pr_debug("SCSI-2 Reserved %s LUN: %u -> MAPPED LUN: %u"
" for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
- cmd->se_lun->unpacked_lun, cmd->se_deve->mapped_lun,
+ cmd->se_lun->unpacked_lun, cmd->orig_fe_lun,
sess->se_node_acl->initiatorname);
out_unlock:
@@ -320,6 +320,7 @@ static int core_scsi3_pr_seq_non_holder(
unsigned char *cdb = cmd->t_task_cdb;
struct se_dev_entry *se_deve;
struct se_session *se_sess = cmd->se_sess;
+ struct se_node_acl *nacl = se_sess->se_node_acl;
int other_cdb = 0, ignore_reg;
int registered_nexus = 0, ret = 1; /* Conflict by default */
int all_reg = 0, reg_only = 0; /* ALL_REG, REG_ONLY */
@@ -327,7 +328,8 @@ static int core_scsi3_pr_seq_non_holder(
int legacy = 0; /* Act like a legacy device and return
* RESERVATION CONFLICT on some CDBs */
- se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
+ rcu_read_lock();
+ se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
/*
* Determine if the registration should be ignored due to
* non-matching ISIDs in target_scsi3_pr_reservation_check().
@@ -368,8 +370,10 @@ static int core_scsi3_pr_seq_non_holder(
registered_nexus = 1;
break;
default:
+ rcu_read_unlock();
return -EINVAL;
}
+ rcu_read_unlock();
/*
* Referenced from spc4r17 table 45 for *NON* PR holder access
*/
@@ -735,7 +739,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
if (strcmp(nacl->initiatorname, nacl_tmp->initiatorname))
continue;
- atomic_inc_mb(&deve_tmp->pr_ref_count);
+ kref_get(&deve_tmp->pr_kref);
spin_unlock_bh(&port->sep_alua_lock);
/*
* Grab a configfs group dependency that is released
@@ -748,7 +752,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
pr_err("core_scsi3_lunacl_depend"
"_item() failed\n");
atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
- atomic_dec_mb(&deve_tmp->pr_ref_count);
+ kref_put(&deve_tmp->pr_kref, target_pr_kref_release);
goto out;
}
/*
@@ -763,7 +767,6 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
sa_res_key, all_tg_pt, aptpl);
if (!pr_reg_atp) {
atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
- atomic_dec_mb(&deve_tmp->pr_ref_count);
core_scsi3_lunacl_undepend_item(deve_tmp);
goto out;
}
@@ -896,7 +899,7 @@ static int __core_scsi3_check_aptpl_registration(
struct se_lun *lun,
u32 target_lun,
struct se_node_acl *nacl,
- struct se_dev_entry *deve)
+ u32 mapped_lun)
{
struct t10_pr_registration *pr_reg, *pr_reg_tmp;
struct t10_reservation *pr_tmpl = &dev->t10_pr;
@@ -924,13 +927,12 @@ static int __core_scsi3_check_aptpl_registration(
pr_reg_aptpl_list) {
if (!strcmp(pr_reg->pr_iport, i_port) &&
- (pr_reg->pr_res_mapped_lun == deve->mapped_lun) &&
+ (pr_reg->pr_res_mapped_lun == mapped_lun) &&
!(strcmp(pr_reg->pr_tport, t_port)) &&
(pr_reg->pr_reg_tpgt == tpgt) &&
(pr_reg->pr_aptpl_target_lun == target_lun)) {
pr_reg->pr_reg_nacl = nacl;
- pr_reg->pr_reg_deve = deve;
pr_reg->pr_reg_tg_pt_lun = lun;
list_del(&pr_reg->pr_reg_aptpl_list);
@@ -968,13 +970,12 @@ int core_scsi3_check_aptpl_registration(
struct se_node_acl *nacl,
u32 mapped_lun)
{
- struct se_dev_entry *deve = nacl->device_list[mapped_lun];
-
if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return 0;
return __core_scsi3_check_aptpl_registration(dev, tpg, lun,
- lun->unpacked_lun, nacl, deve);
+ lun->unpacked_lun, nacl,
+ mapped_lun);
}
static void __core_scsi3_dump_registration(
@@ -1066,6 +1067,7 @@ static void __core_scsi3_add_registration(
pr_reg_tmp->pr_reg_nacl, pr_reg_tmp,
register_type);
spin_unlock(&pr_tmpl->registration_lock);
+
/*
* Drop configfs group dependency reference from
* __core_scsi3_alloc_registration()
@@ -1408,27 +1410,35 @@ static int core_scsi3_lunacl_depend_item(struct se_dev_entry *se_deve)
static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
{
- struct se_lun_acl *lun_acl = se_deve->se_lun_acl;
+ struct se_lun_acl *lun_acl;
struct se_node_acl *nacl;
struct se_portal_group *tpg;
+
+ if (!se_deve) {
+ pr_err("core_scsi3_lunacl_undepend_item passed NULL se_deve\n");
+ dump_stack();
+ return;
+ }
/*
* For nacl->dynamic_node_acl=1
*/
+ lun_acl = se_deve->se_lun_acl;
if (!lun_acl) {
- atomic_dec_mb(&se_deve->pr_ref_count);
+ kref_put(&se_deve->pr_kref, target_pr_kref_release);
return;
}
nacl = lun_acl->se_lun_nacl;
tpg = nacl->se_tpg;
target_undepend_item(&lun_acl->se_lun_group.cg_item);
- atomic_dec_mb(&se_deve->pr_ref_count);
+ kref_put(&se_deve->pr_kref, target_pr_kref_release);
}
static sense_reason_t
core_scsi3_decode_spec_i_port(
struct se_cmd *cmd,
struct se_portal_group *tpg,
+ struct se_dev_entry *local_se_deve,
unsigned char *l_isid,
u64 sa_res_key,
int all_tg_pt,
@@ -1439,7 +1449,7 @@ core_scsi3_decode_spec_i_port(
struct se_portal_group *dest_tpg = NULL, *tmp_tpg;
struct se_session *se_sess = cmd->se_sess;
struct se_node_acl *dest_node_acl = NULL;
- struct se_dev_entry *dest_se_deve = NULL, *local_se_deve;
+ struct se_dev_entry __rcu *dest_se_deve = NULL;
struct t10_pr_registration *dest_pr_reg, *local_pr_reg, *pr_reg_e;
struct t10_pr_registration *pr_reg_tmp, *pr_reg_tmp_safe;
LIST_HEAD(tid_dest_list);
@@ -1452,7 +1462,6 @@ core_scsi3_decode_spec_i_port(
int dest_local_nexus;
u32 dest_rtpi = 0;
- local_se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
/*
* Allocate a struct pr_transport_id_holder and setup the
* local_node_acl and local_se_deve pointers and add to
@@ -1467,7 +1476,6 @@ core_scsi3_decode_spec_i_port(
INIT_LIST_HEAD(&tidh_new->dest_list);
tidh_new->dest_tpg = tpg;
tidh_new->dest_node_acl = se_sess->se_node_acl;
- tidh_new->dest_se_deve = local_se_deve;
local_pr_reg = __core_scsi3_alloc_registration(cmd->se_dev,
se_sess->se_node_acl, local_se_deve, l_isid,
@@ -1476,6 +1484,7 @@ core_scsi3_decode_spec_i_port(
kfree(tidh_new);
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
+ rcu_assign_pointer(tidh_new->dest_se_deve, local_se_deve);
tidh_new->dest_pr_reg = local_pr_reg;
/*
* The local I_T nexus does not hold any configfs dependances,
@@ -1635,7 +1644,7 @@ core_scsi3_decode_spec_i_port(
if (core_scsi3_lunacl_depend_item(dest_se_deve)) {
pr_err("core_scsi3_lunacl_depend_item()"
" failed\n");
- atomic_dec_mb(&dest_se_deve->pr_ref_count);
+ kref_put(&dest_se_deve->pr_kref, target_pr_kref_release);
core_scsi3_nodeacl_undepend_item(dest_node_acl);
core_scsi3_tpg_undepend_item(dest_tpg);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -1990,6 +1999,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
bool aptpl, bool all_tg_pt, bool spec_i_pt, enum register_type register_type)
{
struct se_session *se_sess = cmd->se_sess;
+ struct se_node_acl *nacl = se_sess->se_node_acl;
struct se_device *dev = cmd->se_dev;
struct se_dev_entry *se_deve;
struct se_lun *se_lun = cmd->se_lun;
@@ -2005,7 +2015,14 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
se_tpg = se_sess->se_tpg;
- se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
+
+ rcu_read_lock();
+ se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (!se_deve) {
+ pr_err("Unable to locate se_deve for PRO-REGISTER\n");
+ rcu_read_unlock();
+ return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+ }
if (se_tpg->se_tpg_tfo->sess_get_initiator_sid) {
memset(&isid_buf[0], 0, PR_REG_ISID_LEN);
@@ -2021,14 +2038,16 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
if (res_key) {
pr_warn("SPC-3 PR: Reservation Key non-zero"
" for SA REGISTER, returning CONFLICT\n");
+ rcu_read_unlock();
return TCM_RESERVATION_CONFLICT;
}
/*
* Do nothing but return GOOD status.
*/
- if (!sa_res_key)
+ if (!sa_res_key) {
+ rcu_read_unlock();
return 0;
-
+ }
if (!spec_i_pt) {
/*
* Perform the Service Action REGISTER on the Initiator
@@ -2041,6 +2060,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
register_type, 0)) {
pr_err("Unable to allocate"
" struct t10_pr_registration\n");
+ rcu_read_unlock();
return TCM_INVALID_PARAMETER_LIST;
}
} else {
@@ -2052,14 +2072,17 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
* logic from of core_scsi3_alloc_registration() for
* each TransportID provided SCSI Initiator Port/Device
*/
- ret = core_scsi3_decode_spec_i_port(cmd, se_tpg,
+ ret = core_scsi3_decode_spec_i_port(cmd, se_tpg, se_deve,
isid_ptr, sa_res_key, all_tg_pt, aptpl);
- if (ret != 0)
+ if (ret != 0) {
+ rcu_read_unlock();
return ret;
+ }
}
-
+ rcu_read_unlock();
return core_scsi3_update_and_write_aptpl(dev, aptpl);
}
+ rcu_read_unlock();
/* ok, existing registration */
@@ -3321,7 +3344,7 @@ after_iport_check:
if (core_scsi3_lunacl_depend_item(dest_se_deve)) {
pr_err("core_scsi3_lunacl_depend_item() failed\n");
- atomic_dec_mb(&dest_se_deve->pr_ref_count);
+ kref_put(&dest_se_deve->pr_kref, target_pr_kref_release);
dest_se_deve = NULL;
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto out;
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index f6c954c..7579357 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -47,6 +47,7 @@
#include <target/target_core_backend_configfs.h>
#include "target_core_alua.h"
+#include "target_core_internal.h"
#include "target_core_pscsi.h"
#define ISPRINT(a) ((a >= ' ') && (a <= '~'))
@@ -634,12 +635,14 @@ static void pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg,
* Hack to make sure that Write-Protect modepage is set if R/O mode is
* forced.
*/
- if (!cmd->se_deve || !cmd->data_length)
+ if (!cmd->data_length)
goto after_mode_sense;
if (((cdb[0] == MODE_SENSE) || (cdb[0] == MODE_SENSE_10)) &&
(status_byte(result) << 1) == SAM_STAT_GOOD) {
- if (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY) {
+ bool read_only = target_lun_is_rdonly(cmd);
+
+ if (read_only) {
unsigned char *buf;
buf = transport_kmap_data_sg(cmd);
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 78c0b40..9f995b8 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -981,6 +981,7 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
int length = 0;
int ret;
int i;
+ bool read_only = target_lun_is_rdonly(cmd);;
memset(buf, 0, SE_MODE_PAGE_BUF);
@@ -991,9 +992,7 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
length = ten ? 3 : 2;
/* DEVICE-SPECIFIC PARAMETER */
- if ((cmd->se_lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY) ||
- (cmd->se_deve &&
- (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY)))
+ if ((cmd->se_lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY) || read_only)
spc_modesense_write_protect(&buf[length], type);
/*
@@ -1211,8 +1210,9 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
{
struct se_dev_entry *deve;
struct se_session *sess = cmd->se_sess;
+ struct se_node_acl *nacl;
unsigned char *buf;
- u32 lun_count = 0, offset = 8, i;
+ u32 lun_count = 0, offset = 8;
if (cmd->data_length < 16) {
pr_warn("REPORT LUNS allocation length %u too small\n",
@@ -1234,12 +1234,10 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
lun_count = 1;
goto done;
}
+ nacl = sess->se_node_acl;
- spin_lock_irq(&sess->se_node_acl->device_list_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = sess->se_node_acl->device_list[i];
- if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
- continue;
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) {
/*
* We determine the correct LUN LIST LENGTH even once we
* have reached the initial allocation length.
@@ -1252,7 +1250,7 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
int_to_scsilun(deve->mapped_lun, (struct scsi_lun *)&buf[offset]);
offset += 8;
}
- spin_unlock_irq(&sess->se_node_acl->device_list_lock);
+ rcu_read_unlock();
/*
* See SPC3 r07, page 159.
diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index 64efee2..7b7b524 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -1084,17 +1084,17 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_inst(
struct se_portal_group *tpg;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
tpg = nacl->se_tpg;
/* scsiInstIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n",
tpg->se_tpg_tfo->tpg_get_inst_index(tpg));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(inst);
@@ -1109,16 +1109,16 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_dev(
struct se_lun *lun;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
lun = deve->se_lun;
/* scsiDeviceIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_se_dev->dev_index);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(dev);
@@ -1133,16 +1133,16 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_port(
struct se_portal_group *tpg;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
tpg = nacl->se_tpg;
/* scsiAuthIntrTgtPortIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(port);
@@ -1156,15 +1156,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_indx(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", nacl->acl_index);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(indx);
@@ -1178,15 +1178,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_dev_or_port(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrDevOrPort */
ret = snprintf(page, PAGE_SIZE, "%u\n", 1);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(dev_or_port);
@@ -1200,15 +1200,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_intr_name(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrName */
ret = snprintf(page, PAGE_SIZE, "%s\n", nacl->initiatorname);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(intr_name);
@@ -1222,15 +1222,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_map_indx(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* FIXME: scsiAuthIntrLunMapIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", 0);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(map_indx);
@@ -1244,15 +1244,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_att_count(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrAttachedTimes */
ret = snprintf(page, PAGE_SIZE, "%u\n", deve->attach_count);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(att_count);
@@ -1266,15 +1266,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_num_cmds(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrOutCommands */
ret = snprintf(page, PAGE_SIZE, "%u\n", deve->total_cmds);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(num_cmds);
@@ -1288,15 +1288,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_read_mbytes(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrReadMegaBytes */
ret = snprintf(page, PAGE_SIZE, "%u\n", (u32)(deve->read_bytes >> 20));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(read_mbytes);
@@ -1310,15 +1310,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_write_mbytes(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrWrittenMegaBytes */
ret = snprintf(page, PAGE_SIZE, "%u\n", (u32)(deve->write_bytes >> 20));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(write_mbytes);
@@ -1332,15 +1332,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_hs_num_cmds(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* FIXME: scsiAuthIntrHSOutCommands */
ret = snprintf(page, PAGE_SIZE, "%u\n", 0);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(hs_num_cmds);
@@ -1354,16 +1354,16 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_creation_time(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAuthIntrLastCreation */
ret = snprintf(page, PAGE_SIZE, "%u\n", (u32)(((u32)deve->creation_time -
INITIAL_JIFFIES) * 100 / HZ));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(creation_time);
@@ -1377,15 +1377,15 @@ static ssize_t target_stat_scsi_auth_intr_show_attr_row_status(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* FIXME: scsiAuthIntrRowStatus */
ret = snprintf(page, PAGE_SIZE, "Ready\n");
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_AUTH_INTR_ATTR_RO(row_status);
@@ -1450,17 +1450,17 @@ static ssize_t target_stat_scsi_att_intr_port_show_attr_inst(
struct se_portal_group *tpg;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
tpg = nacl->se_tpg;
/* scsiInstIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n",
tpg->se_tpg_tfo->tpg_get_inst_index(tpg));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_ATTR_INTR_PORT_ATTR_RO(inst);
@@ -1475,16 +1475,16 @@ static ssize_t target_stat_scsi_att_intr_port_show_attr_dev(
struct se_lun *lun;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
lun = deve->se_lun;
/* scsiDeviceIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_se_dev->dev_index);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_ATTR_INTR_PORT_ATTR_RO(dev);
@@ -1499,16 +1499,16 @@ static ssize_t target_stat_scsi_att_intr_port_show_attr_port(
struct se_portal_group *tpg;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
tpg = nacl->se_tpg;
/* scsiPortIndex */
ret = snprintf(page, PAGE_SIZE, "%u\n", tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_ATTR_INTR_PORT_ATTR_RO(port);
@@ -1548,15 +1548,15 @@ static ssize_t target_stat_scsi_att_intr_port_show_attr_port_auth_indx(
struct se_dev_entry *deve;
ssize_t ret;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[lacl->mapped_lun];
- if (!deve->se_lun || !deve->se_lun_acl) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
+ if (!deve) {
+ rcu_read_unlock();
return -ENODEV;
}
/* scsiAttIntrPortAuthIntrIdx */
ret = snprintf(page, PAGE_SIZE, "%u\n", nacl->acl_index);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return ret;
}
DEV_STAT_SCSI_ATTR_INTR_PORT_ATTR_RO(port_auth_indx);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index c0c1f67..0519923 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -47,42 +47,6 @@ extern struct se_device *g_lun0_dev;
static DEFINE_SPINLOCK(tpg_lock);
static LIST_HEAD(tpg_list);
-/* core_clear_initiator_node_from_tpg():
- *
- *
- */
-static void core_clear_initiator_node_from_tpg(
- struct se_node_acl *nacl,
- struct se_portal_group *tpg)
-{
- int i;
- struct se_dev_entry *deve;
- struct se_lun *lun;
-
- spin_lock_irq(&nacl->device_list_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = nacl->device_list[i];
-
- if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
- continue;
-
- if (!deve->se_lun) {
- pr_err("%s device entries device pointer is"
- " NULL, but Initiator has access.\n",
- tpg->se_tpg_tfo->get_fabric_name());
- continue;
- }
-
- lun = deve->se_lun;
- spin_unlock_irq(&nacl->device_list_lock);
- 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);
-}
-
/* __core_tpg_get_initiator_node_acl():
*
* spin_lock_bh(&tpg->acl_node_lock); must be held when calling
@@ -225,35 +189,6 @@ static void *array_zalloc(int n, size_t size, gfp_t flags)
return a;
}
-/* core_create_device_list_for_node():
- *
- *
- */
-static int core_create_device_list_for_node(struct se_node_acl *nacl)
-{
- struct se_dev_entry *deve;
- int i;
-
- nacl->device_list = array_zalloc(TRANSPORT_MAX_LUNS_PER_TPG,
- sizeof(struct se_dev_entry), GFP_KERNEL);
- if (!nacl->device_list) {
- pr_err("Unable to allocate memory for"
- " struct se_node_acl->device_list\n");
- return -ENOMEM;
- }
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- deve = nacl->device_list[i];
-
- atomic_set(&deve->ua_count, 0);
- atomic_set(&deve->pr_ref_count, 0);
- spin_lock_init(&deve->ua_lock);
- INIT_LIST_HEAD(&deve->alua_port_list);
- INIT_LIST_HEAD(&deve->ua_list);
- }
-
- return 0;
-}
-
static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg,
const unsigned char *initiatorname)
{
@@ -266,10 +201,11 @@ static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg,
INIT_LIST_HEAD(&acl->acl_list);
INIT_LIST_HEAD(&acl->acl_sess_list);
+ INIT_HLIST_HEAD(&acl->lun_entry_hlist);
kref_init(&acl->acl_kref);
init_completion(&acl->acl_free_comp);
- spin_lock_init(&acl->device_list_lock);
spin_lock_init(&acl->nacl_sess_lock);
+ mutex_init(&acl->lun_entry_mutex);
atomic_set(&acl->acl_pr_ref_count, 0);
if (tpg->se_tpg_tfo->tpg_get_default_depth)
acl->queue_depth = tpg->se_tpg_tfo->tpg_get_default_depth(tpg);
@@ -281,15 +217,11 @@ static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg,
tpg->se_tpg_tfo->set_default_node_attributes(acl);
- if (core_create_device_list_for_node(acl) < 0)
- goto out_free_acl;
if (core_set_queue_depth_for_node(tpg, acl) < 0)
- goto out_free_device_list;
+ goto out_free_acl;
return acl;
-out_free_device_list:
- core_free_device_list_for_node(acl, tpg);
out_free_acl:
kfree(acl);
return NULL;
@@ -454,7 +386,6 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl *acl)
wait_for_completion(&acl->acl_free_comp);
core_tpg_wait_for_nacl_pr_ref(acl);
- core_clear_initiator_node_from_tpg(acl, tpg);
core_free_device_list_for_node(acl, tpg);
pr_debug("%s_TPG[%hu] - Deleted ACL with TCQ Depth: %d for %s"
diff --git a/drivers/target/target_core_ua.c b/drivers/target/target_core_ua.c
index a0bf0d1..6c9616d 100644
--- a/drivers/target/target_core_ua.c
+++ b/drivers/target/target_core_ua.c
@@ -50,9 +50,17 @@ target_scsi3_ua_check(struct se_cmd *cmd)
if (!nacl)
return 0;
- deve = nacl->device_list[cmd->orig_fe_lun];
- if (!atomic_read(&deve->ua_count))
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (!deve) {
+ rcu_read_unlock();
return 0;
+ }
+ if (!atomic_read(&deve->ua_count)) {
+ rcu_read_unlock();
+ return 0;
+ }
+ rcu_read_unlock();
/*
* From sam4r14, section 5.14 Unit attention condition:
*
@@ -103,9 +111,12 @@ int core_scsi3_ua_allocate(
ua->ua_asc = asc;
ua->ua_ascq = ascq;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[unpacked_lun];
-
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, unpacked_lun);
+ if (!deve) {
+ rcu_read_unlock();
+ return -EINVAL;
+ }
spin_lock(&deve->ua_lock);
list_for_each_entry_safe(ua_p, ua_tmp, &deve->ua_list, ua_nacl_list) {
/*
@@ -113,7 +124,7 @@ int core_scsi3_ua_allocate(
*/
if ((ua_p->ua_asc == asc) && (ua_p->ua_ascq == ascq)) {
spin_unlock(&deve->ua_lock);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
kmem_cache_free(se_ua_cache, ua);
return 0;
}
@@ -158,14 +169,13 @@ int core_scsi3_ua_allocate(
list_add_tail(&ua->ua_nacl_list,
&deve->ua_list);
spin_unlock(&deve->ua_lock);
- spin_unlock_irq(&nacl->device_list_lock);
atomic_inc_mb(&deve->ua_count);
+ rcu_read_unlock();
return 0;
}
list_add_tail(&ua->ua_nacl_list, &deve->ua_list);
spin_unlock(&deve->ua_lock);
- spin_unlock_irq(&nacl->device_list_lock);
pr_debug("[%s]: Allocated UNIT ATTENTION, mapped LUN: %u, ASC:"
" 0x%02x, ASCQ: 0x%02x\n",
@@ -173,6 +183,7 @@ int core_scsi3_ua_allocate(
asc, ascq);
atomic_inc_mb(&deve->ua_count);
+ rcu_read_unlock();
return 0;
}
@@ -210,10 +221,14 @@ void core_scsi3_ua_for_check_condition(
if (!nacl)
return;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[cmd->orig_fe_lun];
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (!deve) {
+ rcu_read_unlock();
+ return;
+ }
if (!atomic_read(&deve->ua_count)) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return;
}
/*
@@ -249,7 +264,7 @@ void core_scsi3_ua_for_check_condition(
atomic_dec_mb(&deve->ua_count);
}
spin_unlock(&deve->ua_lock);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
pr_debug("[%s]: %s UNIT ATTENTION condition with"
" INTLCK_CTRL: %d, mapped LUN: %u, got CDB: 0x%02x"
@@ -278,10 +293,14 @@ int core_scsi3_ua_clear_for_request_sense(
if (!nacl)
return -EINVAL;
- spin_lock_irq(&nacl->device_list_lock);
- deve = nacl->device_list[cmd->orig_fe_lun];
+ rcu_read_lock();
+ deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (!deve) {
+ rcu_read_unlock();
+ return -EINVAL;
+ }
if (!atomic_read(&deve->ua_count)) {
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
return -EPERM;
}
/*
@@ -307,7 +326,7 @@ int core_scsi3_ua_clear_for_request_sense(
atomic_dec_mb(&deve->ua_count);
}
spin_unlock(&deve->ua_lock);
- spin_unlock_irq(&nacl->device_list_lock);
+ rcu_read_unlock();
pr_debug("[%s]: Released UNIT ATTENTION condition, mapped"
" LUN: %u, got REQUEST_SENSE reported ASC: 0x%02x,"
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index ab8ed4c..b688826 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -103,7 +103,7 @@ int target_alloc_sgl(struct scatterlist **, unsigned int *, u32, bool);
sense_reason_t transport_generic_map_mem_to_cmd(struct se_cmd *,
struct scatterlist *, u32, struct scatterlist *, u32);
-void array_free(void *array, int n);
+bool target_lun_is_rdonly(struct se_cmd *);
/* From target_core_configfs.c to setup default backend config_item_types */
void target_core_setup_sub_cits(struct se_subsystem_api *);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 042a734..e2c0eaf 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -160,10 +160,8 @@ enum se_cmd_flags_table {
/* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
enum transport_lunflags_table {
- TRANSPORT_LUNFLAGS_NO_ACCESS = 0x00,
- TRANSPORT_LUNFLAGS_INITIATOR_ACCESS = 0x01,
- TRANSPORT_LUNFLAGS_READ_ONLY = 0x02,
- TRANSPORT_LUNFLAGS_READ_WRITE = 0x04,
+ TRANSPORT_LUNFLAGS_READ_ONLY = 0x01,
+ TRANSPORT_LUNFLAGS_READ_WRITE = 0x02,
};
/*
@@ -584,10 +582,10 @@ struct se_node_acl {
char acl_tag[MAX_ACL_TAG_SIZE];
/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
atomic_t acl_pr_ref_count;
- struct se_dev_entry **device_list;
+ struct hlist_head lun_entry_hlist;
struct se_session *nacl_sess;
struct se_portal_group *se_tpg;
- spinlock_t device_list_lock;
+ struct mutex lun_entry_mutex;
spinlock_t nacl_sess_lock;
struct config_group acl_group;
struct config_group acl_attrib_group;
@@ -652,12 +650,16 @@ struct se_dev_entry {
u64 write_bytes;
atomic_t ua_count;
/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
- atomic_t pr_ref_count;
- struct se_lun_acl *se_lun_acl;
+ struct kref pr_kref;
+ struct completion pr_comp;
+ struct se_node_acl *se_node_acl;
+ struct se_lun_acl __rcu *se_lun_acl;
spinlock_t ua_lock;
struct se_lun *se_lun;
struct list_head alua_port_list;
struct list_head ua_list;
+ struct hlist_node link;
+ struct rcu_head rcu_head;
};
struct se_dev_attrib {
@@ -703,6 +705,7 @@ struct se_port_stat_grps {
};
struct se_lun {
+ u16 lun_rtpi;
#define SE_LUN_LINK_MAGIC 0xffff7771
u32 lun_link_magic;
/* See transport_lun_status_table */
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch converts the core_scsi3_pr_seq_non_holder() check for non
reservation holding registrations to use an atomic bitop.
It also includes associated set_bit() in __core_scsi3_add_registration()
and clear_bit() __core_scsi3_free_registration() for the updater path.
Reported-by: Christoph Hellwig <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_device.c | 1 +
drivers/target/target_core_pr.c | 57 ++++++++++++++++++++++++-------------
include/target/target_core_base.h | 4 +--
3 files changed, 41 insertions(+), 21 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index cfe5cd3..6432d26 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -405,6 +405,7 @@ void core_disable_device_list_for_node(
core_scsi3_ua_release_all(orig);
hlist_del_rcu(&orig->link);
+ clear_bit(1, &orig->pr_reg);
rcu_assign_pointer(orig->se_lun, NULL);
rcu_assign_pointer(orig->se_lun_acl, NULL);
orig->lun_flags = 0;
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index c0b593a..d29b39c 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -327,9 +327,13 @@ static int core_scsi3_pr_seq_non_holder(
int we = 0; /* Write Exclusive */
int legacy = 0; /* Act like a legacy device and return
* RESERVATION CONFLICT on some CDBs */
+ bool registered = false;
rcu_read_lock();
se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (se_deve)
+ registered = test_bit(1, &se_deve->pr_reg);
+ rcu_read_unlock();
/*
* Determine if the registration should be ignored due to
* non-matching ISIDs in target_scsi3_pr_reservation_check().
@@ -346,7 +350,7 @@ static int core_scsi3_pr_seq_non_holder(
* Some commands are only allowed for the persistent reservation
* holder.
*/
- if ((se_deve->def_pr_registered) && !(ignore_reg))
+ if ((registered) && !(ignore_reg))
registered_nexus = 1;
break;
case PR_TYPE_WRITE_EXCLUSIVE_REGONLY:
@@ -356,7 +360,7 @@ static int core_scsi3_pr_seq_non_holder(
* Some commands are only allowed for registered I_T Nexuses.
*/
reg_only = 1;
- if ((se_deve->def_pr_registered) && !(ignore_reg))
+ if ((registered) && !(ignore_reg))
registered_nexus = 1;
break;
case PR_TYPE_WRITE_EXCLUSIVE_ALLREG:
@@ -366,14 +370,12 @@ static int core_scsi3_pr_seq_non_holder(
* Each registered I_T Nexus is a reservation holder.
*/
all_reg = 1;
- if ((se_deve->def_pr_registered) && !(ignore_reg))
+ if ((registered) && !(ignore_reg))
registered_nexus = 1;
break;
default:
- rcu_read_unlock();
return -EINVAL;
}
- rcu_read_unlock();
/*
* Referenced from spc4r17 table 45 for *NON* PR holder access
*/
@@ -1009,10 +1011,6 @@ static void __core_scsi3_dump_registration(
pr_reg->pr_reg_aptpl);
}
-/*
- * this function can be called with struct se_device->dev_reservation_lock
- * when register_move = 1
- */
static void __core_scsi3_add_registration(
struct se_device *dev,
struct se_node_acl *nacl,
@@ -1023,6 +1021,7 @@ static void __core_scsi3_add_registration(
const struct target_core_fabric_ops *tfo = nacl->se_tpg->se_tpg_tfo;
struct t10_pr_registration *pr_reg_tmp, *pr_reg_tmp_safe;
struct t10_reservation *pr_tmpl = &dev->t10_pr;
+ struct se_dev_entry *deve;
/*
* Increment PRgeneration counter for struct se_device upon a successful
@@ -1039,10 +1038,16 @@ static void __core_scsi3_add_registration(
spin_lock(&pr_tmpl->registration_lock);
list_add_tail(&pr_reg->pr_reg_list, &pr_tmpl->registration_list);
- pr_reg->pr_reg_deve->def_pr_registered = 1;
__core_scsi3_dump_registration(tfo, dev, nacl, pr_reg, register_type);
spin_unlock(&pr_tmpl->registration_lock);
+
+ mutex_lock(&nacl->lun_entry_mutex);
+ deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
+ if (deve)
+ set_bit(1, &deve->pr_reg);
+ mutex_unlock(&nacl->lun_entry_mutex);
+
/*
* Skip extra processing for ALL_TG_PT=0 or REGISTER_AND_MOVE.
*/
@@ -1054,6 +1059,8 @@ static void __core_scsi3_add_registration(
*/
list_for_each_entry_safe(pr_reg_tmp, pr_reg_tmp_safe,
&pr_reg->pr_reg_atp_list, pr_reg_atp_mem_list) {
+ struct se_node_acl *nacl_tmp = pr_reg_tmp->pr_reg_nacl;
+
list_del(&pr_reg_tmp->pr_reg_atp_mem_list);
pr_reg_tmp->pr_res_generation = core_scsi3_pr_generation(dev);
@@ -1061,13 +1068,17 @@ static void __core_scsi3_add_registration(
spin_lock(&pr_tmpl->registration_lock);
list_add_tail(&pr_reg_tmp->pr_reg_list,
&pr_tmpl->registration_list);
- pr_reg_tmp->pr_reg_deve->def_pr_registered = 1;
- __core_scsi3_dump_registration(tfo, dev,
- pr_reg_tmp->pr_reg_nacl, pr_reg_tmp,
- register_type);
+ __core_scsi3_dump_registration(tfo, dev, nacl_tmp, pr_reg_tmp,
+ register_type);
spin_unlock(&pr_tmpl->registration_lock);
+ mutex_lock(&nacl->lun_entry_mutex);
+ deve = target_nacl_find_deve(nacl_tmp, pr_reg_tmp->pr_res_mapped_lun);
+ if (deve)
+ set_bit(1, &deve->pr_reg);
+ mutex_unlock(&nacl->lun_entry_mutex);
+
/*
* Drop configfs group dependency reference from
* __core_scsi3_alloc_registration()
@@ -1243,13 +1254,13 @@ static void __core_scsi3_free_registration(
const struct target_core_fabric_ops *tfo =
pr_reg->pr_reg_nacl->se_tpg->se_tpg_tfo;
struct t10_reservation *pr_tmpl = &dev->t10_pr;
+ struct se_node_acl *nacl = pr_reg->pr_reg_nacl;
+ struct se_dev_entry *deve;
char i_buf[PR_REG_ISID_ID_LEN];
memset(i_buf, 0, PR_REG_ISID_ID_LEN);
core_pr_dump_initiator_port(pr_reg, i_buf, PR_REG_ISID_ID_LEN);
- pr_reg->pr_reg_deve->def_pr_registered = 0;
- pr_reg->pr_reg_deve->pr_res_key = 0;
if (!list_empty(&pr_reg->pr_reg_list))
list_del(&pr_reg->pr_reg_list);
/*
@@ -1258,6 +1269,8 @@ static void __core_scsi3_free_registration(
*/
if (dec_holders)
core_scsi3_put_pr_reg(pr_reg);
+
+ spin_unlock(&pr_tmpl->registration_lock);
/*
* Wait until all reference from any other I_T nexuses for this
* *pr_reg have been released. Because list_del() is called above,
@@ -1265,13 +1278,18 @@ static void __core_scsi3_free_registration(
* count back to zero, and we release *pr_reg.
*/
while (atomic_read(&pr_reg->pr_res_holders) != 0) {
- spin_unlock(&pr_tmpl->registration_lock);
pr_debug("SPC-3 PR [%s] waiting for pr_res_holders\n",
tfo->get_fabric_name());
cpu_relax();
- spin_lock(&pr_tmpl->registration_lock);
}
+ mutex_lock(&nacl->lun_entry_mutex);
+ deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
+ if (deve)
+ clear_bit(1, &deve->pr_reg);
+ mutex_unlock(&nacl->lun_entry_mutex);
+
+ spin_lock(&pr_tmpl->registration_lock);
pr_debug("SPC-3 PR [%s] Service Action: UNREGISTER Initiator"
" Node: %s%s\n", tfo->get_fabric_name(),
pr_reg->pr_reg_nacl->initiatorname,
@@ -3428,13 +3446,14 @@ after_iport_check:
dest_pr_reg = __core_scsi3_locate_pr_reg(dev, dest_node_acl,
iport_ptr);
if (!dest_pr_reg) {
+ spin_unlock(&dev->dev_reservation_lock);
if (core_scsi3_alloc_registration(cmd->se_dev,
dest_node_acl, dest_se_deve, iport_ptr,
sa_res_key, 0, aptpl, 2, 1)) {
- spin_unlock(&dev->dev_reservation_lock);
ret = TCM_INVALID_PARAMETER_LIST;
goto out;
}
+ spin_lock(&dev->dev_reservation_lock);
dest_pr_reg = __core_scsi3_locate_pr_reg(dev, dest_node_acl,
iport_ptr);
new_reg = 1;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index e2c0eaf..def5bc8 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -638,7 +638,6 @@ struct se_lun_acl {
};
struct se_dev_entry {
- bool def_pr_registered;
/* See transport_lunflags_table */
u32 lun_flags;
u32 mapped_lun;
@@ -655,7 +654,8 @@ struct se_dev_entry {
struct se_node_acl *se_node_acl;
struct se_lun_acl __rcu *se_lun_acl;
spinlock_t ua_lock;
- struct se_lun *se_lun;
+ struct se_lun __rcu *se_lun;
+ unsigned long pr_reg;
struct list_head alua_port_list;
struct list_head ua_list;
struct hlist_node link;
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch changes __core_scsi3_do_alloc_registration() code to
drop pr_reg->pr_reg_tg_pt_lun pointer usage in favor of a new
pr_reg RPTI + existing target_lun.
It also includes changes to REGISTER, REGISTER_AND_MOVE and APTPL
feature bit codepaths.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_configfs.c | 6 +-
drivers/target/target_core_pr.c | 110 +++++++++++++++-------------------
include/target/target_core_base.h | 4 +-
3 files changed, 53 insertions(+), 67 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 2768221..67d6d5f 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -817,7 +817,6 @@ static ssize_t target_core_dev_pr_show_attr_res_pr_holder_tg_port(
struct se_device *dev, char *page)
{
struct se_node_acl *se_nacl;
- struct se_lun *lun;
struct se_portal_group *se_tpg;
struct t10_pr_registration *pr_reg;
const struct target_core_fabric_ops *tfo;
@@ -832,7 +831,6 @@ static ssize_t target_core_dev_pr_show_attr_res_pr_holder_tg_port(
se_nacl = pr_reg->pr_reg_nacl;
se_tpg = se_nacl->se_tpg;
- lun = pr_reg->pr_reg_tg_pt_lun;
tfo = se_tpg->se_tpg_tfo;
len += sprintf(page+len, "SPC-3 Reservation: %s"
@@ -840,9 +838,9 @@ static ssize_t target_core_dev_pr_show_attr_res_pr_holder_tg_port(
tfo->tpg_get_wwn(se_tpg));
len += sprintf(page+len, "SPC-3 Reservation: Relative Port"
" Identifier Tag: %hu %s Portal Group Tag: %hu"
- " %s Logical Unit: %u\n", lun->lun_sep->sep_rtpi,
+ " %s Logical Unit: %u\n", pr_reg->tg_pt_sep_rtpi,
tfo->get_fabric_name(), tfo->tpg_get_tag(se_tpg),
- tfo->get_fabric_name(), lun->unpacked_lun);
+ tfo->get_fabric_name(), pr_reg->pr_aptpl_target_lun);
out_unlock:
spin_unlock(&dev->dev_reservation_lock);
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index d29b39c..94a136f 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -44,11 +44,10 @@
* Used for Specify Initiator Ports Capable Bit (SPEC_I_PT)
*/
struct pr_transport_id_holder {
- int dest_local_nexus;
struct t10_pr_registration *dest_pr_reg;
struct se_portal_group *dest_tpg;
struct se_node_acl *dest_node_acl;
- struct se_dev_entry __rcu *dest_se_deve;
+ struct se_dev_entry *dest_se_deve;
struct list_head dest_list;
};
@@ -625,7 +624,9 @@ static u32 core_scsi3_pr_generation(struct se_device *dev)
static struct t10_pr_registration *__core_scsi3_do_alloc_registration(
struct se_device *dev,
struct se_node_acl *nacl,
+ struct se_lun *lun,
struct se_dev_entry *deve,
+ u32 mapped_lun,
unsigned char *isid,
u64 sa_res_key,
int all_tg_pt,
@@ -647,12 +648,12 @@ static struct t10_pr_registration *__core_scsi3_do_alloc_registration(
atomic_set(&pr_reg->pr_res_holders, 0);
pr_reg->pr_reg_nacl = nacl;
pr_reg->pr_reg_deve = deve;
- pr_reg->pr_res_mapped_lun = deve->mapped_lun;
- pr_reg->pr_aptpl_target_lun = deve->se_lun->unpacked_lun;
+ pr_reg->pr_res_mapped_lun = mapped_lun;
+ pr_reg->pr_aptpl_target_lun = lun->unpacked_lun;
+ pr_reg->tg_pt_sep_rtpi = lun->lun_sep->sep_rtpi;
pr_reg->pr_res_key = sa_res_key;
pr_reg->pr_reg_all_tg_pt = all_tg_pt;
pr_reg->pr_reg_aptpl = aptpl;
- pr_reg->pr_reg_tg_pt_lun = deve->se_lun;
/*
* If an ISID value for this SCSI Initiator Port exists,
* save it to the registration now.
@@ -676,7 +677,9 @@ static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *);
static struct t10_pr_registration *__core_scsi3_alloc_registration(
struct se_device *dev,
struct se_node_acl *nacl,
+ struct se_lun *lun,
struct se_dev_entry *deve,
+ u32 mapped_lun,
unsigned char *isid,
u64 sa_res_key,
int all_tg_pt,
@@ -692,8 +695,9 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
* Create a registration for the I_T Nexus upon which the
* PROUT REGISTER was received.
*/
- pr_reg = __core_scsi3_do_alloc_registration(dev, nacl, deve, isid,
- sa_res_key, all_tg_pt, aptpl);
+ pr_reg = __core_scsi3_do_alloc_registration(dev, nacl, lun, deve, mapped_lun,
+ isid, sa_res_key, all_tg_pt,
+ aptpl);
if (!pr_reg)
return NULL;
/*
@@ -765,8 +769,9 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
* __core_scsi3_add_registration()
*/
pr_reg_atp = __core_scsi3_do_alloc_registration(dev,
- nacl_tmp, deve_tmp, NULL,
- sa_res_key, all_tg_pt, aptpl);
+ nacl_tmp, deve_tmp->se_lun,
+ deve_tmp, deve_tmp->mapped_lun,
+ NULL, sa_res_key, all_tg_pt, aptpl);
if (!pr_reg_atp) {
atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
core_scsi3_lunacl_undepend_item(deve_tmp);
@@ -835,7 +840,6 @@ int core_scsi3_alloc_aptpl_registration(
pr_reg->pr_res_key = sa_res_key;
pr_reg->pr_reg_all_tg_pt = all_tg_pt;
pr_reg->pr_reg_aptpl = 1;
- pr_reg->pr_reg_tg_pt_lun = NULL;
pr_reg->pr_res_scope = 0; /* Always LUN_SCOPE */
pr_reg->pr_res_type = type;
/*
@@ -935,7 +939,7 @@ static int __core_scsi3_check_aptpl_registration(
(pr_reg->pr_aptpl_target_lun == target_lun)) {
pr_reg->pr_reg_nacl = nacl;
- pr_reg->pr_reg_tg_pt_lun = lun;
+ pr_reg->tg_pt_sep_rtpi = lun->lun_sep->sep_rtpi;
list_del(&pr_reg->pr_reg_aptpl_list);
spin_unlock(&pr_tmpl->aptpl_reg_lock);
@@ -1090,7 +1094,9 @@ static void __core_scsi3_add_registration(
static int core_scsi3_alloc_registration(
struct se_device *dev,
struct se_node_acl *nacl,
+ struct se_lun *lun,
struct se_dev_entry *deve,
+ u32 mapped_lun,
unsigned char *isid,
u64 sa_res_key,
int all_tg_pt,
@@ -1100,8 +1106,9 @@ static int core_scsi3_alloc_registration(
{
struct t10_pr_registration *pr_reg;
- pr_reg = __core_scsi3_alloc_registration(dev, nacl, deve, isid,
- sa_res_key, all_tg_pt, aptpl);
+ pr_reg = __core_scsi3_alloc_registration(dev, nacl, lun, deve, mapped_lun,
+ isid, sa_res_key, all_tg_pt,
+ aptpl);
if (!pr_reg)
return -EPERM;
@@ -1456,7 +1463,6 @@ static sense_reason_t
core_scsi3_decode_spec_i_port(
struct se_cmd *cmd,
struct se_portal_group *tpg,
- struct se_dev_entry *local_se_deve,
unsigned char *l_isid,
u64 sa_res_key,
int all_tg_pt,
@@ -1467,7 +1473,7 @@ core_scsi3_decode_spec_i_port(
struct se_portal_group *dest_tpg = NULL, *tmp_tpg;
struct se_session *se_sess = cmd->se_sess;
struct se_node_acl *dest_node_acl = NULL;
- struct se_dev_entry __rcu *dest_se_deve = NULL;
+ struct se_dev_entry *dest_se_deve = NULL;
struct t10_pr_registration *dest_pr_reg, *local_pr_reg, *pr_reg_e;
struct t10_pr_registration *pr_reg_tmp, *pr_reg_tmp_safe;
LIST_HEAD(tid_dest_list);
@@ -1477,14 +1483,12 @@ core_scsi3_decode_spec_i_port(
char *iport_ptr = NULL, i_buf[PR_REG_ISID_ID_LEN];
sense_reason_t ret;
u32 tpdl, tid_len = 0;
- int dest_local_nexus;
u32 dest_rtpi = 0;
/*
* Allocate a struct pr_transport_id_holder and setup the
- * local_node_acl and local_se_deve pointers and add to
- * struct list_head tid_dest_list for add registration
- * processing in the loop of tid_dest_list below.
+ * local_node_acl pointer and add to struct list_head tid_dest_list
+ * for add registration processing in the loop of tid_dest_list below.
*/
tidh_new = kzalloc(sizeof(struct pr_transport_id_holder), GFP_KERNEL);
if (!tidh_new) {
@@ -1496,20 +1500,20 @@ core_scsi3_decode_spec_i_port(
tidh_new->dest_node_acl = se_sess->se_node_acl;
local_pr_reg = __core_scsi3_alloc_registration(cmd->se_dev,
- se_sess->se_node_acl, local_se_deve, l_isid,
+ se_sess->se_node_acl, cmd->se_lun,
+ NULL, cmd->orig_fe_lun, l_isid,
sa_res_key, all_tg_pt, aptpl);
if (!local_pr_reg) {
kfree(tidh_new);
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
- rcu_assign_pointer(tidh_new->dest_se_deve, local_se_deve);
tidh_new->dest_pr_reg = local_pr_reg;
/*
* The local I_T nexus does not hold any configfs dependances,
- * so we set tid_h->dest_local_nexus=1 to prevent the
+ * so we set tidh_new->dest_se_deve to NULL to prevent the
* configfs_undepend_item() calls in the tid_dest_list loops below.
*/
- tidh_new->dest_local_nexus = 1;
+ tidh_new->dest_se_deve = NULL;
list_add_tail(&tidh_new->dest_list, &tid_dest_list);
if (cmd->data_length < 28) {
@@ -1727,8 +1731,9 @@ core_scsi3_decode_spec_i_port(
* 2nd loop which will never fail.
*/
dest_pr_reg = __core_scsi3_alloc_registration(cmd->se_dev,
- dest_node_acl, dest_se_deve, iport_ptr,
- sa_res_key, all_tg_pt, aptpl);
+ dest_node_acl, dest_se_deve->se_lun,
+ dest_se_deve, dest_se_deve->mapped_lun,
+ iport_ptr, sa_res_key, all_tg_pt, aptpl);
if (!dest_pr_reg) {
core_scsi3_lunacl_undepend_item(dest_se_deve);
core_scsi3_nodeacl_undepend_item(dest_node_acl);
@@ -1766,7 +1771,6 @@ core_scsi3_decode_spec_i_port(
dest_node_acl = tidh->dest_node_acl;
dest_se_deve = tidh->dest_se_deve;
dest_pr_reg = tidh->dest_pr_reg;
- dest_local_nexus = tidh->dest_local_nexus;
list_del(&tidh->dest_list);
kfree(tidh);
@@ -1780,9 +1784,10 @@ core_scsi3_decode_spec_i_port(
pr_debug("SPC-3 PR [%s] SPEC_I_PT: Successfully"
" registered Transport ID for Node: %s%s Mapped LUN:"
" %u\n", dest_tpg->se_tpg_tfo->get_fabric_name(),
- dest_node_acl->initiatorname, i_buf, dest_se_deve->mapped_lun);
+ dest_node_acl->initiatorname, i_buf, (dest_se_deve) ?
+ dest_se_deve->mapped_lun : 0);
- if (dest_local_nexus)
+ if (!dest_se_deve)
continue;
core_scsi3_lunacl_undepend_item(dest_se_deve);
@@ -1803,7 +1808,6 @@ out:
dest_node_acl = tidh->dest_node_acl;
dest_se_deve = tidh->dest_se_deve;
dest_pr_reg = tidh->dest_pr_reg;
- dest_local_nexus = tidh->dest_local_nexus;
list_del(&tidh->dest_list);
kfree(tidh);
@@ -1821,7 +1825,7 @@ out:
kmem_cache_free(t10_pr_reg_cache, dest_pr_reg);
- if (dest_local_nexus)
+ if (!dest_se_deve)
continue;
core_scsi3_lunacl_undepend_item(dest_se_deve);
@@ -1836,7 +1840,6 @@ static int core_scsi3_update_aptpl_buf(
unsigned char *buf,
u32 pr_aptpl_buf_len)
{
- struct se_lun *lun;
struct se_portal_group *tpg;
struct t10_pr_registration *pr_reg;
unsigned char tmp[512], isid_buf[32];
@@ -1855,7 +1858,6 @@ static int core_scsi3_update_aptpl_buf(
tmp[0] = '\0';
isid_buf[0] = '\0';
tpg = pr_reg->pr_reg_nacl->se_tpg;
- lun = pr_reg->pr_reg_tg_pt_lun;
/*
* Write out any ISID value to APTPL metadata that was included
* in the original registration.
@@ -1907,7 +1909,8 @@ static int core_scsi3_update_aptpl_buf(
" %d\n", tpg->se_tpg_tfo->get_fabric_name(),
tpg->se_tpg_tfo->tpg_get_wwn(tpg),
tpg->se_tpg_tfo->tpg_get_tag(tpg),
- lun->lun_sep->sep_rtpi, lun->unpacked_lun, reg_count);
+ pr_reg->tg_pt_sep_rtpi, pr_reg->pr_aptpl_target_lun,
+ reg_count);
if ((len + strlen(tmp) >= pr_aptpl_buf_len)) {
pr_err("Unable to update renaming APTPL metadata,"
@@ -2017,9 +2020,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
bool aptpl, bool all_tg_pt, bool spec_i_pt, enum register_type register_type)
{
struct se_session *se_sess = cmd->se_sess;
- struct se_node_acl *nacl = se_sess->se_node_acl;
struct se_device *dev = cmd->se_dev;
- struct se_dev_entry *se_deve;
struct se_lun *se_lun = cmd->se_lun;
struct se_portal_group *se_tpg;
struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_reg_tmp;
@@ -2034,14 +2035,6 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
}
se_tpg = se_sess->se_tpg;
- rcu_read_lock();
- se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
- if (!se_deve) {
- pr_err("Unable to locate se_deve for PRO-REGISTER\n");
- rcu_read_unlock();
- return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
- }
-
if (se_tpg->se_tpg_tfo->sess_get_initiator_sid) {
memset(&isid_buf[0], 0, PR_REG_ISID_LEN);
se_tpg->se_tpg_tfo->sess_get_initiator_sid(se_sess, &isid_buf[0],
@@ -2056,16 +2049,14 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
if (res_key) {
pr_warn("SPC-3 PR: Reservation Key non-zero"
" for SA REGISTER, returning CONFLICT\n");
- rcu_read_unlock();
return TCM_RESERVATION_CONFLICT;
}
/*
* Do nothing but return GOOD status.
*/
- if (!sa_res_key) {
- rcu_read_unlock();
+ if (!sa_res_key)
return 0;
- }
+
if (!spec_i_pt) {
/*
* Perform the Service Action REGISTER on the Initiator
@@ -2073,12 +2064,12 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
* Logical Unit of the SCSI device server.
*/
if (core_scsi3_alloc_registration(cmd->se_dev,
- se_sess->se_node_acl, se_deve, isid_ptr,
+ se_sess->se_node_acl, cmd->se_lun,
+ NULL, cmd->orig_fe_lun, isid_ptr,
sa_res_key, all_tg_pt, aptpl,
register_type, 0)) {
pr_err("Unable to allocate"
" struct t10_pr_registration\n");
- rcu_read_unlock();
return TCM_INVALID_PARAMETER_LIST;
}
} else {
@@ -2090,17 +2081,13 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
* logic from of core_scsi3_alloc_registration() for
* each TransportID provided SCSI Initiator Port/Device
*/
- ret = core_scsi3_decode_spec_i_port(cmd, se_tpg, se_deve,
+ ret = core_scsi3_decode_spec_i_port(cmd, se_tpg,
isid_ptr, sa_res_key, all_tg_pt, aptpl);
- if (ret != 0) {
- rcu_read_unlock();
+ if (ret != 0)
return ret;
- }
}
- rcu_read_unlock();
return core_scsi3_update_and_write_aptpl(dev, aptpl);
}
- rcu_read_unlock();
/* ok, existing registration */
@@ -3447,9 +3434,10 @@ after_iport_check:
iport_ptr);
if (!dest_pr_reg) {
spin_unlock(&dev->dev_reservation_lock);
- if (core_scsi3_alloc_registration(cmd->se_dev,
- dest_node_acl, dest_se_deve, iport_ptr,
- sa_res_key, 0, aptpl, 2, 1)) {
+ if (core_scsi3_alloc_registration(cmd->se_dev, dest_node_acl,
+ dest_se_deve->se_lun, dest_se_deve,
+ dest_se_deve->mapped_lun, iport_ptr,
+ sa_res_key, 0, aptpl, 2, 1)) {
ret = TCM_INVALID_PARAMETER_LIST;
goto out;
}
@@ -4017,10 +4005,10 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
* IDENTIFIER field are not defined by this standard.
*/
if (!pr_reg->pr_reg_all_tg_pt) {
- struct se_port *port = pr_reg->pr_reg_tg_pt_lun->lun_sep;
+ u16 sep_rtpi = pr_reg->tg_pt_sep_rtpi;
- buf[off++] = ((port->sep_rtpi >> 8) & 0xff);
- buf[off++] = (port->sep_rtpi & 0xff);
+ buf[off++] = ((sep_rtpi >> 8) & 0xff);
+ buf[off++] = (sep_rtpi & 0xff);
} else
off += 2; /* Skip over RELATIVE TARGET PORT IDENTIFIER */
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index def5bc8..034f4a6 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -372,13 +372,14 @@ struct t10_pr_registration {
bool isid_present_at_reg;
u32 pr_res_mapped_lun;
u32 pr_aptpl_target_lun;
+ u16 tg_pt_sep_rtpi;
u32 pr_res_generation;
u64 pr_reg_bin_isid;
u64 pr_res_key;
atomic_t pr_res_holders;
struct se_node_acl *pr_reg_nacl;
+ /* Used by ALL_TG_PT=1 registration with deve->pr_ref taken */
struct se_dev_entry *pr_reg_deve;
- struct se_lun *pr_reg_tg_pt_lun;
struct list_head pr_reg_list;
struct list_head pr_reg_abort_list;
struct list_head pr_reg_aptpl_list;
@@ -498,7 +499,6 @@ struct se_cmd {
struct list_head se_delayed_node;
struct list_head se_qf_node;
struct se_device *se_dev;
- struct se_dev_entry *se_deve;
struct se_lun *se_lun;
/* Only used for internal passthrough and legacy TCM fabric modules */
struct se_session *se_sess;
--
1.9.1
From: Christoph Hellwig <[email protected]>
Clean up the mess of registered variables, and pass the isid mismatch
flag explicitly instead of overloading the registration type.
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_pr.c | 43 ++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 94a136f..36bcc60 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -312,34 +312,30 @@ out:
* This function is called by those initiator ports who are *NOT*
* the active PR reservation holder when a reservation is present.
*/
-static int core_scsi3_pr_seq_non_holder(
- struct se_cmd *cmd,
- u32 pr_reg_type)
+static int core_scsi3_pr_seq_non_holder(struct se_cmd *cmd, u32 pr_reg_type,
+ bool isid_mismatch)
{
unsigned char *cdb = cmd->t_task_cdb;
- struct se_dev_entry *se_deve;
struct se_session *se_sess = cmd->se_sess;
struct se_node_acl *nacl = se_sess->se_node_acl;
- int other_cdb = 0, ignore_reg;
+ int other_cdb = 0;
int registered_nexus = 0, ret = 1; /* Conflict by default */
int all_reg = 0, reg_only = 0; /* ALL_REG, REG_ONLY */
int we = 0; /* Write Exclusive */
int legacy = 0; /* Act like a legacy device and return
* RESERVATION CONFLICT on some CDBs */
- bool registered = false;
- rcu_read_lock();
- se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
- if (se_deve)
- registered = test_bit(1, &se_deve->pr_reg);
- rcu_read_unlock();
- /*
- * Determine if the registration should be ignored due to
- * non-matching ISIDs in target_scsi3_pr_reservation_check().
- */
- ignore_reg = (pr_reg_type & 0x80000000);
- if (ignore_reg)
- pr_reg_type &= ~0x80000000;
+ if (isid_mismatch) {
+ registered_nexus = 0;
+ } else {
+ struct se_dev_entry *se_deve;
+
+ rcu_read_lock();
+ se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
+ if (se_deve)
+ registered_nexus = test_bit(1, &se_deve->pr_reg);
+ rcu_read_unlock();
+ }
switch (pr_reg_type) {
case PR_TYPE_WRITE_EXCLUSIVE:
@@ -349,8 +345,6 @@ static int core_scsi3_pr_seq_non_holder(
* Some commands are only allowed for the persistent reservation
* holder.
*/
- if ((registered) && !(ignore_reg))
- registered_nexus = 1;
break;
case PR_TYPE_WRITE_EXCLUSIVE_REGONLY:
we = 1;
@@ -359,8 +353,6 @@ static int core_scsi3_pr_seq_non_holder(
* Some commands are only allowed for registered I_T Nexuses.
*/
reg_only = 1;
- if ((registered) && !(ignore_reg))
- registered_nexus = 1;
break;
case PR_TYPE_WRITE_EXCLUSIVE_ALLREG:
we = 1;
@@ -369,8 +361,6 @@ static int core_scsi3_pr_seq_non_holder(
* Each registered I_T Nexus is a reservation holder.
*/
all_reg = 1;
- if ((registered) && !(ignore_reg))
- registered_nexus = 1;
break;
default:
return -EINVAL;
@@ -576,6 +566,7 @@ target_scsi3_pr_reservation_check(struct se_cmd *cmd)
struct se_device *dev = cmd->se_dev;
struct se_session *sess = cmd->se_sess;
u32 pr_reg_type;
+ bool isid_mismatch = false;
if (!dev->dev_pr_res_holder)
return 0;
@@ -588,7 +579,7 @@ target_scsi3_pr_reservation_check(struct se_cmd *cmd)
if (dev->dev_pr_res_holder->isid_present_at_reg) {
if (dev->dev_pr_res_holder->pr_reg_bin_isid !=
sess->sess_bin_isid) {
- pr_reg_type |= 0x80000000;
+ isid_mismatch = true;
goto check_nonholder;
}
}
@@ -596,7 +587,7 @@ target_scsi3_pr_reservation_check(struct se_cmd *cmd)
return 0;
check_nonholder:
- if (core_scsi3_pr_seq_non_holder(cmd, pr_reg_type))
+ if (core_scsi3_pr_seq_non_holder(cmd, pr_reg_type, isid_mismatch))
return TCM_RESERVATION_CONFLICT;
return 0;
}
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch converts the fixed size se_portal_group->tpg_lun_list[]
to use modern RCU with hlist_head in order to support an arbitary
number of se_lun ports per target endpoint.
It includes dropping core_tpg_alloc_lun() from core_dev_add_lun(),
and calling it directly from target_fabric_make_lun() to allocate
a new se_lun.
Also add a new target_fabric_port_release() configfs item callback
to invoke kfree_rcu() to release memory during se_lun->lun_group
shutdown.
Also now that se_node_acl->lun_entry_hlist is using RCU, convert
existing tpg_lun_lock to struct mutex so core_tpg_add_node_to_devs()
can perform RCU updater logic without releasing ->tpg_lun_mutex.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Chris Boot <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/iscsi/iscsi_target_tpg.c | 2 -
drivers/target/sbp/sbp_target.c | 97 +++++++++-----------
drivers/target/sbp/sbp_target.h | 2 +-
drivers/target/target_core_device.c | 92 ++-----------------
drivers/target/target_core_fabric_configfs.c | 34 ++++---
drivers/target/target_core_internal.h | 6 +-
drivers/target/target_core_tpg.c | 132 ++++++---------------------
drivers/xen/xen-scsiback.c | 27 +++---
include/target/target_core_base.h | 6 +-
include/target/target_core_fabric.h | 1 -
10 files changed, 122 insertions(+), 277 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c
index 3af76e3..86f888e 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.c
+++ b/drivers/target/iscsi/iscsi_target_tpg.c
@@ -281,8 +281,6 @@ int iscsit_tpg_del_portal_group(
return -EPERM;
}
- core_tpg_clear_object_luns(&tpg->tpg_se_tpg);
-
if (tpg->param_list) {
iscsi_release_param_list(tpg->param_list);
tpg->param_list = NULL;
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index 5d7755e..47fb12f 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -108,13 +108,13 @@ static struct sbp_session *sbp_session_find_by_guid(
}
static struct sbp_login_descriptor *sbp_login_find_by_lun(
- struct sbp_session *session, struct se_lun *lun)
+ struct sbp_session *session, u32 unpacked_lun)
{
struct sbp_login_descriptor *login, *found = NULL;
spin_lock_bh(&session->lock);
list_for_each_entry(login, &session->login_list, link) {
- if (login->lun == lun)
+ if (login->login_lun == unpacked_lun)
found = login;
}
spin_unlock_bh(&session->lock);
@@ -124,7 +124,7 @@ static struct sbp_login_descriptor *sbp_login_find_by_lun(
static int sbp_login_count_all_by_lun(
struct sbp_tpg *tpg,
- struct se_lun *lun,
+ u32 unpacked_lun,
int exclusive)
{
struct se_session *se_sess;
@@ -138,7 +138,7 @@ static int sbp_login_count_all_by_lun(
spin_lock_bh(&sess->lock);
list_for_each_entry(login, &sess->login_list, link) {
- if (login->lun != lun)
+ if (login->login_lun != unpacked_lun)
continue;
if (!exclusive || login->exclusive)
@@ -174,23 +174,23 @@ static struct sbp_login_descriptor *sbp_login_find_by_id(
return found;
}
-static struct se_lun *sbp_get_lun_from_tpg(struct sbp_tpg *tpg, int lun)
+static u32 sbp_get_lun_from_tpg(struct sbp_tpg *tpg, u32 login_lun, int *err)
{
struct se_portal_group *se_tpg = &tpg->se_tpg;
struct se_lun *se_lun;
- if (lun >= TRANSPORT_MAX_LUNS_PER_TPG)
- return ERR_PTR(-EINVAL);
-
- spin_lock(&se_tpg->tpg_lun_lock);
- se_lun = se_tpg->tpg_lun_list[lun];
-
- if (se_lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
- se_lun = ERR_PTR(-ENODEV);
-
- spin_unlock(&se_tpg->tpg_lun_lock);
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(se_lun, &se_tpg->tpg_lun_hlist, link) {
+ if (se_lun->unpacked_lun == login_lun) {
+ rcu_read_unlock();
+ *err = 0;
+ return login_lun;
+ }
+ }
+ rcu_read_unlock();
- return se_lun;
+ *err = -ENODEV;
+ return login_lun;
}
static struct sbp_session *sbp_session_create(
@@ -294,17 +294,16 @@ static void sbp_management_request_login(
{
struct sbp_tport *tport = agent->tport;
struct sbp_tpg *tpg = tport->tpg;
- struct se_lun *se_lun;
- int ret;
- u64 guid;
struct sbp_session *sess;
struct sbp_login_descriptor *login;
struct sbp_login_response_block *response;
- int login_response_len;
+ u64 guid;
+ u32 unpacked_lun;
+ int login_response_len, ret;
- se_lun = sbp_get_lun_from_tpg(tpg,
- LOGIN_ORB_LUN(be32_to_cpu(req->orb.misc)));
- if (IS_ERR(se_lun)) {
+ unpacked_lun = sbp_get_lun_from_tpg(tpg,
+ LOGIN_ORB_LUN(be32_to_cpu(req->orb.misc)), &ret);
+ if (ret) {
pr_notice("login to unknown LUN: %d\n",
LOGIN_ORB_LUN(be32_to_cpu(req->orb.misc)));
@@ -325,11 +324,11 @@ static void sbp_management_request_login(
}
pr_notice("mgt_agent LOGIN to LUN %d from %016llx\n",
- se_lun->unpacked_lun, guid);
+ unpacked_lun, guid);
sess = sbp_session_find_by_guid(tpg, guid);
if (sess) {
- login = sbp_login_find_by_lun(sess, se_lun);
+ login = sbp_login_find_by_lun(sess, unpacked_lun);
if (login) {
pr_notice("initiator already logged-in\n");
@@ -357,7 +356,7 @@ static void sbp_management_request_login(
* reject with access_denied if any logins present
*/
if (LOGIN_ORB_EXCLUSIVE(be32_to_cpu(req->orb.misc)) &&
- sbp_login_count_all_by_lun(tpg, se_lun, 0)) {
+ sbp_login_count_all_by_lun(tpg, unpacked_lun, 0)) {
pr_warn("refusing exclusive login with other active logins\n");
req->status.status = cpu_to_be32(
@@ -370,7 +369,7 @@ static void sbp_management_request_login(
* check exclusive bit in any existing login descriptor
* reject with access_denied if any exclusive logins present
*/
- if (sbp_login_count_all_by_lun(tpg, se_lun, 1)) {
+ if (sbp_login_count_all_by_lun(tpg, unpacked_lun, 1)) {
pr_warn("refusing login while another exclusive login present\n");
req->status.status = cpu_to_be32(
@@ -383,7 +382,7 @@ static void sbp_management_request_login(
* check we haven't exceeded the number of allowed logins
* reject with resources_unavailable if we have
*/
- if (sbp_login_count_all_by_lun(tpg, se_lun, 0) >=
+ if (sbp_login_count_all_by_lun(tpg, unpacked_lun, 0) >=
tport->max_logins_per_lun) {
pr_warn("max number of logins reached\n");
@@ -439,7 +438,7 @@ static void sbp_management_request_login(
}
login->sess = sess;
- login->lun = se_lun;
+ login->login_lun = unpacked_lun;
login->status_fifo_addr = sbp2_pointer_to_addr(&req->orb.status_fifo);
login->exclusive = LOGIN_ORB_EXCLUSIVE(be32_to_cpu(req->orb.misc));
login->login_id = atomic_inc_return(&login_id);
@@ -601,7 +600,7 @@ static void sbp_management_request_logout(
}
pr_info("mgt_agent LOGOUT from LUN %d session %d\n",
- login->lun->unpacked_lun, login->login_id);
+ login->login_lun, login->login_id);
if (req->node_addr != login->sess->node_id) {
pr_warn("logout from different node ID\n");
@@ -1227,7 +1226,7 @@ static void sbp_handle_command(struct sbp_target_request *req)
goto err;
}
- unpacked_lun = req->login->lun->unpacked_lun;
+ unpacked_lun = req->login->login_lun;
sbp_calc_data_length_direction(req, &data_length, &data_dir);
pr_debug("sbp_handle_command ORB:0x%llx unpacked_lun:%d data_len:%d data_dir:%d\n",
@@ -1826,25 +1825,21 @@ static int sbp_check_stop_free(struct se_cmd *se_cmd)
static int sbp_count_se_tpg_luns(struct se_portal_group *tpg)
{
- int i, count = 0;
-
- spin_lock(&tpg->tpg_lun_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- struct se_lun *se_lun = tpg->tpg_lun_list[i];
-
- if (se_lun->lun_status == TRANSPORT_LUN_STATUS_FREE)
- continue;
+ struct se_lun *lun;
+ int count = 0;
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(lun, &tpg->tpg_lun_hlist, link)
count++;
- }
- spin_unlock(&tpg->tpg_lun_lock);
+ rcu_read_unlock();
return count;
}
static int sbp_update_unit_directory(struct sbp_tport *tport)
{
- int num_luns, num_entries, idx = 0, mgt_agt_addr, ret, i;
+ struct se_lun *lun;
+ int num_luns, num_entries, idx = 0, mgt_agt_addr, ret;
u32 *data;
if (tport->unit_directory.data) {
@@ -1906,28 +1901,20 @@ static int sbp_update_unit_directory(struct sbp_tport *tport)
/* unit unique ID (leaf is just after LUNs) */
data[idx++] = 0x8d000000 | (num_luns + 1);
- spin_lock(&tport->tpg->se_tpg.tpg_lun_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- struct se_lun *se_lun = tport->tpg->se_tpg.tpg_lun_list[i];
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(lun, &tport->tpg->se_tpg.tpg_lun_hlist, link) {
struct se_device *dev;
int type;
- if (se_lun->lun_status == TRANSPORT_LUN_STATUS_FREE)
- continue;
-
- spin_unlock(&tport->tpg->se_tpg.tpg_lun_lock);
-
- dev = se_lun->lun_se_dev;
+ dev = lun->lun_se_dev;
type = dev->transport->get_device_type(dev);
/* logical_unit_number */
data[idx++] = 0x14000000 |
((type << 16) & 0x1f0000) |
- (se_lun->unpacked_lun & 0xffff);
-
- spin_lock(&tport->tpg->se_tpg.tpg_lun_lock);
+ (lun->unpacked_lun & 0xffff);
}
- spin_unlock(&tport->tpg->se_tpg.tpg_lun_lock);
+ rcu_read_unlock();
/* unit unique ID leaf */
data[idx++] = 2 << 16;
diff --git a/drivers/target/sbp/sbp_target.h b/drivers/target/sbp/sbp_target.h
index e1b0b84..73bcb12 100644
--- a/drivers/target/sbp/sbp_target.h
+++ b/drivers/target/sbp/sbp_target.h
@@ -125,7 +125,7 @@ struct sbp_login_descriptor {
struct sbp_session *sess;
struct list_head link;
- struct se_lun *lun;
+ u32 login_lun;
u64 status_fifo_addr;
int exclusive;
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 6432d26..5b74c33 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1161,22 +1161,17 @@ int se_dev_set_block_size(struct se_device *dev, u32 block_size)
}
EXPORT_SYMBOL(se_dev_set_block_size);
-struct se_lun *core_dev_add_lun(
+int core_dev_add_lun(
struct se_portal_group *tpg,
struct se_device *dev,
- u32 unpacked_lun)
+ struct se_lun *lun)
{
- struct se_lun *lun;
int rc;
- lun = core_tpg_alloc_lun(tpg, unpacked_lun);
- if (IS_ERR(lun))
- return lun;
-
rc = core_tpg_add_lun(tpg, lun,
TRANSPORT_LUNFLAGS_READ_WRITE, dev);
if (rc < 0)
- return ERR_PTR(rc);
+ return rc;
pr_debug("%s_TPG[%u]_LUN[%u] - Activated %s Logical Unit from"
" CORE HBA: %u\n", tpg->se_tpg_tfo->get_fabric_name(),
@@ -1201,7 +1196,7 @@ struct se_lun *core_dev_add_lun(
spin_unlock_irq(&tpg->acl_node_lock);
}
- return lun;
+ return 0;
}
/* core_dev_del_lun():
@@ -1220,68 +1215,6 @@ void core_dev_del_lun(
core_tpg_remove_lun(tpg, lun);
}
-struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun)
-{
- struct se_lun *lun;
-
- spin_lock(&tpg->tpg_lun_lock);
- if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
- pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS"
- "_PER_TPG-1: %u for Target Portal Group: %hu\n",
- tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
- TRANSPORT_MAX_LUNS_PER_TPG-1,
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock(&tpg->tpg_lun_lock);
- return NULL;
- }
- lun = tpg->tpg_lun_list[unpacked_lun];
-
- if (lun->lun_status != TRANSPORT_LUN_STATUS_FREE) {
- pr_err("%s Logical Unit Number: %u is not free on"
- " Target Portal Group: %hu, ignoring request.\n",
- tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock(&tpg->tpg_lun_lock);
- return NULL;
- }
- spin_unlock(&tpg->tpg_lun_lock);
-
- return lun;
-}
-
-/* core_dev_get_lun():
- *
- *
- */
-static struct se_lun *core_dev_get_lun(struct se_portal_group *tpg, u32 unpacked_lun)
-{
- struct se_lun *lun;
-
- spin_lock(&tpg->tpg_lun_lock);
- if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
- pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER"
- "_TPG-1: %u for Target Portal Group: %hu\n",
- tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
- TRANSPORT_MAX_LUNS_PER_TPG-1,
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock(&tpg->tpg_lun_lock);
- return NULL;
- }
- lun = tpg->tpg_lun_list[unpacked_lun];
-
- if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE) {
- pr_err("%s Logical Unit Number: %u is not active on"
- " Target Portal Group: %hu, ignoring request.\n",
- tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock(&tpg->tpg_lun_lock);
- return NULL;
- }
- spin_unlock(&tpg->tpg_lun_lock);
-
- return lun;
-}
-
struct se_lun_acl *core_dev_init_initiator_node_lun_acl(
struct se_portal_group *tpg,
struct se_node_acl *nacl,
@@ -1315,22 +1248,11 @@ struct se_lun_acl *core_dev_init_initiator_node_lun_acl(
int core_dev_add_initiator_node_lun_acl(
struct se_portal_group *tpg,
struct se_lun_acl *lacl,
- u32 unpacked_lun,
+ struct se_lun *lun,
u32 lun_access)
{
- struct se_lun *lun;
- struct se_node_acl *nacl;
+ struct se_node_acl *nacl = lacl->se_lun_nacl;
- lun = core_dev_get_lun(tpg, unpacked_lun);
- if (!lun) {
- pr_err("%s Logical Unit Number: %u is not active on"
- " Target Portal Group: %hu, ignoring request.\n",
- tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- return -EINVAL;
- }
-
- nacl = lacl->se_lun_nacl;
if (!nacl)
return -EINVAL;
@@ -1351,7 +1273,7 @@ int core_dev_add_initiator_node_lun_acl(
pr_debug("%s_TPG[%hu]_LUN[%u->%u] - Added %s ACL for "
" InitiatorNode: %s\n", tpg->se_tpg_tfo->get_fabric_name(),
- tpg->se_tpg_tfo->tpg_get_tag(tpg), unpacked_lun, lacl->mapped_lun,
+ tpg->se_tpg_tfo->tpg_get_tag(tpg), lun->unpacked_lun, lacl->mapped_lun,
(lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) ? "RW" : "RO",
lacl->initiatorname);
/*
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 0939a54..9be8030 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -81,7 +81,7 @@ static int target_fabric_mappedlun_link(
struct se_lun_acl, se_lun_group);
struct se_portal_group *se_tpg;
struct config_item *nacl_ci, *tpg_ci, *tpg_ci_s, *wwn_ci, *wwn_ci_s;
- int ret = 0, lun_access;
+ int lun_access;
if (lun->lun_link_magic != SE_LUN_LINK_MAGIC) {
pr_err("Bad lun->lun_link_magic, not a valid lun_ci pointer:"
@@ -137,12 +137,9 @@ static int target_fabric_mappedlun_link(
* Determine the actual mapped LUN value user wants..
*
* This value is what the SCSI Initiator actually sees the
- * iscsi/$IQN/$TPGT/lun/lun_* as on their SCSI Initiator Ports.
+ * $FABRIC/$WWPN/$TPGT/lun/lun_* as on their SCSI Initiator Ports.
*/
- ret = core_dev_add_initiator_node_lun_acl(se_tpg, lacl,
- lun->unpacked_lun, lun_access);
-
- return (ret < 0) ? -EINVAL : 0;
+ return core_dev_add_initiator_node_lun_acl(se_tpg, lacl, lun, lun_access);
}
static int target_fabric_mappedlun_unlink(
@@ -761,7 +758,6 @@ static int target_fabric_port_link(
struct config_item *tpg_ci;
struct se_lun *lun = container_of(to_config_group(lun_ci),
struct se_lun, lun_group);
- struct se_lun *lun_p;
struct se_portal_group *se_tpg;
struct se_device *dev =
container_of(to_config_group(se_dev_ci), struct se_device, dev_group);
@@ -789,10 +785,9 @@ static int target_fabric_port_link(
return -EEXIST;
}
- 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);
+ ret = core_dev_add_lun(se_tpg, dev, lun);
+ if (ret) {
+ pr_err("core_dev_add_lun() failed: %d\n", ret);
goto out;
}
@@ -832,9 +827,18 @@ static int target_fabric_port_unlink(
return 0;
}
+static void target_fabric_port_release(struct config_item *item)
+{
+ struct se_lun *lun = container_of(to_config_group(item),
+ struct se_lun, lun_group);
+
+ kfree_rcu(lun, rcu_head);
+}
+
static struct configfs_item_operations target_fabric_port_item_ops = {
.show_attribute = target_fabric_port_attr_show,
.store_attribute = target_fabric_port_attr_store,
+ .release = target_fabric_port_release,
.allow_link = target_fabric_port_link,
.drop_link = target_fabric_port_unlink,
};
@@ -893,15 +897,16 @@ static struct config_group *target_fabric_make_lun(
if (unpacked_lun > UINT_MAX)
return ERR_PTR(-EINVAL);
- lun = core_get_lun_from_tpg(se_tpg, unpacked_lun);
- if (!lun)
- return ERR_PTR(-EINVAL);
+ lun = core_tpg_alloc_lun(se_tpg, unpacked_lun);
+ if (IS_ERR(lun))
+ return ERR_CAST(lun);
lun_cg = &lun->lun_group;
lun_cg->default_groups = kmalloc(sizeof(struct config_group *) * 2,
GFP_KERNEL);
if (!lun_cg->default_groups) {
pr_err("Unable to allocate lun_cg->default_groups\n");
+ kfree(lun);
return ERR_PTR(-ENOMEM);
}
@@ -918,6 +923,7 @@ static struct config_group *target_fabric_make_lun(
if (!port_stat_grp->default_groups) {
pr_err("Unable to allocate port_stat_grp->default_groups\n");
kfree(lun_cg->default_groups);
+ kfree(lun);
return ERR_PTR(-ENOMEM);
}
target_stat_setup_port_default_groups(lun);
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index a04a6e3..2c160ce 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -23,13 +23,13 @@ int core_dev_export(struct se_device *, struct se_portal_group *,
struct se_lun *);
void core_dev_unexport(struct se_device *, struct se_portal_group *,
struct se_lun *);
-struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_device *, u32);
+int core_dev_add_lun(struct se_portal_group *, struct se_device *,
+ struct se_lun *lun);
void core_dev_del_lun(struct se_portal_group *, struct se_lun *);
-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 *,
struct se_node_acl *, u32, int *);
int core_dev_add_initiator_node_lun_acl(struct se_portal_group *,
- struct se_lun_acl *, u32, u32);
+ struct se_lun_acl *, struct se_lun *lun, u32);
int core_dev_del_initiator_node_lun_acl(struct se_portal_group *,
struct se_lun *, struct se_lun_acl *);
void core_dev_free_initiator_node_lun_acl(struct se_portal_group *,
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 0519923..13d34e2 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -91,19 +91,15 @@ void core_tpg_add_node_to_devs(
struct se_node_acl *acl,
struct se_portal_group *tpg)
{
- int i = 0;
u32 lun_access = 0;
struct se_lun *lun;
struct se_device *dev;
- spin_lock(&tpg->tpg_lun_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- lun = tpg->tpg_lun_list[i];
+ mutex_lock(&tpg->tpg_lun_mutex);
+ hlist_for_each_entry_rcu(lun, &tpg->tpg_lun_hlist, link) {
if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
continue;
- spin_unlock(&tpg->tpg_lun_lock);
-
dev = lun->lun_se_dev;
/*
* By default in LIO-Target $FABRIC_MOD,
@@ -130,7 +126,7 @@ void core_tpg_add_node_to_devs(
"READ-WRITE" : "READ-ONLY");
core_enable_device_list_for_node(lun, NULL, lun->unpacked_lun,
- lun_access, acl, tpg);
+ lun_access, acl, tpg);
/*
* Check to see if there are any existing persistent reservation
* APTPL pre-registrations that need to be enabled for this dynamic
@@ -138,9 +134,8 @@ void core_tpg_add_node_to_devs(
*/
core_scsi3_check_aptpl_registration(dev, tpg, lun, acl,
lun->unpacked_lun);
- spin_lock(&tpg->tpg_lun_lock);
}
- spin_unlock(&tpg->tpg_lun_lock);
+ mutex_unlock(&tpg->tpg_lun_mutex);
}
/* core_set_queue_depth_for_node():
@@ -161,34 +156,6 @@ static int core_set_queue_depth_for_node(
return 0;
}
-void array_free(void *array, int n)
-{
- void **a = array;
- int i;
-
- for (i = 0; i < n; i++)
- kfree(a[i]);
- kfree(a);
-}
-
-static void *array_zalloc(int n, size_t size, gfp_t flags)
-{
- void **a;
- int i;
-
- a = kzalloc(n * sizeof(void*), flags);
- if (!a)
- return NULL;
- for (i = 0; i < n; i++) {
- a[i] = kzalloc(size, flags);
- if (!a[i]) {
- array_free(a, n);
- return NULL;
- }
- }
- return a;
-}
-
static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg,
const unsigned char *initiatorname)
{
@@ -284,27 +251,6 @@ void core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *nacl)
cpu_relax();
}
-void core_tpg_clear_object_luns(struct se_portal_group *tpg)
-{
- int i;
- struct se_lun *lun;
-
- spin_lock(&tpg->tpg_lun_lock);
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- lun = tpg->tpg_lun_list[i];
-
- if ((lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE) ||
- (lun->lun_se_dev == NULL))
- continue;
-
- spin_unlock(&tpg->tpg_lun_lock);
- core_dev_del_lun(tpg, lun);
- spin_lock(&tpg->tpg_lun_lock);
- }
- spin_unlock(&tpg->tpg_lun_lock);
-}
-EXPORT_SYMBOL(core_tpg_clear_object_luns);
-
struct se_node_acl *core_tpg_add_initiator_node_acl(
struct se_portal_group *tpg,
const char *initiatorname)
@@ -567,30 +513,7 @@ int core_tpg_register(
struct se_portal_group *se_tpg,
int proto_id)
{
- struct se_lun *lun;
- u32 i;
-
- se_tpg->tpg_lun_list = array_zalloc(TRANSPORT_MAX_LUNS_PER_TPG,
- sizeof(struct se_lun), GFP_KERNEL);
- if (!se_tpg->tpg_lun_list) {
- pr_err("Unable to allocate struct se_portal_group->"
- "tpg_lun_list\n");
- return -ENOMEM;
- }
-
- for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
- lun = se_tpg->tpg_lun_list[i];
- lun->unpacked_lun = i;
- lun->lun_link_magic = SE_LUN_LINK_MAGIC;
- lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
- atomic_set(&lun->lun_acl_count, 0);
- init_completion(&lun->lun_shutdown_comp);
- INIT_LIST_HEAD(&lun->lun_acl_list);
- spin_lock_init(&lun->lun_acl_lock);
- spin_lock_init(&lun->lun_sep_lock);
- init_completion(&lun->lun_ref_comp);
- }
-
+ INIT_HLIST_HEAD(&se_tpg->tpg_lun_hlist);
se_tpg->proto_id = proto_id;
se_tpg->se_tpg_tfo = tfo;
se_tpg->se_tpg_wwn = se_wwn;
@@ -600,14 +523,11 @@ int core_tpg_register(
INIT_LIST_HEAD(&se_tpg->tpg_sess_list);
spin_lock_init(&se_tpg->acl_node_lock);
spin_lock_init(&se_tpg->session_lock);
- spin_lock_init(&se_tpg->tpg_lun_lock);
+ mutex_init(&se_tpg->tpg_lun_mutex);
if (se_tpg->proto_id >= 0) {
- if (core_tpg_setup_virtual_lun0(se_tpg) < 0) {
- array_free(se_tpg->tpg_lun_list,
- TRANSPORT_MAX_LUNS_PER_TPG);
+ if (core_tpg_setup_virtual_lun0(se_tpg) < 0)
return -ENOMEM;
- }
}
spin_lock_bh(&tpg_lock);
@@ -662,7 +582,6 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
if (se_tpg->proto_id >= 0)
core_tpg_remove_lun(se_tpg, &se_tpg->tpg_virt_lun0);
- array_free(se_tpg->tpg_lun_list, TRANSPORT_MAX_LUNS_PER_TPG);
return 0;
}
EXPORT_SYMBOL(core_tpg_deregister);
@@ -682,17 +601,20 @@ struct se_lun *core_tpg_alloc_lun(
return ERR_PTR(-EOVERFLOW);
}
- spin_lock(&tpg->tpg_lun_lock);
- lun = tpg->tpg_lun_list[unpacked_lun];
- if (lun->lun_status == TRANSPORT_LUN_STATUS_ACTIVE) {
- pr_err("TPG Logical Unit Number: %u is already active"
- " on %s Target Portal Group: %u, ignoring request.\n",
- unpacked_lun, tpg->se_tpg_tfo->get_fabric_name(),
- tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock(&tpg->tpg_lun_lock);
- return ERR_PTR(-EINVAL);
+ lun = kzalloc(sizeof(*lun), GFP_KERNEL);
+ if (!lun) {
+ pr_err("Unable to allocate se_lun memory\n");
+ return ERR_PTR(-ENOMEM);
}
- spin_unlock(&tpg->tpg_lun_lock);
+ lun->unpacked_lun = unpacked_lun;
+ lun->lun_link_magic = SE_LUN_LINK_MAGIC;
+ lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
+ atomic_set(&lun->lun_acl_count, 0);
+ init_completion(&lun->lun_shutdown_comp);
+ INIT_LIST_HEAD(&lun->lun_acl_list);
+ spin_lock_init(&lun->lun_acl_lock);
+ spin_lock_init(&lun->lun_sep_lock);
+ init_completion(&lun->lun_ref_comp);
return lun;
}
@@ -716,10 +638,12 @@ int core_tpg_add_lun(
return ret;
}
- spin_lock(&tpg->tpg_lun_lock);
+ mutex_lock(&tpg->tpg_lun_mutex);
lun->lun_access = lun_access;
lun->lun_status = TRANSPORT_LUN_STATUS_ACTIVE;
- spin_unlock(&tpg->tpg_lun_lock);
+ if (!(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE))
+ hlist_add_head_rcu(&lun->link, &tpg->tpg_lun_hlist);
+ mutex_unlock(&tpg->tpg_lun_mutex);
return 0;
}
@@ -728,14 +652,18 @@ void core_tpg_remove_lun(
struct se_portal_group *tpg,
struct se_lun *lun)
{
+ struct se_device *dev = lun->lun_se_dev;
+
core_clear_lun_from_tpg(lun, tpg);
transport_clear_lun_ref(lun);
core_dev_unexport(lun->lun_se_dev, tpg, lun);
- spin_lock(&tpg->tpg_lun_lock);
+ mutex_lock(&tpg->tpg_lun_mutex);
lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
- spin_unlock(&tpg->tpg_lun_lock);
+ if (!(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE))
+ hlist_del_rcu(&lun->link);
+ mutex_unlock(&tpg->tpg_lun_mutex);
percpu_ref_exit(&lun->lun_ref);
}
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 8b7dd47..555033b 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -866,7 +866,8 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
struct list_head *head = &(info->v2p_entry_lists);
unsigned long flags;
char *lunp;
- unsigned int lun;
+ unsigned int unpacked_lun;
+ struct se_lun *se_lun;
struct scsiback_tpg *tpg_entry, *tpg = NULL;
char *error = "doesn't exist";
@@ -877,7 +878,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
}
*lunp = 0;
lunp++;
- if (kstrtouint(lunp, 10, &lun) || lun >= TRANSPORT_MAX_LUNS_PER_TPG) {
+ if (kstrtouint(lunp, 10, &unpacked_lun) || unpacked_lun >= TRANSPORT_MAX_LUNS_PER_TPG) {
pr_err("lun number not valid: %s\n", lunp);
return -EINVAL;
}
@@ -886,15 +887,17 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
list_for_each_entry(tpg_entry, &scsiback_list, tv_tpg_list) {
if (!strcmp(phy, tpg_entry->tport->tport_name) ||
!strcmp(phy, tpg_entry->param_alias)) {
- spin_lock(&tpg_entry->se_tpg.tpg_lun_lock);
- if (tpg_entry->se_tpg.tpg_lun_list[lun]->lun_status ==
- TRANSPORT_LUN_STATUS_ACTIVE) {
- if (!tpg_entry->tpg_nexus)
- error = "nexus undefined";
- else
- tpg = tpg_entry;
+ mutex_lock(&tpg_entry->se_tpg.tpg_lun_mutex);
+ hlist_for_each_entry(se_lun, &tpg_entry->se_tpg.tpg_lun_hlist, link) {
+ if (se_lun->unpacked_lun == unpacked_lun) {
+ if (!tpg_entry->tpg_nexus)
+ error = "nexus undefined";
+ else
+ tpg = tpg_entry;
+ break;
+ }
}
- spin_unlock(&tpg_entry->se_tpg.tpg_lun_lock);
+ mutex_unlock(&tpg_entry->se_tpg.tpg_lun_mutex);
break;
}
}
@@ -906,7 +909,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
mutex_unlock(&scsiback_mutex);
if (!tpg) {
- pr_err("%s:%d %s\n", phy, lun, error);
+ pr_err("%s:%d %s\n", phy, unpacked_lun, error);
return -ENODEV;
}
@@ -934,7 +937,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
kref_init(&new->kref);
new->v = *v;
new->tpg = tpg;
- new->lun = lun;
+ new->lun = unpacked_lun;
list_add_tail(&new->l, head);
out:
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 034f4a6..1049f6c 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -724,6 +724,8 @@ struct se_lun {
struct se_port_stat_grps port_stat_grps;
struct completion lun_ref_comp;
struct percpu_ref lun_ref;
+ struct hlist_node link;
+ struct rcu_head rcu_head;
};
struct se_dev_stat_grps {
@@ -876,11 +878,11 @@ struct se_portal_group {
spinlock_t acl_node_lock;
/* Spinlock for adding/removing sessions */
spinlock_t session_lock;
- spinlock_t tpg_lun_lock;
+ struct mutex tpg_lun_mutex;
struct list_head se_tpg_node;
/* linked list for initiator ACL list */
struct list_head acl_node_list;
- struct se_lun **tpg_lun_list;
+ struct hlist_head tpg_lun_hlist;
struct se_lun tpg_virt_lun0;
/* List of TCM sessions associated wth this TPG */
struct list_head tpg_sess_list;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 55654c9..b1e00a7 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -157,7 +157,6 @@ struct se_node_acl *core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
unsigned char *);
struct se_node_acl *core_tpg_check_initiator_node_acl(struct se_portal_group *,
unsigned char *);
-void core_tpg_clear_object_luns(struct se_portal_group *);
int core_tpg_set_initiator_node_queue_depth(struct se_portal_group *,
unsigned char *, u32, int);
int core_tpg_set_initiator_node_tag(struct se_portal_group *,
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch converts se_tpg->acl_node_lock to struct mutex, so that
->acl_node_acl walkers in core_clear_lun_from_tpg() can block when
calling core_disable_device_list_for_node().
It also updates core_dev_add_lun() to hold ->acl_node_mutex when
calling core_tpg_add_node_to_devs() to build ->lun_entry_hlist
for dynamically generated se_node_acl.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_device.c | 14 ++++------
drivers/target/target_core_pr.c | 8 +++---
drivers/target/target_core_tpg.c | 51 +++++++++++++++++-----------------
drivers/target/target_core_transport.c | 20 ++++++-------
drivers/target/tcm_fc/tfc_conf.c | 4 +--
include/target/target_core_base.h | 2 +-
6 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 5b74c33..99a60e6 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -433,9 +433,8 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
struct se_node_acl *nacl;
struct se_dev_entry *deve;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
list_for_each_entry(nacl, &tpg->acl_node_list, acl_list) {
- spin_unlock_irq(&tpg->acl_node_lock);
mutex_lock(&nacl->lun_entry_mutex);
hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) {
@@ -445,10 +444,8 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
core_disable_device_list_for_node(lun, deve, nacl, tpg);
}
mutex_unlock(&nacl->lun_entry_mutex);
-
- spin_lock_irq(&tpg->acl_node_lock);
}
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
}
static struct se_port *core_alloc_port(struct se_device *dev)
@@ -1183,17 +1180,16 @@ int core_dev_add_lun(
*/
if (tpg->se_tpg_tfo->tpg_check_demo_mode(tpg)) {
struct se_node_acl *acl;
- spin_lock_irq(&tpg->acl_node_lock);
+
+ mutex_lock(&tpg->acl_node_mutex);
list_for_each_entry(acl, &tpg->acl_node_list, acl_list) {
if (acl->dynamic_node_acl &&
(!tpg->se_tpg_tfo->tpg_check_demo_mode_login_only ||
!tpg->se_tpg_tfo->tpg_check_demo_mode_login_only(tpg))) {
- spin_unlock_irq(&tpg->acl_node_lock);
core_tpg_add_node_to_devs(acl, tpg);
- spin_lock_irq(&tpg->acl_node_lock);
}
}
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
}
return 0;
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 36bcc60..cee2b31 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1582,12 +1582,12 @@ core_scsi3_decode_spec_i_port(
* from the decoded fabric module specific TransportID
* at *i_str.
*/
- spin_lock_irq(&tmp_tpg->acl_node_lock);
+ mutex_lock(&tmp_tpg->acl_node_mutex);
dest_node_acl = __core_tpg_get_initiator_node_acl(
tmp_tpg, i_str);
if (dest_node_acl)
atomic_inc_mb(&dest_node_acl->acl_pr_ref_count);
- spin_unlock_irq(&tmp_tpg->acl_node_lock);
+ mutex_unlock(&tmp_tpg->acl_node_mutex);
if (!dest_node_acl) {
core_scsi3_tpg_undepend_item(tmp_tpg);
@@ -3298,12 +3298,12 @@ after_iport_check:
/*
* Locate the destination struct se_node_acl from the received Transport ID
*/
- spin_lock_irq(&dest_se_tpg->acl_node_lock);
+ mutex_lock(&dest_se_tpg->acl_node_mutex);
dest_node_acl = __core_tpg_get_initiator_node_acl(dest_se_tpg,
initiator_str);
if (dest_node_acl)
atomic_inc_mb(&dest_node_acl->acl_pr_ref_count);
- spin_unlock_irq(&dest_se_tpg->acl_node_lock);
+ mutex_unlock(&dest_se_tpg->acl_node_mutex);
if (!dest_node_acl) {
pr_err("Unable to locate %s dest_node_acl for"
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 13d34e2..229e827 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -49,7 +49,7 @@ static LIST_HEAD(tpg_list);
/* __core_tpg_get_initiator_node_acl():
*
- * spin_lock_bh(&tpg->acl_node_lock); must be held when calling
+ * mutex_lock(&tpg->acl_node_mutex); must be held when calling
*/
struct se_node_acl *__core_tpg_get_initiator_node_acl(
struct se_portal_group *tpg,
@@ -75,9 +75,9 @@ struct se_node_acl *core_tpg_get_initiator_node_acl(
{
struct se_node_acl *acl;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname);
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return acl;
}
@@ -198,10 +198,10 @@ static void target_add_node_acl(struct se_node_acl *acl)
{
struct se_portal_group *tpg = acl->se_tpg;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
list_add_tail(&acl->acl_list, &tpg->acl_node_list);
tpg->num_node_acls++;
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
pr_debug("%s_TPG[%hu] - Added %s ACL with TCQ Depth: %d for %s"
" Initiator Node: %s\n",
@@ -257,7 +257,7 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
{
struct se_node_acl *acl;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname);
if (acl) {
if (acl->dynamic_node_acl) {
@@ -265,7 +265,7 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
pr_debug("%s_TPG[%u] - Replacing dynamic ACL"
" for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
tpg->se_tpg_tfo->tpg_get_tag(tpg), initiatorname);
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return acl;
}
@@ -273,10 +273,10 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
" Node %s already exists for TPG %u, ignoring"
" request.\n", tpg->se_tpg_tfo->get_fabric_name(),
initiatorname, tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return ERR_PTR(-EEXIST);
}
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
acl = target_alloc_node_acl(tpg, initiatorname);
if (!acl)
@@ -294,13 +294,13 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl *acl)
unsigned long flags;
int rc;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
if (acl->dynamic_node_acl) {
acl->dynamic_node_acl = 0;
}
list_del(&acl->acl_list);
tpg->num_node_acls--;
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
spin_lock_irqsave(&acl->nacl_sess_lock, flags);
acl->acl_stop = 1;
@@ -357,21 +357,21 @@ int core_tpg_set_initiator_node_queue_depth(
unsigned long flags;
int dynamic_acl = 0;
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname);
if (!acl) {
pr_err("Access Control List entry for %s Initiator"
" Node %s does not exists for TPG %hu, ignoring"
" request.\n", tpg->se_tpg_tfo->get_fabric_name(),
initiatorname, tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return -ENODEV;
}
if (acl->dynamic_node_acl) {
acl->dynamic_node_acl = 0;
dynamic_acl = 1;
}
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
spin_lock_irqsave(&tpg->session_lock, flags);
list_for_each_entry(sess, &tpg->tpg_sess_list, sess_list) {
@@ -387,10 +387,10 @@ int core_tpg_set_initiator_node_queue_depth(
tpg->se_tpg_tfo->get_fabric_name(), initiatorname);
spin_unlock_irqrestore(&tpg->session_lock, flags);
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
if (dynamic_acl)
acl->dynamic_node_acl = 1;
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return -EEXIST;
}
/*
@@ -425,10 +425,10 @@ int core_tpg_set_initiator_node_queue_depth(
if (init_sess)
tpg->se_tpg_tfo->close_session(init_sess);
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
if (dynamic_acl)
acl->dynamic_node_acl = 1;
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return -EINVAL;
}
spin_unlock_irqrestore(&tpg->session_lock, flags);
@@ -444,10 +444,10 @@ int core_tpg_set_initiator_node_queue_depth(
initiatorname, tpg->se_tpg_tfo->get_fabric_name(),
tpg->se_tpg_tfo->tpg_get_tag(tpg));
- spin_lock_irq(&tpg->acl_node_lock);
+ mutex_lock(&tpg->acl_node_mutex);
if (dynamic_acl)
acl->dynamic_node_acl = 1;
- spin_unlock_irq(&tpg->acl_node_lock);
+ mutex_unlock(&tpg->acl_node_mutex);
return 0;
}
@@ -521,9 +521,9 @@ int core_tpg_register(
INIT_LIST_HEAD(&se_tpg->acl_node_list);
INIT_LIST_HEAD(&se_tpg->se_tpg_node);
INIT_LIST_HEAD(&se_tpg->tpg_sess_list);
- spin_lock_init(&se_tpg->acl_node_lock);
spin_lock_init(&se_tpg->session_lock);
mutex_init(&se_tpg->tpg_lun_mutex);
+ mutex_init(&se_tpg->acl_node_mutex);
if (se_tpg->proto_id >= 0) {
if (core_tpg_setup_virtual_lun0(se_tpg) < 0)
@@ -559,25 +559,26 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
while (atomic_read(&se_tpg->tpg_pr_ref_count) != 0)
cpu_relax();
+
/*
* Release any remaining demo-mode generated se_node_acl that have
* not been released because of TFO->tpg_check_demo_mode_cache() == 1
* in transport_deregister_session().
*/
- spin_lock_irq(&se_tpg->acl_node_lock);
+ mutex_lock(&se_tpg->acl_node_mutex);
list_for_each_entry_safe(nacl, nacl_tmp, &se_tpg->acl_node_list,
acl_list) {
list_del(&nacl->acl_list);
se_tpg->num_node_acls--;
- spin_unlock_irq(&se_tpg->acl_node_lock);
+ mutex_unlock(&se_tpg->acl_node_mutex);
core_tpg_wait_for_nacl_pr_ref(nacl);
core_free_device_list_for_node(nacl, se_tpg);
kfree(nacl);
- spin_lock_irq(&se_tpg->acl_node_lock);
+ mutex_lock(&se_tpg->acl_node_mutex);
}
- spin_unlock_irq(&se_tpg->acl_node_lock);
+ mutex_unlock(&se_tpg->acl_node_mutex);
if (se_tpg->proto_id >= 0)
core_tpg_remove_lun(se_tpg, &se_tpg->tpg_virt_lun0);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 6aa6287..2b9f41a 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -498,7 +498,7 @@ void transport_deregister_session(struct se_session *se_sess)
const struct target_core_fabric_ops *se_tfo;
struct se_node_acl *se_nacl;
unsigned long flags;
- bool comp_nacl = true;
+ bool comp_nacl = true, drop_nacl = false;
if (!se_tpg) {
transport_free_session(se_sess);
@@ -518,22 +518,22 @@ void transport_deregister_session(struct se_session *se_sess)
*/
se_nacl = se_sess->se_node_acl;
- spin_lock_irqsave(&se_tpg->acl_node_lock, flags);
+ mutex_lock(&se_tpg->acl_node_mutex);
if (se_nacl && se_nacl->dynamic_node_acl) {
if (!se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
list_del(&se_nacl->acl_list);
se_tpg->num_node_acls--;
- spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags);
- core_tpg_wait_for_nacl_pr_ref(se_nacl);
- core_free_device_list_for_node(se_nacl, se_tpg);
- kfree(se_nacl);
-
- comp_nacl = false;
- spin_lock_irqsave(&se_tpg->acl_node_lock, flags);
+ drop_nacl = true;
}
}
- spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags);
+ mutex_unlock(&se_tpg->acl_node_mutex);
+ if (drop_nacl) {
+ core_tpg_wait_for_nacl_pr_ref(se_nacl);
+ core_free_device_list_for_node(se_nacl, se_tpg);
+ kfree(se_nacl);
+ comp_nacl = false;
+ }
pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n",
se_tpg->se_tpg_tfo->get_fabric_name());
/*
diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c
index ee56531..8e1a54f 100644
--- a/drivers/target/tcm_fc/tfc_conf.c
+++ b/drivers/target/tcm_fc/tfc_conf.c
@@ -217,7 +217,7 @@ struct ft_node_acl *ft_acl_get(struct ft_tpg *tpg, struct fc_rport_priv *rdata)
struct se_portal_group *se_tpg = &tpg->se_tpg;
struct se_node_acl *se_acl;
- spin_lock_irq(&se_tpg->acl_node_lock);
+ mutex_lock(&se_tpg->acl_node_mutex);
list_for_each_entry(se_acl, &se_tpg->acl_node_list, acl_list) {
acl = container_of(se_acl, struct ft_node_acl, se_node_acl);
pr_debug("acl %p port_name %llx\n",
@@ -231,7 +231,7 @@ struct ft_node_acl *ft_acl_get(struct ft_tpg *tpg, struct fc_rport_priv *rdata)
break;
}
}
- spin_unlock_irq(&se_tpg->acl_node_lock);
+ mutex_unlock(&se_tpg->acl_node_mutex);
return found;
}
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 1049f6c..f6cf401 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -875,7 +875,7 @@ struct se_portal_group {
/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
atomic_t tpg_pr_ref_count;
/* Spinlock for adding/removing ACLed Nodes */
- spinlock_t acl_node_lock;
+ struct mutex acl_node_mutex;
/* Spinlock for adding/removing sessions */
spinlock_t session_lock;
struct mutex tpg_lun_mutex;
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch converts core_tpg_deregister() to perform a list splice
for any remaining dynamically generated se_node_acls attached to
se_tpg, before calling kfree(nacl) to free memory.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_tpg.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 229e827..e7745d2 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -547,6 +547,7 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
{
const struct target_core_fabric_ops *tfo = se_tpg->se_tpg_tfo;
struct se_node_acl *nacl, *nacl_tmp;
+ LIST_HEAD(node_list);
pr_debug("TARGET_CORE[%s]: Deallocating portal_group for endpoint: %s, "
"Proto: %d, Portal Tag: %u\n", tfo->get_fabric_name(),
@@ -560,25 +561,22 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
while (atomic_read(&se_tpg->tpg_pr_ref_count) != 0)
cpu_relax();
+ mutex_lock(&se_tpg->acl_node_mutex);
+ list_splice_init(&se_tpg->acl_node_list, &node_list);
+ mutex_unlock(&se_tpg->acl_node_mutex);
/*
* Release any remaining demo-mode generated se_node_acl that have
* not been released because of TFO->tpg_check_demo_mode_cache() == 1
* in transport_deregister_session().
*/
- mutex_lock(&se_tpg->acl_node_mutex);
- list_for_each_entry_safe(nacl, nacl_tmp, &se_tpg->acl_node_list,
- acl_list) {
+ list_for_each_entry_safe(nacl, nacl_tmp, &node_list, acl_list) {
list_del(&nacl->acl_list);
se_tpg->num_node_acls--;
- mutex_unlock(&se_tpg->acl_node_mutex);
core_tpg_wait_for_nacl_pr_ref(nacl);
core_free_device_list_for_node(nacl, se_tpg);
kfree(nacl);
-
- mutex_lock(&se_tpg->acl_node_mutex);
}
- mutex_unlock(&se_tpg->acl_node_mutex);
if (se_tpg->proto_id >= 0)
core_tpg_remove_lun(se_tpg, &se_tpg->tpg_virt_lun0);
--
1.9.1
From: Nicholas Bellinger <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_device.c | 15 ---------------
drivers/target/target_core_tpg.c | 4 ----
include/target/target_core_base.h | 3 ---
3 files changed, 22 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 99a60e6..7674c8f 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1232,7 +1232,6 @@ struct se_lun_acl *core_dev_init_initiator_node_lun_acl(
return NULL;
}
- INIT_LIST_HEAD(&lacl->lacl_list);
lacl->mapped_lun = mapped_lun;
lacl->se_lun_nacl = nacl;
snprintf(lacl->initiatorname, TRANSPORT_IQN_LEN, "%s",
@@ -1262,11 +1261,6 @@ int core_dev_add_initiator_node_lun_acl(
lun_access, nacl, tpg) < 0)
return -EINVAL;
- spin_lock(&lun->lun_acl_lock);
- list_add_tail(&lacl->lacl_list, &lun->lun_acl_list);
- atomic_inc_mb(&lun->lun_acl_count);
- spin_unlock(&lun->lun_acl_lock);
-
pr_debug("%s_TPG[%hu]_LUN[%u->%u] - Added %s ACL for "
" InitiatorNode: %s\n", tpg->se_tpg_tfo->get_fabric_name(),
tpg->se_tpg_tfo->tpg_get_tag(tpg), lun->unpacked_lun, lacl->mapped_lun,
@@ -1293,19 +1287,12 @@ int core_dev_del_initiator_node_lun_acl(
if (!nacl)
return -EINVAL;
- spin_lock(&lun->lun_acl_lock);
- list_del(&lacl->lacl_list);
- atomic_dec_mb(&lun->lun_acl_count);
- spin_unlock(&lun->lun_acl_lock);
-
mutex_lock(&nacl->lun_entry_mutex);
deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
if (deve)
core_disable_device_list_for_node(lun, deve, nacl, tpg);
mutex_unlock(&nacl->lun_entry_mutex);
- lacl->se_lun = NULL;
-
pr_debug("%s_TPG[%hu]_LUN[%u] - Removed ACL for"
" InitiatorNode: %s Mapped LUN: %u\n",
tpg->se_tpg_tfo->get_fabric_name(),
@@ -1435,8 +1422,6 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
xcopy_lun = &dev->xcopy_lun;
xcopy_lun->lun_se_dev = dev;
init_completion(&xcopy_lun->lun_shutdown_comp);
- INIT_LIST_HEAD(&xcopy_lun->lun_acl_list);
- spin_lock_init(&xcopy_lun->lun_acl_lock);
spin_lock_init(&xcopy_lun->lun_sep_lock);
init_completion(&xcopy_lun->lun_ref_comp);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index e7745d2..73c25bd 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -495,8 +495,6 @@ static int core_tpg_setup_virtual_lun0(struct se_portal_group *se_tpg)
lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
atomic_set(&lun->lun_acl_count, 0);
init_completion(&lun->lun_shutdown_comp);
- INIT_LIST_HEAD(&lun->lun_acl_list);
- spin_lock_init(&lun->lun_acl_lock);
spin_lock_init(&lun->lun_sep_lock);
init_completion(&lun->lun_ref_comp);
@@ -610,8 +608,6 @@ struct se_lun *core_tpg_alloc_lun(
lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
atomic_set(&lun->lun_acl_count, 0);
init_completion(&lun->lun_shutdown_comp);
- INIT_LIST_HEAD(&lun->lun_acl_list);
- spin_lock_init(&lun->lun_acl_lock);
spin_lock_init(&lun->lun_sep_lock);
init_completion(&lun->lun_ref_comp);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index f6cf401..25d5ebe 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -632,7 +632,6 @@ struct se_lun_acl {
u32 mapped_lun;
struct se_node_acl *se_lun_nacl;
struct se_lun *se_lun;
- struct list_head lacl_list;
struct config_group se_lun_group;
struct se_ml_stat_grps ml_stat_grps;
};
@@ -714,10 +713,8 @@ struct se_lun {
u32 lun_flags;
u32 unpacked_lun;
atomic_t lun_acl_count;
- spinlock_t lun_acl_lock;
spinlock_t lun_sep_lock;
struct completion lun_shutdown_comp;
- struct list_head lun_acl_list;
struct se_device *lun_se_dev;
struct se_port *lun_sep;
struct config_group lun_group;
--
1.9.1
From: Nicholas Bellinger <[email protected]>
This patch changes core_tpg_add_node_to_devs() to avoid unnecessarly
resetting every se_dev_entry in se_node_acl->tpg_lun_hlist when the
operation is driven by an explicit configfs se_lun->lun_group creation
via core_dev_add_lun() to only update a single se_lun.
Otherwise for the second core_tpg_check_initiator_node_acl() case, go
ahead and continue to scan the full set of currently active se_lun in
se_portal_group->tpg_lun_hlist.
Cc: Hannes Reinecke <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Signed-off-by: Nicholas Bellinger <[email protected]>
---
drivers/target/target_core_device.c | 2 +-
drivers/target/target_core_internal.h | 3 ++-
drivers/target/target_core_tpg.c | 7 +++++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 7674c8f..c80a7a8 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1186,7 +1186,7 @@ int core_dev_add_lun(
if (acl->dynamic_node_acl &&
(!tpg->se_tpg_tfo->tpg_check_demo_mode_login_only ||
!tpg->se_tpg_tfo->tpg_check_demo_mode_login_only(tpg))) {
- core_tpg_add_node_to_devs(acl, tpg);
+ core_tpg_add_node_to_devs(acl, tpg, lun);
}
}
mutex_unlock(&tpg->acl_node_mutex);
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 2c160ce..ce80ca7 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -64,7 +64,8 @@ extern struct se_device *g_lun0_dev;
struct se_node_acl *__core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
const char *);
-void core_tpg_add_node_to_devs(struct se_node_acl *, struct se_portal_group *);
+void core_tpg_add_node_to_devs(struct se_node_acl *, struct se_portal_group *,
+ struct se_lun *);
void core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *);
struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32);
int core_tpg_add_lun(struct se_portal_group *, struct se_lun *,
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 73c25bd..f66c208 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -89,7 +89,8 @@ EXPORT_SYMBOL(core_tpg_get_initiator_node_acl);
*/
void core_tpg_add_node_to_devs(
struct se_node_acl *acl,
- struct se_portal_group *tpg)
+ struct se_portal_group *tpg,
+ struct se_lun *lun_orig)
{
u32 lun_access = 0;
struct se_lun *lun;
@@ -99,6 +100,8 @@ void core_tpg_add_node_to_devs(
hlist_for_each_entry_rcu(lun, &tpg->tpg_lun_hlist, link) {
if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
continue;
+ if (lun_orig && lun != lun_orig)
+ continue;
dev = lun->lun_se_dev;
/*
@@ -238,7 +241,7 @@ struct se_node_acl *core_tpg_check_initiator_node_acl(
*/
if ((tpg->se_tpg_tfo->tpg_check_demo_mode_login_only == NULL) ||
(tpg->se_tpg_tfo->tpg_check_demo_mode_login_only(tpg) != 1))
- core_tpg_add_node_to_devs(acl, tpg);
+ core_tpg_add_node_to_devs(acl, tpg, NULL);
target_add_node_acl(acl);
return acl;
--
1.9.1
On 05/22/2015 08:11 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <[email protected]>
>
> Hi all,
>
> Here is -v2 series for converting LIO target se_node_acl + se_lun
> mapping tables from fixed size arrays to dynamic RCU hlist_heads.
>
> This turns fast-path I/O into a lock-less RCU reader using existing
> percpu based se_lun->lun_ref logic, and converts the RCU updater
> path to allow for an arbitrary number of LUNs for both types of
> mappings within target-core.
>
> This series also squashes a number of previous se_node_acl RCU
> related changes into a single commit (#1) for easier review,
> and to avoid potential bisect issues.
>
> There have been a number of changes since -v1, including:
>
> - Mirror port->sep_rtpi in lun->lun_rtpi for RCU
> - Drop unnecessary synchronize_rcu() usage
> - Convert call_rcu() to kfree_rcu() usage
> - Move hlist_del_rcu head of rcu_assign_pointer in se_dev_entry
> - Drop unnecessary lookup deve in target_fabric_mappedlun_unlink()
> - Add target_lun_is_rdonly helper
> - Acquire lun_entry_mutex during core_disable_device_list_for_node
> - Drop TRANSPORT_LUNFLAGS_*_ACCESS usage
> - Pass se_dev_entry directly to core_disable_device_list_for_node
> - Convert sbp-target se_lun usage to use ->login_lun
> - Fix se_session dereference in spc_emulate_report_luns
> - Fix testing for NULL instead of IS_ERR in fabric_make_lun()
> - Convert BUG_ON to EINVAL for wrong dynamic -> explicit ACL conversion
> - Add missing hlist_del_rcu when swapping orig with new
> - Add HBA_FLAGS_INTERNAL_USE checks in add/remove lun
>
> Please review.
>
Very nice.
Reviewed-by: Hannes Reinecke <[email protected]>
Although it would be _perfect_ if you could move to full 64-bit
LUNs; now that the LUN array is gone there is no need to have a
limitation on the number of LUNs, and we can trivially support
64-bit LUNs ...
But then, I can easily send a patch once this one is in.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)
Can you put up a git branch with these? Without that or a known
good baseline it's hard to test, or even to do a deep review.
On Fri, 2015-05-22 at 01:07 -0700, Christoph Hellwig wrote:
> Can you put up a git branch with these? Without that or a known
> good baseline it's hard to test, or even to do a deep review.
Pushed to target-pending/for-next.
> - spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
> - se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
> - if (se_cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
> - struct se_dev_entry *deve = se_cmd->se_deve;
> -
> + rcu_read_lock();
> + deve = target_nacl_find_deve(nacl, unpacked_lun);
> + if (deve) {
> deve->total_cmds++;
This update will now be racy, ditto for the read/write_bytes update
later.
> +bool target_lun_is_rdonly(struct se_cmd *cmd)
> +{
> + struct se_session *se_sess = cmd->se_sess;
> + struct se_dev_entry *deve;
> + bool ret;
> +
> + if (cmd->se_lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY)
> + return true;
> +
> + rcu_read_lock();
> + deve = target_nacl_find_deve(se_sess->se_node_acl, cmd->orig_fe_lun);
> + ret = (deve && deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY);
> + rcu_read_unlock();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(target_lun_is_rdonly);
This should be a separate prep patch like, like it was in my original
version. I also still think you want this whole patch:
http://git.infradead.org/users/hch/scsi.git/commitdiff/e9a71bda1a120e0488c5c4e4b2f17f14333e2dc6
as storing a pointer to the dev entry without a refcount is bound to
cause trouble. I don't have a tree with all the patches applied
available, but I doubt it fully gets that right.
> +void target_pr_kref_release(struct kref *kref)
> +{
> + struct se_dev_entry *deve = container_of(kref, struct se_dev_entry,
> + pr_kref);
> + complete(&deve->pr_comp);
> }
>
> /* core_enable_device_list_for_node():
> + kref_put(&orig->pr_kref, target_pr_kref_release);
> + wait_for_completion(&orig->pr_comp);
>
> + kref_put(&orig->pr_kref, target_pr_kref_release);
> /*
> - * Disable struct se_dev_entry LUN ACL mapping
> + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
> + * or REGISTER_AND_MOVE PR operation to complete.
> */
> + wait_for_completion(&orig->pr_comp);
> + kfree_rcu(orig, rcu_head);
The release callback should just call kfree_rcu, no need to wait for the
release in the caller.
Also can you drop the _pr from the name? It's a generic refcount now
even if the PR code is the only consumer so far.
> +void target_pr_kref_release(struct kref *);
Instead of exporting the release function it would be much more obvious
to have a
void target_deve_put(struct se_dev_entry *deve)
{
kref_put(&deve->pr_kref, target_deve_release);
}
helper. Probably paired with one for the get side.
> static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
> {
> - struct se_lun_acl *lun_acl = se_deve->se_lun_acl;
> + struct se_lun_acl *lun_acl;
> struct se_node_acl *nacl;
> struct se_portal_group *tpg;
> +
> + if (!se_deve) {
> + pr_err("core_scsi3_lunacl_undepend_item passed NULL se_deve\n");
> + dump_stack();
> + return;
> + }
How could this happen and how is it related to this patch?
> - if (!deve->se_lun || !deve->se_lun_acl) {
> - spin_unlock_irq(&nacl->device_list_lock);
> + rcu_read_lock();
> + deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
> + if (!deve) {
> + rcu_read_unlock();
> return -ENODEV;
So previously a lot of these files returned -ENODEV when not having
an explicit node ACL, and now they don't. If that was intentional
it should be documented in the changelog, or preferably moved into
a preparation patch of its own.
> + struct se_node_acl *se_node_acl;
Where is this field coming from? It's not documented in the changelog
and doesn't seem to be actually used either.
On Fri, May 22, 2015 at 06:11:04AM +0000, Nicholas A. Bellinger wrote:
> + clear_bit(1, &orig->pr_reg);
Can you call it ->flags and give the bit a meaningful name?
> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index c0b593a..d29b39c 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -327,9 +327,13 @@ static int core_scsi3_pr_seq_non_holder(
> int we = 0; /* Write Exclusive */
> int legacy = 0; /* Act like a legacy device and return
> * RESERVATION CONFLICT on some CDBs */
> + bool registered = false;
>
> rcu_read_lock();
> se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
> + if (se_deve)
> + registered = test_bit(1, &se_deve->pr_reg);
> + rcu_read_unlock();
It would be good to just sort out the registered and co variables
here before the RCU changes, as in:
http://git.infradead.org/users/hch/scsi.git/commitdiff/6372d9f62c83acb30d051387c40deb4dbdcaa376
Oh, you've actually go it - still would make sense to move it the beginning of
the series.
> This patch converts the fixed size se_portal_group->tpg_lun_list[]
> to use modern RCU with hlist_head in order to support an arbitary
> number of se_lun ports per target endpoint.
>
> It includes dropping core_tpg_alloc_lun() from core_dev_add_lun(),
> and calling it directly from target_fabric_make_lun() to allocate
> a new se_lun.
>
> Also add a new target_fabric_port_release() configfs item callback
> to invoke kfree_rcu() to release memory during se_lun->lun_group
> shutdown.
>
> Also now that se_node_acl->lun_entry_hlist is using RCU, convert
> existing tpg_lun_lock to struct mutex so core_tpg_add_node_to_devs()
> can perform RCU updater logic without releasing ->tpg_lun_mutex.
Still doesn't explain why core_tpg_clear_object_luns also disappears.
> + hlist_for_each_entry_rcu(lun, &tpg->tpg_lun_hlist, link) {
> if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
> continue;
lun_status will always be active here as that's set and cleared at the
same time as the list addition / removal. Which means that lun_status
should go away in this patch.
On Fri, 2015-05-22 at 10:31 +0200, Christoph Hellwig wrote:
> > This patch converts the fixed size se_portal_group->tpg_lun_list[]
> > to use modern RCU with hlist_head in order to support an arbitary
> > number of se_lun ports per target endpoint.
> >
> > It includes dropping core_tpg_alloc_lun() from core_dev_add_lun(),
> > and calling it directly from target_fabric_make_lun() to allocate
> > a new se_lun.
> >
> > Also add a new target_fabric_port_release() configfs item callback
> > to invoke kfree_rcu() to release memory during se_lun->lun_group
> > shutdown.
> >
> > Also now that se_node_acl->lun_entry_hlist is using RCU, convert
> > existing tpg_lun_lock to struct mutex so core_tpg_add_node_to_devs()
> > can perform RCU updater logic without releasing ->tpg_lun_mutex.
>
> Still doesn't explain why core_tpg_clear_object_luns also disappears.
It's duplicate logic in iscsi-target to delete active TPG LUNs.
Adding a comment to clarify the removal.
>
> > + hlist_for_each_entry_rcu(lun, &tpg->tpg_lun_hlist, link) {
> > if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
> > continue;
>
> lun_status will always be active here as that's set and cleared at the
> same time as the list addition / removal. Which means that lun_status
> should go away in this patch.
>
Yes, thanks for the reminder. Dropping lun_status now.
(resending)
On Fri, 2015-05-22 at 01:24 -0700, Christoph Hellwig wrote:
> > - spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
> > - se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
> > - if (se_cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
> > - struct se_dev_entry *deve = se_cmd->se_deve;
> > -
> > + rcu_read_lock();
> > + deve = target_nacl_find_deve(nacl, unpacked_lun);
> > + if (deve) {
> > deve->total_cmds++;
>
> This update will now be racy, ditto for the read/write_bytes update
> later.
This should become an atomic_long_t increment, yes..?
>
> > +bool target_lun_is_rdonly(struct se_cmd *cmd)
> > +{
> > + struct se_session *se_sess = cmd->se_sess;
> > + struct se_dev_entry *deve;
> > + bool ret;
> > +
> > + if (cmd->se_lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY)
> > + return true;
> > +
> > + rcu_read_lock();
> > + deve = target_nacl_find_deve(se_sess->se_node_acl, cmd->orig_fe_lun);
> > + ret = (deve && deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY);
> > + rcu_read_unlock();
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(target_lun_is_rdonly);
>
> This should be a separate prep patch like, like it was in my original
> version. I also still think you want this whole patch:
>
> http://git.infradead.org/users/hch/scsi.git/commitdiff/e9a71bda1a120e0488c5c4e4b2f17f14333e2dc6
>
> as storing a pointer to the dev entry without a refcount is bound to
> cause trouble. I don't have a tree with all the patches applied
> available, but I doubt it fully gets that right.
>
Yes, this helper is from your patch above.
Considering there is a single user of it here, and complexities involved
for a RCU conversion + bisect, is it really work adding as a separate
patch ahead of this one..?
> > +void target_pr_kref_release(struct kref *kref)
> > +{
> > + struct se_dev_entry *deve = container_of(kref, struct se_dev_entry,
> > + pr_kref);
> > + complete(&deve->pr_comp);
> > }
> >
> > /* core_enable_device_list_for_node():
>
> > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > + wait_for_completion(&orig->pr_comp);
> >
>
> > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > /*
> > - * Disable struct se_dev_entry LUN ACL mapping
> > + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
> > + * or REGISTER_AND_MOVE PR operation to complete.
> > */
> > + wait_for_completion(&orig->pr_comp);
> > + kfree_rcu(orig, rcu_head);
>
> The release callback should just call kfree_rcu, no need to wait for the
> release in the caller.
>
Why doesn't se_dev_entry release this need to wait for the special case
references to drop..?
> Also can you drop the _pr from the name? It's a generic refcount now
> even if the PR code is the only consumer so far.
>
> > +void target_pr_kref_release(struct kref *);
>
> Instead of exporting the release function it would be much more obvious
> to have a
>
> void target_deve_put(struct se_dev_entry *deve)
> {
> kref_put(&deve->pr_kref, target_deve_release);
> }
>
> helper. Probably paired with one for the get side.
>
Sure. Adding this now.
> > static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
> > {
> > - struct se_lun_acl *lun_acl = se_deve->se_lun_acl;
> > + struct se_lun_acl *lun_acl;
> > struct se_node_acl *nacl;
> > struct se_portal_group *tpg;
> > +
> > + if (!se_deve) {
> > + pr_err("core_scsi3_lunacl_undepend_item passed NULL se_deve\n");
> > + dump_stack();
> > + return;
> > + }
>
> How could this happen and how is it related to this patch?
>
Dropped.
> > - if (!deve->se_lun || !deve->se_lun_acl) {
> > - spin_unlock_irq(&nacl->device_list_lock);
> > + rcu_read_lock();
> > + deve = target_nacl_find_deve(nacl, lacl->mapped_lun);
> > + if (!deve) {
> > + rcu_read_unlock();
> > return -ENODEV;
>
> So previously a lot of these files returned -ENODEV when not having
> an explicit node ACL, and now they don't. If that was intentional
> it should be documented in the changelog, or preferably moved into
> a preparation patch of its own.
>
Ok, will update the changelog for this.
> > + struct se_node_acl *se_node_acl;
>
> Where is this field coming from? It's not documented in the changelog
> and doesn't seem to be actually used either.
>
Nice catch. Dropping this unused pointer now.
On Fri, 2015-05-22 at 10:26 +0200, Christoph Hellwig wrote:
> On Fri, May 22, 2015 at 06:11:04AM +0000, Nicholas A. Bellinger wrote:
> > + clear_bit(1, &orig->pr_reg);
>
> Can you call it ->flags and give the bit a meaningful name?
The bit is signaling if se_dev_entry has a PR registration active.
I don't see how ->flags is a more meaningful name without other bits
defined.
>
> > diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> > index c0b593a..d29b39c 100644
> > --- a/drivers/target/target_core_pr.c
> > +++ b/drivers/target/target_core_pr.c
> > @@ -327,9 +327,13 @@ static int core_scsi3_pr_seq_non_holder(
> > int we = 0; /* Write Exclusive */
> > int legacy = 0; /* Act like a legacy device and return
> > * RESERVATION CONFLICT on some CDBs */
> > + bool registered = false;
> >
> > rcu_read_lock();
> > se_deve = target_nacl_find_deve(nacl, cmd->orig_fe_lun);
> > + if (se_deve)
> > + registered = test_bit(1, &se_deve->pr_reg);
> > + rcu_read_unlock();
>
> It would be good to just sort out the registered and co variables
> here before the RCU changes, as in:
>
> http://git.infradead.org/users/hch/scsi.git/commitdiff/6372d9f62c83acb30d051387c40deb4dbdcaa376
Why not just keep this patch squashed into the relevant commit in the
context of the larger RCU conversion..?
On 05/22/15 08:11, Nicholas A. Bellinger wrote:
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index e2c0eaf..def5bc8 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -638,7 +638,6 @@ struct se_lun_acl {
> };
>
> struct se_dev_entry {
> - bool def_pr_registered;
> /* See transport_lunflags_table */
> u32 lun_flags;
> u32 mapped_lun;
> @@ -655,7 +654,8 @@ struct se_dev_entry {
> struct se_node_acl *se_node_acl;
> struct se_lun_acl __rcu *se_lun_acl;
> spinlock_t ua_lock;
> - struct se_lun *se_lun;
> + struct se_lun __rcu *se_lun;
> + unsigned long pr_reg;
> struct list_head alua_port_list;
> struct list_head ua_list;
> struct hlist_node link;
Hello Nic,
This change causes the "se_lun = deve->se_lun" assignment in
transport_lookup_cmd_lun() to assign an RCU pointer to a non-RCU
pointer. Shouldn't such an assignment be protected via rcu_dereference() ?
Thanks,
Bart.
On 05/22/15 08:11, Nicholas A. Bellinger wrote:
> Here is -v2 series for converting LIO target se_node_acl + se_lun
> mapping tables from fixed size arrays to dynamic RCU hlist_heads.
The full list of new sparse warnings introduced by this patch series for
source files under drivers/target is as follows:
$ make M=drivers/target C=2
CHECK drivers/target/target_core_configfs.c
CC [M] drivers/target/target_core_configfs.o
CHECK drivers/target/target_core_device.c
drivers/target/target_core_device.c:89:24: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:89:24: expected struct se_lun *se_lun
drivers/target/target_core_device.c:89:24: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:90:32: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:90:32: expected struct se_lun *se_lun
drivers/target/target_core_device.c:90:32: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:130:24: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:130:24: expected struct se_device *se_dev
drivers/target/target_core_device.c:130:24: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:132:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:132:13: expected struct se_device *dev
drivers/target/target_core_device.c:132:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:158:33: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:158:33: expected struct se_lun *tmr_lun
drivers/target/target_core_device.c:158:33: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:159:32: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:159:32: expected struct se_lun *se_lun
drivers/target/target_core_device.c:159:32: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:160:24: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:160:24: expected struct se_lun *se_lun
drivers/target/target_core_device.c:160:24: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:175:24: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:175:24: expected struct se_device *se_dev
drivers/target/target_core_device.c:175:24: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:176:25: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:176:25: expected struct se_device *tmr_dev
drivers/target/target_core_device.c:176:25: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:219:21: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:219:21: expected struct se_lun *lun
drivers/target/target_core_device.c:219:21: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:247:55: warning: incorrect type in argument 1 (different address spaces)
drivers/target/target_core_device.c:247:55: expected struct se_lun *<noident>
drivers/target/target_core_device.c:247:55: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_device.c:335:34: error: incompatible types in comparison expression (different address spaces)
drivers/target/target_core_device.c:421:45: warning: incorrect type in argument 1 (different address spaces)
drivers/target/target_core_device.c:421:45: expected struct se_device *<noident>
drivers/target/target_core_device.c:421:45: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:438:33: error: incompatible types in comparison expression (different address spaces)
drivers/target/target_core_device.c:519:6: warning: symbol 'se_dev_check_wce' was not declared. Should it be static?
drivers/target/target_core_device.c:643:48: warning: incorrect type in argument 1 (different address spaces)
drivers/target/target_core_device.c:643:48: expected struct se_device *<noident>
drivers/target/target_core_device.c:643:48: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:793:31: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_device.c:793:31: expected struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_device.c:793:31: got struct se_device *[assigned] dev
CC [M] drivers/target/target_core_device.o
CHECK drivers/target/target_core_fabric_configfs.c
CC [M] drivers/target/target_core_fabric_configfs.o
CHECK drivers/target/target_core_fabric_lib.c
CC [M] drivers/target/target_core_fabric_lib.o
CHECK drivers/target/target_core_hba.c
CC [M] drivers/target/target_core_hba.o
CHECK drivers/target/target_core_pr.c
drivers/target/target_core_pr.c:1412:45: warning: incorrect type in initializer (different address spaces)
drivers/target/target_core_pr.c:1412:45: expected struct se_lun_acl *lun_acl
drivers/target/target_core_pr.c:1412:45: got struct se_lun_acl [noderef] <asn:4>*se_lun_acl
drivers/target/target_core_pr.c:1435:17: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_pr.c:1435:17: expected struct se_lun_acl *lun_acl
drivers/target/target_core_pr.c:1435:17: got struct se_lun_acl [noderef] <asn:4>*se_lun_acl
drivers/target/target_core_pr.c:763:67: warning: incorrect type in argument 3 (different address spaces)
drivers/target/target_core_pr.c:763:67: expected struct se_lun *lun
drivers/target/target_core_pr.c:763:67: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_pr.c:1718:68: warning: incorrect type in argument 3 (different address spaces)
drivers/target/target_core_pr.c:1718:68: expected struct se_lun *lun
drivers/target/target_core_pr.c:1718:68: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_pr.c:3419:53: warning: incorrect type in argument 3 (different address spaces)
drivers/target/target_core_pr.c:3419:53: expected struct se_lun *lun
drivers/target/target_core_pr.c:3419:53: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_pr.c:719:44: warning: dereference of noderef expression
CC [M] drivers/target/target_core_pr.o
CHECK drivers/target/target_core_alua.c
drivers/target/target_core_alua.c:978:30: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_alua.c:978:30: expected struct se_lun_acl *lacl
drivers/target/target_core_alua.c:978:30: got struct se_lun_acl [noderef] <asn:4>*se_lun_acl
drivers/target/target_core_alua.c:1938:36: warning: incorrect type in initializer (different address spaces)
drivers/target/target_core_alua.c:1938:36: expected struct se_device *dev
drivers/target/target_core_alua.c:1938:36: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_alua.c:2192:36: warning: incorrect type in initializer (different address spaces)
drivers/target/target_core_alua.c:2192:36: expected struct se_device *dev
drivers/target/target_core_alua.c:2192:36: got struct se_device [noderef] <asn:4>*lun_se_dev
CC [M] drivers/target/target_core_alua.o
CHECK drivers/target/target_core_tmr.c
CC [M] drivers/target/target_core_tmr.o
CHECK drivers/target/target_core_tpg.c
drivers/target/target_core_tpg.c:108:21: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_tpg.c:108:21: expected struct se_device *dev
drivers/target/target_core_tpg.c:108:21: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_tpg.c:669:36: warning: incorrect type in initializer (different address spaces)
drivers/target/target_core_tpg.c:669:36: expected struct se_device *dev
drivers/target/target_core_tpg.c:669:36: got struct se_device [noderef] <asn:4>*lun_se_dev
CC [M] drivers/target/target_core_tpg.o
CHECK drivers/target/target_core_transport.c
CC [M] drivers/target/target_core_transport.o
CHECK drivers/target/target_core_sbc.c
CC [M] drivers/target/target_core_sbc.o
CHECK drivers/target/target_core_spc.c
drivers/target/target_core_spc.c:695:17: error: incompatible types in comparison expression (different address spaces)
CC [M] drivers/target/target_core_spc.o
CHECK drivers/target/target_core_ua.c
CC [M] drivers/target/target_core_ua.o
CHECK drivers/target/target_core_rd.c
CC [M] drivers/target/target_core_rd.o
CHECK drivers/target/target_core_stat.c
drivers/target/target_core_stat.c:549:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:549:13: expected struct se_device *dev
drivers/target/target_core_stat.c:549:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:565:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:565:13: expected struct se_device *dev
drivers/target/target_core_stat.c:565:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:595:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:595:13: expected struct se_device *dev
drivers/target/target_core_stat.c:595:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:668:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:668:13: expected struct se_device *dev
drivers/target/target_core_stat.c:668:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:684:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:684:13: expected struct se_device *dev
drivers/target/target_core_stat.c:684:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:839:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:839:13: expected struct se_device *dev
drivers/target/target_core_stat.c:839:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:879:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:879:13: expected struct se_device *dev
drivers/target/target_core_stat.c:879:13: got struct se_device [noderef] <asn:4>*lun_se_dev
drivers/target/target_core_stat.c:997:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:997:13: expected struct se_lun *lun
drivers/target/target_core_stat.c:997:13: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_stat.c:1363:13: warning: incorrect type in assignment (different address spaces)
drivers/target/target_core_stat.c:1363:13: expected struct se_lun *lun
drivers/target/target_core_stat.c:1363:13: got struct se_lun [noderef] <asn:4>*se_lun
drivers/target/target_core_stat.c:999:52: warning: dereference of noderef expression
drivers/target/target_core_stat.c:1365:52: warning: dereference of noderef expression
CC [M] drivers/target/target_core_stat.o
CHECK drivers/target/target_core_xcopy.c
CC [M] drivers/target/target_core_xcopy.o
LD [M] drivers/target/target_core_mod.o
CHECK drivers/target/target_core_iblock.c
CC [M] drivers/target/target_core_iblock.o
CHECK drivers/target/target_core_file.c
CC [M] drivers/target/target_core_file.o
CHECK drivers/target/target_core_pscsi.c
CC [M] drivers/target/target_core_pscsi.o
CHECK drivers/target/iscsi/iscsi_target_parameters.c
CC [M] drivers/target/iscsi/iscsi_target_parameters.o
CHECK drivers/target/iscsi/iscsi_target_seq_pdu_list.c
CC [M] drivers/target/iscsi/iscsi_target_seq_pdu_list.o
CHECK drivers/target/iscsi/iscsi_target_auth.c
CC [M] drivers/target/iscsi/iscsi_target_auth.o
CHECK drivers/target/iscsi/iscsi_target_datain_values.c
CC [M] drivers/target/iscsi/iscsi_target_datain_values.o
CHECK drivers/target/iscsi/iscsi_target_device.c
CC [M] drivers/target/iscsi/iscsi_target_device.o
CHECK drivers/target/iscsi/iscsi_target_erl0.c
CC [M] drivers/target/iscsi/iscsi_target_erl0.o
CHECK drivers/target/iscsi/iscsi_target_erl1.c
CC [M] drivers/target/iscsi/iscsi_target_erl1.o
CHECK drivers/target/iscsi/iscsi_target_erl2.c
CC [M] drivers/target/iscsi/iscsi_target_erl2.o
CHECK drivers/target/iscsi/iscsi_target_login.c
CC [M] drivers/target/iscsi/iscsi_target_login.o
CHECK drivers/target/iscsi/iscsi_target_nego.c
CC [M] drivers/target/iscsi/iscsi_target_nego.o
CHECK drivers/target/iscsi/iscsi_target_nodeattrib.c
CC [M] drivers/target/iscsi/iscsi_target_nodeattrib.o
CHECK drivers/target/iscsi/iscsi_target_tmr.c
CC [M] drivers/target/iscsi/iscsi_target_tmr.o
CHECK drivers/target/iscsi/iscsi_target_tpg.c
CC [M] drivers/target/iscsi/iscsi_target_tpg.o
CHECK drivers/target/iscsi/iscsi_target_util.c
CC [M] drivers/target/iscsi/iscsi_target_util.o
CHECK drivers/target/iscsi/iscsi_target.c
CC [M] drivers/target/iscsi/iscsi_target.o
CHECK drivers/target/iscsi/iscsi_target_configfs.c
CC [M] drivers/target/iscsi/iscsi_target_configfs.o
CHECK drivers/target/iscsi/iscsi_target_stat.c
CC [M] drivers/target/iscsi/iscsi_target_stat.o
CHECK drivers/target/iscsi/iscsi_target_transport.c
CC [M] drivers/target/iscsi/iscsi_target_transport.o
LD [M] drivers/target/iscsi/iscsi_target_mod.o
CHECK drivers/target/loopback/tcm_loop.c
CC [M] drivers/target/loopback/tcm_loop.o
CHECK drivers/target/sbp/sbp_target.c
drivers/target/sbp/sbp_target.c:1909:21: warning: incorrect type in assignment (different address spaces)
drivers/target/sbp/sbp_target.c:1909:21: expected struct se_device *dev
drivers/target/sbp/sbp_target.c:1909:21: got struct se_device [noderef] <asn:4>*lun_se_dev
On Fri, May 22, 2015 at 01:55:30AM -0700, Nicholas A. Bellinger wrote:
> > This update will now be racy, ditto for the read/write_bytes update
> > later.
>
> This should become an atomic_long_t increment, yes..?
Yes.
> Yes, this helper is from your patch above.
>
> Considering there is a single user of it here, and complexities involved
> for a RCU conversion + bisect, is it really work adding as a separate
> patch ahead of this one..?
The golden Linus style is to put preparatory patches first so that the
actual logic change is as small as possible. Adding helpers so that
low level accesses that will e changed soon is a very typical case for that.
> > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > + wait_for_completion(&orig->pr_comp);
> > >
> >
> > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > /*
> > > - * Disable struct se_dev_entry LUN ACL mapping
> > > + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
> > > + * or REGISTER_AND_MOVE PR operation to complete.
> > > */
> > > + wait_for_completion(&orig->pr_comp);
> > > + kfree_rcu(orig, rcu_head);
> >
> > The release callback should just call kfree_rcu, no need to wait for the
> > release in the caller.
> >
>
> Why doesn't se_dev_entry release this need to wait for the special case
> references to drop..?
Why would it? There is no access to the structure at this point, so there
is no point to keep it around localy. If there were other references to
it they by defintion don't need it anymore by the time they drop the
reference count. Freeing a structure as soon as the refcount drops
zero is the normal style all over the place. Waiting for a reference
count only makes sense if it's a drain style operation where you don't
free the structure but you just want to wait for some class of consumers
to stop using it.
On Fri, May 22, 2015 at 02:05:57AM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2015-05-22 at 10:26 +0200, Christoph Hellwig wrote:
> > On Fri, May 22, 2015 at 06:11:04AM +0000, Nicholas A. Bellinger wrote:
> > > + clear_bit(1, &orig->pr_reg);
> >
> > Can you call it ->flags and give the bit a meaningful name?
>
> The bit is signaling if se_dev_entry has a PR registration active.
>
> I don't see how ->flags is a more meaningful name without other bits
> defined.
It's pretty normal style: define a flags variable for any sort of
bitops state that might show up, and then give the actual bits a meaningful
name. There's almost no users of using a magic numberic value with
atomic bitops.
Besides being the usual and thus easier to read style it's also good
future proofing.
> > It would be good to just sort out the registered and co variables
> > here before the RCU changes, as in:
> >
> > http://git.infradead.org/users/hch/scsi.git/commitdiff/6372d9f62c83acb30d051387c40deb4dbdcaa376
>
> Why not just keep this patch squashed into the relevant commit in the
> context of the larger RCU conversion..?
Because the logic in and aroudn core_scsi3_pr_seq_non_holder right
now is rather confusing. So before doing changes to it it's better
to clean it up first, document that cleanup in a standalon patch
and then apply the logic change on top.
>
> -/*
> - * this function can be called with struct se_device->dev_reservation_lock
> - * when register_move = 1
> - */
> static void __core_scsi3_add_registration(
> struct se_device *dev,
> struct se_node_acl *nacl,
> @@ -1023,6 +1021,7 @@ static void __core_scsi3_add_registration(
> const struct target_core_fabric_ops *tfo = nacl->se_tpg->se_tpg_tfo;
> struct t10_pr_registration *pr_reg_tmp, *pr_reg_tmp_safe;
> struct t10_reservation *pr_tmpl = &dev->t10_pr;
> + struct se_dev_entry *deve;
>
> /*
> * Increment PRgeneration counter for struct se_device upon a successful
> @@ -1039,10 +1038,16 @@ static void __core_scsi3_add_registration(
>
> spin_lock(&pr_tmpl->registration_lock);
> list_add_tail(&pr_reg->pr_reg_list, &pr_tmpl->registration_list);
> - pr_reg->pr_reg_deve->def_pr_registered = 1;
>
> __core_scsi3_dump_registration(tfo, dev, nacl, pr_reg, register_type);
> spin_unlock(&pr_tmpl->registration_lock);
> +
> + mutex_lock(&nacl->lun_entry_mutex);
> + deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
> + if (deve)
> + set_bit(1, &deve->pr_reg);
> + mutex_unlock(&nacl->lun_entry_mutex);
Why can't we rely on pr_reg->pr_reg_deve here? The way the it's
set up is a bit convoluted, but unless I miss something it needs to
have a reference if it's set (and it it doesn't that needs to be fixed
ASAP).
Also even if we would need a target_nacl_find_deve call here it could
be done under rcu_read_lock as lun_entry_hlist isn't modified.
> + mutex_lock(&nacl->lun_entry_mutex);
> + deve = target_nacl_find_deve(nacl_tmp, pr_reg_tmp->pr_res_mapped_lun);
> + if (deve)
> + set_bit(1, &deve->pr_reg);
> + mutex_unlock(&nacl->lun_entry_mutex);
> +
Same here.
> @@ -1258,6 +1269,8 @@ static void __core_scsi3_free_registration(
> */
> if (dec_holders)
> core_scsi3_put_pr_reg(pr_reg);
> +
> + spin_unlock(&pr_tmpl->registration_lock);
> /*
> * Wait until all reference from any other I_T nexuses for this
> * *pr_reg have been released. Because list_del() is called above,
> @@ -1265,13 +1278,18 @@ static void __core_scsi3_free_registration(
> * count back to zero, and we release *pr_reg.
> */
> while (atomic_read(&pr_reg->pr_res_holders) != 0) {
> - spin_unlock(&pr_tmpl->registration_lock);
> pr_debug("SPC-3 PR [%s] waiting for pr_res_holders\n",
> tfo->get_fabric_name());
> cpu_relax();
> - spin_lock(&pr_tmpl->registration_lock);
> }
>
> + mutex_lock(&nacl->lun_entry_mutex);
> + deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
> + if (deve)
> + clear_bit(1, &deve->pr_reg);
> + mutex_unlock(&nacl->lun_entry_mutex);
And here.
On Fri, 2015-05-22 at 12:12 +0200, Bart Van Assche wrote:
> On 05/22/15 08:11, Nicholas A. Bellinger wrote:
> > diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> > index e2c0eaf..def5bc8 100644
> > --- a/include/target/target_core_base.h
> > +++ b/include/target/target_core_base.h
> > @@ -638,7 +638,6 @@ struct se_lun_acl {
> > };
> >
> > struct se_dev_entry {
> > - bool def_pr_registered;
> > /* See transport_lunflags_table */
> > u32 lun_flags;
> > u32 mapped_lun;
> > @@ -655,7 +654,8 @@ struct se_dev_entry {
> > struct se_node_acl *se_node_acl;
> > struct se_lun_acl __rcu *se_lun_acl;
> > spinlock_t ua_lock;
> > - struct se_lun *se_lun;
> > + struct se_lun __rcu *se_lun;
> > + unsigned long pr_reg;
> > struct list_head alua_port_list;
> > struct list_head ua_list;
> > struct hlist_node link;
>
> Hello Nic,
>
> This change causes the "se_lun = deve->se_lun" assignment in
> transport_lookup_cmd_lun() to assign an RCU pointer to a non-RCU
> pointer. Shouldn't such an assignment be protected via rcu_dereference() ?
>
FYI, these assignments are done in patch #1.
This __rcu notation should be included there instead.
On Fri, 2015-05-22 at 12:15 +0200, Bart Van Assche wrote:
> On 05/22/15 08:11, Nicholas A. Bellinger wrote:
> > Here is -v2 series for converting LIO target se_node_acl + se_lun
> > mapping tables from fixed size arrays to dynamic RCU hlist_heads.
>
> The full list of new sparse warnings introduced by this patch series for
> source files under drivers/target is as follows:
Yes, I'm aware this needs the associated RCU notation to make sparse
happy.
On Fri, 2015-05-22 at 13:31 +0200, Christoph Hellwig wrote:
> On Fri, May 22, 2015 at 01:55:30AM -0700, Nicholas A. Bellinger wrote:
> > > This update will now be racy, ditto for the read/write_bytes update
> > > later.
> >
> > This should become an atomic_long_t increment, yes..?
>
> Yes.
Converted.
>
> > Yes, this helper is from your patch above.
> >
> > Considering there is a single user of it here, and complexities involved
> > for a RCU conversion + bisect, is it really work adding as a separate
> > patch ahead of this one..?
>
> The golden Linus style is to put preparatory patches first so that the
> actual logic change is as small as possible. Adding helpers so that
> low level accesses that will e changed soon is a very typical case for that.
>
That would be applicable here, if the patch in question had anything to
do with the actual RCU conversion itself.
Since it doesn't, I'll keep it as a separate patch after the RCU
specific changes, along with the other improvements.
> > > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > > + wait_for_completion(&orig->pr_comp);
> > > >
> > >
> > > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > > /*
> > > > - * Disable struct se_dev_entry LUN ACL mapping
> > > > + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
> > > > + * or REGISTER_AND_MOVE PR operation to complete.
> > > > */
> > > > + wait_for_completion(&orig->pr_comp);
> > > > + kfree_rcu(orig, rcu_head);
> > >
> > > The release callback should just call kfree_rcu, no need to wait for the
> > > release in the caller.
> > >
> >
> > Why doesn't se_dev_entry release this need to wait for the special case
> > references to drop..?
>
> Why would it?
It originally had to wait in order for the se_dev_entry consumers to
drop, before it could be reused in se_node_acl->device_list[]..
> There is no access to the structure at this point, so there
> is no point to keep it around localy. If there were other references to
> it they by defintion don't need it anymore by the time they drop the
> reference count. Freeing a structure as soon as the refcount drops
> zero is the normal style all over the place. Waiting for a reference
> count only makes sense if it's a drain style operation where you don't
> free the structure but you just want to wait for some class of consumers
> to stop using it.
.. but since it's not being reused anymore, doing a kfree_rcu() from the
final kref_put() should be fine.
Converting now.
On Fri, 2015-05-22 at 13:34 +0200, Christoph Hellwig wrote:
> On Fri, May 22, 2015 at 02:05:57AM -0700, Nicholas A. Bellinger wrote:
> > On Fri, 2015-05-22 at 10:26 +0200, Christoph Hellwig wrote:
> > > On Fri, May 22, 2015 at 06:11:04AM +0000, Nicholas A. Bellinger wrote:
> > > > + clear_bit(1, &orig->pr_reg);
> > >
> > > Can you call it ->flags and give the bit a meaningful name?
> >
> > The bit is signaling if se_dev_entry has a PR registration active.
> >
> > I don't see how ->flags is a more meaningful name without other bits
> > defined.
>
> It's pretty normal style: define a flags variable for any sort of
> bitops state that might show up, and then give the actual bits a meaningful
> name. There's almost no users of using a magic numberic value with
> atomic bitops.
>
> Besides being the usual and thus easier to read style it's also good
> future proofing.
Fair enough. Changing this to ->deve_flags
>
> > > It would be good to just sort out the registered and co variables
> > > here before the RCU changes, as in:
> > >
> > > http://git.infradead.org/users/hch/scsi.git/commitdiff/6372d9f62c83acb30d051387c40deb4dbdcaa376
> >
> > Why not just keep this patch squashed into the relevant commit in the
> > context of the larger RCU conversion..?
>
> Because the logic in and aroudn core_scsi3_pr_seq_non_holder right
> now is rather confusing. So before doing changes to it it's better
> to clean it up first, document that cleanup in a standalon patch
> and then apply the logic change on top.
> --
I'll keep it in a standalone patch, but having it precede the RCU
changes when it doesn't actually involve RCU is confusing.
On Fri, 2015-05-22 at 13:52 +0200, Christoph Hellwig wrote:
> >
> > -/*
> > - * this function can be called with struct se_device->dev_reservation_lock
> > - * when register_move = 1
> > - */
> > static void __core_scsi3_add_registration(
> > struct se_device *dev,
> > struct se_node_acl *nacl,
> > @@ -1023,6 +1021,7 @@ static void __core_scsi3_add_registration(
> > const struct target_core_fabric_ops *tfo = nacl->se_tpg->se_tpg_tfo;
> > struct t10_pr_registration *pr_reg_tmp, *pr_reg_tmp_safe;
> > struct t10_reservation *pr_tmpl = &dev->t10_pr;
> > + struct se_dev_entry *deve;
> >
> > /*
> > * Increment PRgeneration counter for struct se_device upon a successful
> > @@ -1039,10 +1038,16 @@ static void __core_scsi3_add_registration(
> >
> > spin_lock(&pr_tmpl->registration_lock);
> > list_add_tail(&pr_reg->pr_reg_list, &pr_tmpl->registration_list);
> > - pr_reg->pr_reg_deve->def_pr_registered = 1;
> >
> > __core_scsi3_dump_registration(tfo, dev, nacl, pr_reg, register_type);
> > spin_unlock(&pr_tmpl->registration_lock);
> > +
> > + mutex_lock(&nacl->lun_entry_mutex);
> > + deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
> > + if (deve)
> > + set_bit(1, &deve->pr_reg);
> > + mutex_unlock(&nacl->lun_entry_mutex);
>
> Why can't we rely on pr_reg->pr_reg_deve here? The way the it's
> set up is a bit convoluted, but unless I miss something it needs to
> have a reference if it's set (and it it doesn't that needs to be fixed
> ASAP).
>
So pr_reg->pr_reg_deve is only referenced in the context of ALL_TG_PT=1,
I_PORT=1 and REGISTER_AND_MOVE registration, when pr_reg->pr_kref is
already held.
>
> Also even if we would need a target_nacl_find_deve call here it could
> be done under rcu_read_lock as lun_entry_hlist isn't modified.
>
Probably not. Changing these to rcu_read_lock.
> > + mutex_lock(&nacl->lun_entry_mutex);
> > + deve = target_nacl_find_deve(nacl_tmp, pr_reg_tmp->pr_res_mapped_lun);
> > + if (deve)
> > + set_bit(1, &deve->pr_reg);
> > + mutex_unlock(&nacl->lun_entry_mutex);
> > +
>
It should be fine to dereference pr_reg_tmp->pr_reg_deve directly here.
>
> > @@ -1258,6 +1269,8 @@ static void __core_scsi3_free_registration(
> > */
> > if (dec_holders)
> > core_scsi3_put_pr_reg(pr_reg);
> > +
> > + spin_unlock(&pr_tmpl->registration_lock);
> > /*
> > * Wait until all reference from any other I_T nexuses for this
> > * *pr_reg have been released. Because list_del() is called above,
> > @@ -1265,13 +1278,18 @@ static void __core_scsi3_free_registration(
> > * count back to zero, and we release *pr_reg.
> > */
> > while (atomic_read(&pr_reg->pr_res_holders) != 0) {
> > - spin_unlock(&pr_tmpl->registration_lock);
> > pr_debug("SPC-3 PR [%s] waiting for pr_res_holders\n",
> > tfo->get_fabric_name());
> > cpu_relax();
> > - spin_lock(&pr_tmpl->registration_lock);
> > }
> >
> > + mutex_lock(&nacl->lun_entry_mutex);
> > + deve = target_nacl_find_deve(nacl, pr_reg->pr_res_mapped_lun);
> > + if (deve)
> > + clear_bit(1, &deve->pr_reg);
> > + mutex_unlock(&nacl->lun_entry_mutex);
>
> And here.
This is not going to work, because __core_scsi3_free_registration() can
be invoked via core_disable_device_list_for_node() ->
core_scsi3_free_pr_reg_from_nacl(), after the kfree_rcu() for
se_dev_entry has occurred.
So keeping the extra lookup for this one particular case, and dropping
the rest.
On Mon, 2015-05-25 at 15:14 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2015-05-22 at 13:31 +0200, Christoph Hellwig wrote:
> > On Fri, May 22, 2015 at 01:55:30AM -0700, Nicholas A. Bellinger wrote:
<SNIP>
> > > > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > > > + wait_for_completion(&orig->pr_comp);
> > > > >
> > > >
> > > > > + kref_put(&orig->pr_kref, target_pr_kref_release);
> > > > > /*
> > > > > - * Disable struct se_dev_entry LUN ACL mapping
> > > > > + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1
> > > > > + * or REGISTER_AND_MOVE PR operation to complete.
> > > > > */
> > > > > + wait_for_completion(&orig->pr_comp);
> > > > > + kfree_rcu(orig, rcu_head);
> > > >
> > > > The release callback should just call kfree_rcu, no need to wait for the
> > > > release in the caller.
> > > >
> > >
> > > Why doesn't se_dev_entry release this need to wait for the special case
> > > references to drop..?
> >
> > Why would it?
>
> It originally had to wait in order for the se_dev_entry consumers to
> drop, before it could be reused in se_node_acl->device_list[]..
>
> > There is no access to the structure at this point, so there
> > is no point to keep it around localy. If there were other references to
> > it they by defintion don't need it anymore by the time they drop the
> > reference count. Freeing a structure as soon as the refcount drops
> > zero is the normal style all over the place. Waiting for a reference
> > count only makes sense if it's a drain style operation where you don't
> > free the structure but you just want to wait for some class of consumers
> > to stop using it.
>
> .. but since it's not being reused anymore, doing a kfree_rcu() from the
> final kref_put() should be fine.
>
> Converting now.
Looking at this more while adding rcu notation, just calling kfree_rcu()
from kref_put() is not going to work here.
The wait_for_completion(&orig->pr_comp) above is still required because
core_scsi3_lunacl_depend_item() has to dereference se_deve->se_lun_acl
outside of the lock (with se_deve->pr_kref held) to obtain the configfs
dependency for se_lun_acl->se_lun_group.
Otherwise, se_lun_acl->se_lun_group can be removed from out below the
core_scsi3_lunacl_depend_item() caller, if se_deve->pr_comp doesn't wait
for the configfs dependency to be released.
--nab