scsi: target: Make state_list per CPU

Do a state_list/execute_task_lock per CPU, so we can do submissions from
different CPUs without contention with each other.

Note: tcm_fc was passing TARGET_SCF_USE_CPUID, but never set cpuid.  The
assumption is that it wanted to set the cpuid to the CPU it was submitting
from so it will get this behavior with this patch.

[mkp: s/printk/pr_err/ + resolve COMPARE AND WRITE patch conflict]

Link: https://lore.kernel.org/r/1604257174-4524-8-git-send-email-michael.christie@oracle.com
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 1f673fb..7787c52 100644 (file)
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -721,11 +721,24 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 {
        struct se_device *dev;
        struct se_lun *xcopy_lun;
+       int i;
 
        dev = hba->backend->ops->alloc_device(hba, name);
        if (!dev)
                return NULL;
 
+       dev->queues = kcalloc(nr_cpu_ids, sizeof(*dev->queues), GFP_KERNEL);
+       if (!dev->queues) {
+               dev->transport->free_device(dev);
+               return NULL;
+       }
+
+       dev->queue_cnt = nr_cpu_ids;
+       for (i = 0; i < dev->queue_cnt; i++) {
+               INIT_LIST_HEAD(&dev->queues[i].state_list);
+               spin_lock_init(&dev->queues[i].lock);
+       }
+
        dev->se_hba = hba;
        dev->transport = hba->backend->ops;
        dev->transport_flags = dev->transport->transport_flags_default;
@@ -735,9 +748,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
        INIT_LIST_HEAD(&dev->dev_sep_list);
        INIT_LIST_HEAD(&dev->dev_tmr_list);
        INIT_LIST_HEAD(&dev->delayed_cmd_list);
-       INIT_LIST_HEAD(&dev->state_list);
        INIT_LIST_HEAD(&dev->qf_cmd_list);
-       spin_lock_init(&dev->execute_task_lock);
        spin_lock_init(&dev->delayed_cmd_lock);
        spin_lock_init(&dev->dev_reservation_lock);
        spin_lock_init(&dev->se_port_lock);
@@ -1010,6 +1021,7 @@ void target_free_device(struct se_device *dev)
        if (dev->transport->free_prot)
                dev->transport->free_prot(dev);
 
+       kfree(dev->queues);
        dev->transport->free_device(dev);
 }
 

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index e4513ef..7347285 100644 (file)
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -121,57 +121,61 @@ void core_tmr_abort_task(
        unsigned long flags;
        bool rc;
        u64 ref_tag;
-
-       spin_lock_irqsave(&dev->execute_task_lock, flags);
-       list_for_each_entry_safe(se_cmd, next, &dev->state_list, state_list) {
-
-               if (se_sess != se_cmd->se_sess)
-                       continue;
-
-               /* skip task management functions, including tmr->task_cmd */
-               if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
-                       continue;
-
-               ref_tag = se_cmd->tag;
-               if (tmr->ref_task_tag != ref_tag)
-                       continue;
-
-               printk("ABORT_TASK: Found referenced %s task_tag: %llu\n",
-                       se_cmd->se_tfo->fabric_name, ref_tag);
-
-               spin_lock(&se_sess->sess_cmd_lock);
-               rc = __target_check_io_state(se_cmd, se_sess, 0);
-               spin_unlock(&se_sess->sess_cmd_lock);
-               if (!rc)
-                       continue;
-
-               list_move_tail(&se_cmd->state_list, &aborted_list);
-               se_cmd->state_active = false;
-
-               spin_unlock_irqrestore(&dev->execute_task_lock, flags);
-
-               /*
-                * Ensure that this ABORT request is visible to the LU RESET
-                * code.
-                */
-               if (!tmr->tmr_dev)
-                       WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) <
-                                       0);
-
-               if (dev->transport->tmr_notify)
-                       dev->transport->tmr_notify(dev, TMR_ABORT_TASK,
-                                                  &aborted_list);
-
-               list_del_init(&se_cmd->state_list);
-               target_put_cmd_and_wait(se_cmd);
-
-               printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for"
-                               " ref_tag: %llu\n", ref_tag);
-               tmr->response = TMR_FUNCTION_COMPLETE;
-               atomic_long_inc(&dev->aborts_complete);
-               return;
+       int i;
+
+       for (i = 0; i < dev->queue_cnt; i++) {
+               spin_lock_irqsave(&dev->queues[i].lock, flags);
+               list_for_each_entry_safe(se_cmd, next, &dev->queues[i].state_list,
+                                        state_list) {
+                       if (se_sess != se_cmd->se_sess)
+                               continue;
+
+                       /*
+                        * skip task management functions, including
+                        * tmr->task_cmd
+                        */
+                       if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
+                               continue;
+
+                       ref_tag = se_cmd->tag;
+                       if (tmr->ref_task_tag != ref_tag)
+                               continue;
+
+                       pr_err("ABORT_TASK: Found referenced %s task_tag: %llu\n",
+                              se_cmd->se_tfo->fabric_name, ref_tag);
+
+                       spin_lock(&se_sess->sess_cmd_lock);
+                       rc = __target_check_io_state(se_cmd, se_sess, 0);
+                       spin_unlock(&se_sess->sess_cmd_lock);
+                       if (!rc)
+                               continue;
+
+                       list_move_tail(&se_cmd->state_list, &aborted_list);
+                       se_cmd->state_active = false;
+                       spin_unlock_irqrestore(&dev->queues[i].lock, flags);
+
+                       /*
+                        * Ensure that this ABORT request is visible to the LU
+                        * RESET code.
+                        */
+                       if (!tmr->tmr_dev)
+                               WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < 0);
+
+                       if (dev->transport->tmr_notify)
+                               dev->transport->tmr_notify(dev, TMR_ABORT_TASK,
+                                                          &aborted_list);
+
+                       list_del_init(&se_cmd->state_list);
+                       target_put_cmd_and_wait(se_cmd);
+
+                       pr_err("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %llu\n",
+                              ref_tag);
+                       tmr->response = TMR_FUNCTION_COMPLETE;
+                       atomic_long_inc(&dev->aborts_complete);
+                       return;
+               }
+               spin_unlock_irqrestore(&dev->queues[i].lock, flags);
        }
-       spin_unlock_irqrestore(&dev->execute_task_lock, flags);
 
        if (dev->transport->tmr_notify)
                dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list);
@@ -273,7 +277,7 @@ static void core_tmr_drain_state_list(
        struct se_session *sess;
        struct se_cmd *cmd, *next;
        unsigned long flags;
-       int rc;
+       int rc, i;
 
        /*
         * Complete outstanding commands with TASK_ABORTED SAM status.
@@ -297,35 +301,39 @@ static void core_tmr_drain_state_list(
         * Note that this seems to be independent of TAS (Task Aborted Status)
         * in the Control Mode Page.
         */
-       spin_lock_irqsave(&dev->execute_task_lock, flags);
-       list_for_each_entry_safe(cmd, next, &dev->state_list, state_list) {
-               /*
-                * For PREEMPT_AND_ABORT usage, only process commands
-                * with a matching reservation key.
-                */
-               if (target_check_cdb_and_preempt(preempt_and_abort_list, cmd))
-                       continue;
-
-               /*
-                * Not aborting PROUT PREEMPT_AND_ABORT CDB..
-                */
-               if (prout_cmd == cmd)
-                       continue;
-
-               sess = cmd->se_sess;
-               if (WARN_ON_ONCE(!sess))
-                       continue;
-
-               spin_lock(&sess->sess_cmd_lock);
-               rc = __target_check_io_state(cmd, tmr_sess, tas);
-               spin_unlock(&sess->sess_cmd_lock);
-               if (!rc)
-                       continue;
-
-               list_move_tail(&cmd->state_list, &drain_task_list);
-               cmd->state_active = false;
+       for (i = 0; i < dev->queue_cnt; i++) {
+               spin_lock_irqsave(&dev->queues[i].lock, flags);
+               list_for_each_entry_safe(cmd, next, &dev->queues[i].state_list,
+                                        state_list) {
+                       /*
+                        * For PREEMPT_AND_ABORT usage, only process commands
+                        * with a matching reservation key.
+                        */
+                       if (target_check_cdb_and_preempt(preempt_and_abort_list,
+                                                        cmd))
+                               continue;
+
+                       /*
+                        * Not aborting PROUT PREEMPT_AND_ABORT CDB..
+                        */
+                       if (prout_cmd == cmd)
+                               continue;
+
+                       sess = cmd->se_sess;
+                       if (WARN_ON_ONCE(!sess))
+                               continue;
+
+                       spin_lock(&sess->sess_cmd_lock);
+                       rc = __target_check_io_state(cmd, tmr_sess, tas);
+                       spin_unlock(&sess->sess_cmd_lock);
+                       if (!rc)
+                               continue;
+
+                       list_move_tail(&cmd->state_list, &drain_task_list);
+                       cmd->state_active = false;
+               }
+               spin_unlock_irqrestore(&dev->queues[i].lock, flags);
        }
-       spin_unlock_irqrestore(&dev->execute_task_lock, flags);
 
        if (dev->transport->tmr_notify)
                dev->transport->tmr_notify(dev, preempt_and_abort_list ?

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 485317c..fca4bd0 100644 (file)
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -659,12 +659,12 @@ static void target_remove_from_state_list(struct se_cmd *cmd)
        if (!dev)
                return;
 
-       spin_lock_irqsave(&dev->execute_task_lock, flags);
+       spin_lock_irqsave(&dev->queues[cmd->cpuid].lock, flags);
        if (cmd->state_active) {
                list_del(&cmd->state_list);
                cmd->state_active = false;
        }
-       spin_unlock_irqrestore(&dev->execute_task_lock, flags);
+       spin_unlock_irqrestore(&dev->queues[cmd->cpuid].lock, flags);
 }
 
 /*
@@ -875,10 +875,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
 
        INIT_WORK(&cmd->work, success ? target_complete_ok_work :
                  target_complete_failure_work);
-       if (cmd->se_cmd_flags & SCF_USE_CPUID)
-               queue_work_on(cmd->cpuid, target_completion_wq, &cmd->work);
-       else
-               queue_work(target_completion_wq, &cmd->work);
+       queue_work_on(cmd->cpuid, target_completion_wq, &cmd->work);
 }
 EXPORT_SYMBOL(target_complete_cmd);
 
@@ -906,12 +903,13 @@ static void target_add_to_state_list(struct se_cmd *cmd)
        struct se_device *dev = cmd->se_dev;
        unsigned long flags;
 
-       spin_lock_irqsave(&dev->execute_task_lock, flags);
+       spin_lock_irqsave(&dev->queues[cmd->cpuid].lock, flags);
        if (!cmd->state_active) {
-               list_add_tail(&cmd->state_list, &dev->state_list);
+               list_add_tail(&cmd->state_list,
+                             &dev->queues[cmd->cpuid].state_list);
                cmd->state_active = true;
        }
-       spin_unlock_irqrestore(&dev->execute_task_lock, flags);
+       spin_unlock_irqrestore(&dev->queues[cmd->cpuid].lock, flags);
 }
 
 /*
@@ -1399,6 +1397,9 @@ void transport_init_se_cmd(
        cmd->sense_buffer = sense_buffer;
        cmd->orig_fe_lun = unpacked_lun;
 
+       if (!(cmd->se_cmd_flags & SCF_USE_CPUID))
+               cmd->cpuid = smp_processor_id();
+
        cmd->state_active = false;
 }
 EXPORT_SYMBOL(transport_init_se_cmd);
@@ -1616,6 +1617,9 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
        BUG_ON(!se_tpg);
        BUG_ON(se_cmd->se_tfo || se_cmd->se_sess);
        BUG_ON(in_interrupt());
+
+       if (flags & TARGET_SCF_USE_CPUID)
+               se_cmd->se_cmd_flags |= SCF_USE_CPUID;
        /*
         * Initialize se_cmd for target operation.  From this point
         * exceptions are handled by sending exception status via
@@ -1625,11 +1629,6 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
                                data_length, data_dir, task_attr, sense,
                                unpacked_lun);
 
-       if (flags & TARGET_SCF_USE_CPUID)
-               se_cmd->se_cmd_flags |= SCF_USE_CPUID;
-       else
-               se_cmd->cpuid = WORK_CPU_UNBOUND;
-
        if (flags & TARGET_SCF_UNKNOWN_SIZE)
                se_cmd->unknown_data_length = 1;
        /*

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 9c8c38a..768f250 100644 (file)
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -550,7 +550,7 @@ static void ft_send_work(struct work_struct *work)
        if (target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, fcp->fc_cdb,
                              &cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun),
                              ntohl(fcp->fc_dl), task_attr, data_dir,
-                             TARGET_SCF_ACK_KREF | TARGET_SCF_USE_CPUID))
+                             TARGET_SCF_ACK_KREF))
                goto err;
 
        pr_debug("r_ctl %x target_submit_cmd %p\n", fh->fh_r_ctl, cmd);

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index c858dc0..63dd121 100644 (file)
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -540,6 +540,10 @@ struct se_cmd {
        unsigned int            t_prot_nents;
        sense_reason_t          pi_err;
        u64                     sense_info;
+       /*
+        * CPU LIO will execute the cmd on. Defaults to the CPU the cmd is
+        * initialized on. Drivers can override.
+        */
        int                     cpuid;
 };
 
@@ -760,6 +764,11 @@ struct se_dev_stat_grps {
        struct config_group scsi_lu_group;
 };
 
+struct se_device_queue {
+       struct list_head        state_list;
+       spinlock_t              lock;
+};
+
 struct se_device {
        /* RELATIVE TARGET PORT IDENTIFER Counter */
        u16                     dev_rpti_counter;
@@ -792,7 +801,6 @@ struct se_device {
        atomic_t                dev_qf_count;
        u32                     export_count;
        spinlock_t              delayed_cmd_lock;
-       spinlock_t              execute_task_lock;
        spinlock_t              dev_reservation_lock;
        unsigned int            dev_reservation_flags;
 #define DRF_SPC2_RESERVATIONS                  0x00000001
@@ -811,7 +819,6 @@ struct se_device {
        struct list_head        dev_tmr_list;
        struct work_struct      qf_work_queue;
        struct list_head        delayed_cmd_list;
-       struct list_head        state_list;
        struct list_head        qf_cmd_list;
        /* Pointer to associated SE HBA */
        struct se_hba           *se_hba;
@@ -838,6 +845,8 @@ struct se_device {
        /* For se_lun->lun_se_dev RCU read-side critical access */
        u32                     hba_index;
        struct rcu_head         rcu_head;
+       int                     queue_cnt;
+       struct se_device_queue  *queues;
 };
 
 struct se_hba {