On Fri, 2013-12-13 at 15:59 -0800, Andy Grover wrote:
> Instead of an array, use a rbtree. Less memory use on average, and
> can allow >255 entries. We go from O(1) to O(log n) on lookups. If this
> shows up on profiling (it won't) then transition to other kernel lookup
> methods is straightforward from here.
>
Ugh.
There is no reason to be using rbtrees in a performance critical path
here.
The number of pointer lookups is what ends up hurting the most vs. a
flat array, so given that 256 LUNs per endpoint is currently not an
issue, and there is no hard limit on the number of endpoints with
virtual addressing, I don't see the benefit of this patch.
NAK.
--nab
> Change core_disable_device_list_for_node to be called with device_list_lock
> held, and tweaked params a little.
>
> TRANSPORT_LUNFLAGS_INITIATOR_ACCESS no longer needed, presence in the
> rbtree is equivalent to this being set.
>
> Signed-off-by: Andy Grover
> ---
> drivers/target/target_core_device.c | 213 ++---
> drivers/target/target_core_fabric_configfs.c | 24 ++--
> drivers/target/target_core_internal.h| 15 +-
> drivers/target/target_core_pr.c | 25 +++-
> drivers/target/target_core_spc.c | 11 +-
> drivers/target/target_core_stat.c| 72 +-
> drivers/target/target_core_tpg.c | 53 +--
> drivers/target/target_core_ua.c | 18 ++-
> include/target/target_core_base.h|8 +-
> 9 files changed, 222 insertions(+), 217 deletions(-)
>
> diff --git a/drivers/target/target_core_device.c
> b/drivers/target/target_core_device.c
> index af12456..a18724b 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -54,6 +54,51 @@ static struct se_hba *lun0_hba;
> /* not static, needed by tpg.c */
> struct se_device *g_lun0_dev;
>
> +bool transport_insert_deve(struct se_node_acl *nacl, struct se_dev_entry
> *deve)
> +{
> + struct rb_root *root = &nacl->rb_device_list;
> + struct rb_node **new = &(root->rb_node), *parent = NULL;
> +
> + /* Figure out where to put new node */
> + while (*new) {
> + struct se_dev_entry *this = rb_entry(*new, struct se_dev_entry,
> rb_node);
> +
> + parent = *new;
> + if (deve->mapped_lun < this->mapped_lun)
> + new = &((*new)->rb_left);
> + else if (deve->mapped_lun > this->mapped_lun)
> + new = &((*new)->rb_right);
> + else
> + return false;
> + }
> +
> + /* Add new node and rebalance tree. */
> + rb_link_node(&deve->rb_node, parent, new);
> + rb_insert_color(&deve->rb_node, root);
> +
> + return true;
> +}
> +
> +
> +struct se_dev_entry *transport_search_deve(struct se_node_acl *nacl, u32
> mapped_lun)
> +{
> + struct rb_root *root = &nacl->rb_device_list;
> + struct rb_node *node = root->rb_node;
> +
> + while (node) {
> + struct se_dev_entry *deve = rb_entry(node, struct se_dev_entry,
> rb_node);
> +
> + if (mapped_lun < deve->mapped_lun)
> + node = node->rb_left;
> + else if (mapped_lun > deve->mapped_lun)
> + node = node->rb_right;
> + else
> + return deve;
> + }
> + return NULL;
> +}
> +
> +
> sense_reason_t
> transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
> {
> @@ -66,8 +111,8 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32
> unpacked_lun)
> 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) {
> + se_cmd->se_deve = transport_search_deve(se_sess->se_node_acl,
> unpacked_lun);
> + if (se_cmd->se_deve) {
> struct se_dev_entry *deve = se_cmd->se_deve;
>
> deve->total_cmds++;
> @@ -153,10 +198,10 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32
> unpacked_lun)
> 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;
> + se_cmd->se_deve = transport_search_deve(se_sess->se_node_acl,
> unpacked_lun);
> + if (se_cmd->se_deve) {
> + deve = se_cmd->se_deve;
>
> - if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
> se_tmr->tmr_lun = deve->se_lun;
> se_cmd->se_lun = deve->se_lun;
> se_lun = deve->se_lun;
> @@ -187,25 +232,22 @@ EXPORT_SYMBOL(transport_lookup_tmr_lun);
>
> /*
> * This function is called from core_scsi3_emulate_pro_register_and_move()
> - * and core_scsi3_decode_spec_i_port(), and will increment
> &de
