- Makes sessions registration/unregistration independant from other activities
authorvlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 14 Dec 2007 16:51:36 +0000 (16:51 +0000)
committervlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 14 Dec 2007 16:51:36 +0000 (16:51 +0000)
 - Cleanups
 - Docs update

git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@236 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/README
scst/include/scsi_tgt.h
scst/src/scst_lib.c
scst/src/scst_main.c
scst/src/scst_priv.h
scst/src/scst_targ.c

index 5909449..7f1b3d8 100644 (file)
@@ -676,15 +676,18 @@ debug2perf Makefile target.
 
  - Make sure that your target hardware (e.g. target FC or network card)
    and underlaying IO hardware (e.g. IO card, like SATA, SCSI or RAID to
-   which your disks connected) don't share the same PCI bus. They have
-   to work in parallel, so it will be better if they don't compete for
-   the bus. The problem is not only in the bandwidth, which they have to
-   share, but also in the interaction between cards during that
-   competition. This is very important, because in some cases if target
-   and backend storage controllers share the same PCI bus, it could lead
-   up to 5-10 times less performance, than expected. If you have no
-   choice, but PCI bus sharing, set in the BIOS PCI latency as low as
-   possible.
+   which your disks connected) don't share the same PCI bus. You can
+   check it using lspci utility. They have to work in parallel, so it
+   will be better if they don't compete for the bus. The problem is not
+   only in the bandwidth, which they have to share, but also in the
+   interaction between cards during that competition. This is very
+   important, because in some cases if target and backend storage
+   controllers share the same PCI bus, it could lead up to 5-10 times
+   less performance, than expected. Moreover, some motherboard (by
+   Supermicro, particularly) have serious stability issues if there are
+   several high speed devices on the same bus working in parallel. If
+   you have no choice, but PCI bus sharing, set in the BIOS PCI latency
+   as low as possible.
 
 IMPORTANT: If you use on initiator some versions of Windows (at least W2K)
 =========  you can't get good write performance for VDISK FILEIO devices with
index cd32e74..4f25a7e 100644 (file)
@@ -1355,8 +1355,8 @@ struct scst_device
        struct list_head dev_list_entry;
        
        /*
-        * List of tgt_dev's, one per session, protected by scst_mutex and
-        * suspended activity
+        * List of tgt_dev's, one per session, protected by scst_mutex or
+        * dev_lock for reads and both for writes
         */
        struct list_head dev_tgt_dev_list;
        
index 7bc2604..7a96da6 100644 (file)
@@ -206,8 +206,7 @@ void scst_free_device(struct scst_device *dev)
 
 #ifdef EXTRACHECKS
        if (!list_empty(&dev->dev_tgt_dev_list) || 
-           !list_empty(&dev->dev_acg_dev_list))
-       {
+           !list_empty(&dev->dev_acg_dev_list)) {
                PRINT_ERROR("%s: dev_tgt_dev_list or dev_acg_dev_list "
                        "is not empty!", __FUNCTION__);
                sBUG();
@@ -311,8 +310,7 @@ int scst_destroy_acg(struct scst_acg *acg)
        
        /* Freeing acg_devs */
        list_for_each_entry_safe(acg_dev, acg_dev_tmp, &acg->acg_dev_list, 
-               acg_dev_list_entry)
-       {
+                       acg_dev_list_entry) {
                struct scst_tgt_dev *tgt_dev, *tt;
                list_for_each_entry_safe(tgt_dev, tt,
                                 &acg_dev->dev->dev_tgt_dev_list,
@@ -325,8 +323,7 @@ int scst_destroy_acg(struct scst_acg *acg)
 
        /* Freeing names */
        list_for_each_entry_safe(n, nn, &acg->acn_list, 
-               acn_list_entry)
-       {
+                       acn_list_entry) {
                list_del(&n->acn_list_entry);
                kfree(n->name);
                kfree(n);
@@ -339,10 +336,7 @@ out:
        return res;
 }
 
-/*
- * No spin locks supposed to be held, scst_mutex - held.
- * The activity is suspended.
- */
+/* scst_mutex supposed to be held, there must not be parallel activity in this sess */
 static struct scst_tgt_dev *scst_alloc_add_tgt_dev(struct scst_session *sess,
        struct scst_acg_dev *acg_dev)
 {
@@ -479,10 +473,12 @@ static struct scst_tgt_dev *scst_alloc_add_tgt_dev(struct scst_session *sess,
                        goto out_thr_free;
                }
        }
-       
+
+       spin_lock_bh(&dev->dev_lock);   
        list_add_tail(&tgt_dev->dev_tgt_dev_list_entry, &dev->dev_tgt_dev_list);
        if (dev->dev_reserved)
                __set_bit(SCST_TGT_DEV_RESERVED, &tgt_dev->tgt_dev_flags);
+       spin_unlock_bh(&dev->dev_lock);
 
        sess_tgt_dev_list_head = 
                &sess->sess_tgt_dev_list_hash[HASH_VAL(tgt_dev->lun)];
@@ -508,10 +504,7 @@ out_free:
 
 static void scst_clear_reservation(struct scst_tgt_dev *tgt_dev);
 
-/* 
- * No locks supposed to be held, scst_mutex - held.
- * The activity is suspended.
- */
+/* No locks supposed to be held, scst_mutex - held */
 void scst_nexus_loss(struct scst_tgt_dev *tgt_dev)
 {
        TRACE_ENTRY();
@@ -533,10 +526,7 @@ void scst_nexus_loss(struct scst_tgt_dev *tgt_dev)
        return;
 }
 
-/* 
- * No locks supposed to be held, scst_mutex - held.
- * The activity is suspended.
- */
+/* scst_mutex supposed to be held, there must not be parallel activity in this sess */
 static void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev)
 {
        struct scst_device *dev = tgt_dev->dev;
@@ -546,7 +536,10 @@ static void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev)
 
        tm_dbg_deinit_tgt_dev(tgt_dev);
 
+       spin_lock_bh(&dev->dev_lock);
        list_del(&tgt_dev->dev_tgt_dev_list_entry);
+       spin_unlock_bh(&dev->dev_lock);
+
        list_del(&tgt_dev->sess_tgt_dev_list_entry);
 
        scst_clear_reservation(tgt_dev);
@@ -572,7 +565,7 @@ static void scst_free_tgt_dev(struct scst_tgt_dev *tgt_dev)
        return;
 }
 
-/* The activity supposed to be suspended and scst_mutex held */
+/* scst_mutex supposed to be held */
 int scst_sess_alloc_tgt_devs(struct scst_session *sess)
 {
        int res = 0;
@@ -582,8 +575,7 @@ int scst_sess_alloc_tgt_devs(struct scst_session *sess)
        TRACE_ENTRY();
 
        list_for_each_entry(acg_dev, &sess->acg->acg_dev_list, 
-               acg_dev_list_entry)
-       {
+                       acg_dev_list_entry) {
                tgt_dev = scst_alloc_add_tgt_dev(sess, acg_dev);
                if (tgt_dev == NULL) {
                        res = -ENOMEM;
@@ -600,7 +592,7 @@ out_free:
        goto out;
 }
 
-/* scst_mutex supposed to be held and activity suspended */
+/* scst_mutex supposed to be held, there must not be parallel activity in this sess */
 void scst_sess_free_tgt_devs(struct scst_session *sess)
 {
        int i;
@@ -1062,15 +1054,17 @@ out:
 }
 #endif /* LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18) */
 
+/* scst_mutex supposed to be held */
 static void scst_clear_reservation(struct scst_tgt_dev *tgt_dev)
 {
        struct scst_device *dev = tgt_dev->dev;
+       int release = 0;
 
        TRACE_ENTRY();
 
+       spin_lock_bh(&dev->dev_lock);
        if (dev->dev_reserved &&
-           !test_bit(SCST_TGT_DEV_RESERVED, &tgt_dev->tgt_dev_flags)) 
-       {
+           !test_bit(SCST_TGT_DEV_RESERVED, &tgt_dev->tgt_dev_flags)) {
                /* This is one who holds the reservation */
                struct scst_tgt_dev *tgt_dev_tmp;
                list_for_each_entry(tgt_dev_tmp, &dev->dev_tgt_dev_list,
@@ -1079,9 +1073,11 @@ static void scst_clear_reservation(struct scst_tgt_dev *tgt_dev)
                                    &tgt_dev_tmp->tgt_dev_flags);
                }
                dev->dev_reserved = 0;
+       }
+       spin_unlock_bh(&dev->dev_lock);
 
+       if (release)
                scst_send_release(tgt_dev);
-       }
 
        TRACE_EXIT();
        return;
@@ -1149,7 +1145,6 @@ void scst_free_session(struct scst_session *sess)
 {
        TRACE_ENTRY();
 
-       scst_suspend_activity();
        mutex_lock(&scst_mutex);
 
        TRACE_DBG("Removing sess %p from the list", sess);
@@ -1162,7 +1157,6 @@ void scst_free_session(struct scst_session *sess)
        wake_up_all(&sess->tgt->unreg_waitQ);
 
        mutex_unlock(&scst_mutex);
-       scst_resume_activity();
 
        kfree(sess->initiator_name);
        kmem_cache_free(scst_sess_cachep, sess);
@@ -2332,7 +2326,6 @@ void scst_process_reset(struct scst_device *dev,
 
        /* Clear RESERVE'ation, if necessary */
        if (dev->dev_reserved) {
-               /* Either scst_mutex held or exclude_cmd non-NULL */
                list_for_each_entry(tgt_dev, &dev->dev_tgt_dev_list,
                                    dev_tgt_dev_list_entry) {
                        TRACE(TRACE_MGMT, "Clearing RESERVE'ation for tgt_dev "
@@ -2452,7 +2445,7 @@ out_unlock:
        goto out;
 }
 
-/* Called under dev_lock, tgt_dev_lock and BH off */
+/* Called under tgt_dev_lock and BH off */
 void scst_alloc_set_UA(struct scst_tgt_dev *tgt_dev,
        const uint8_t *sense, int sense_len, int head)
 {
@@ -2515,7 +2508,7 @@ void scst_check_set_UA(struct scst_tgt_dev *tgt_dev,
        return;
 }
 
-/* No locks, but the activity must not get suspended while inside this function */
+/* Called under dev_lock and BH off */
 void scst_dev_check_set_local_UA(struct scst_device *dev,
        struct scst_cmd *exclude, const uint8_t *sense, int sense_len)
 {
@@ -2676,11 +2669,6 @@ void scst_dev_del_all_thr_data(struct scst_device *dev)
 
        TRACE_ENTRY();
 
-       /* 
-        * This is read-only function for dev->dev_tgt_dev_list, so
-        * suspending the activity isn't necessary.
-        */
-
        mutex_lock(&scst_mutex);
 
        list_for_each_entry(tgt_dev, &dev->dev_tgt_dev_list,
index 0008272..6248d87 100644 (file)
@@ -920,7 +920,7 @@ void scst_unregister_virtual_dev_driver(struct scst_dev_type *dev_type)
        return;
 }
 
-/* Called under scst_mutex and suspended activity */
+/* Called under scst_mutex */
 int scst_add_dev_threads(struct scst_device *dev, int num)
 {
        int i, res = 0;
@@ -992,7 +992,7 @@ out:
        return res;
 }
 
-/* Called under scst_mutex and suspended activity */
+/* Called under scst_mutex */
 void scst_del_dev_threads(struct scst_device *dev, int num)
 {
        struct scst_cmd_thread_t *ct, *tmp;
index 061a4f1..1af0296 100644 (file)
@@ -366,6 +366,7 @@ static inline void scst_dev_check_set_UA(struct scst_device *dev,
 }
 void scst_dev_check_set_local_UA(struct scst_device *dev,
        struct scst_cmd *exclude, const uint8_t *sense, int sense_len);
+
 void scst_check_set_UA(struct scst_tgt_dev *tgt_dev,
        const uint8_t *sense, int sense_len, int head);
 void scst_alloc_set_UA(struct scst_tgt_dev *tgt_dev, const uint8_t *sense,
index bbdcb38..ed3538c 100644 (file)
@@ -1368,8 +1368,7 @@ static int scst_reserve_local(struct scst_cmd *cmd)
        }
 
        list_for_each_entry(tgt_dev_tmp, &dev->dev_tgt_dev_list,
-                           dev_tgt_dev_list_entry) 
-       {
+                           dev_tgt_dev_list_entry) {
                if (cmd->tgt_dev != tgt_dev_tmp)
                        set_bit(SCST_TGT_DEV_RESERVED, 
                                &tgt_dev_tmp->tgt_dev_flags);
@@ -2140,17 +2139,22 @@ static int scst_done_cmd_check(struct scst_cmd *cmd, int *pres)
                        if (!test_bit(SCST_TGT_DEV_RESERVED,
                                        &cmd->tgt_dev->tgt_dev_flags)) {
                                struct scst_tgt_dev *tgt_dev_tmp;
+                               struct scst_device *dev = cmd->dev;
+
                                TRACE(TRACE_SCSI, "Real RESERVE failed lun=%Ld, status=%x",
                                      (uint64_t)cmd->lun, cmd->status);
                                TRACE_BUFF_FLAG(TRACE_SCSI, "Sense", cmd->sense_buffer,
                                             sizeof(cmd->sense_buffer));
+
                                /* Clearing the reservation */
-                               list_for_each_entry(tgt_dev_tmp, &cmd->dev->dev_tgt_dev_list,
+                               spin_lock_bh(&dev->dev_lock);
+                               list_for_each_entry(tgt_dev_tmp, &dev->dev_tgt_dev_list,
                                                    dev_tgt_dev_list_entry) {
                                        clear_bit(SCST_TGT_DEV_RESERVED, 
                                                &tgt_dev_tmp->tgt_dev_flags);
                                }
-                               cmd->dev->dev_reserved = 0;
+                               dev->dev_reserved = 0;
+                               spin_unlock_bh(&dev->dev_lock);
                        }
                }
 
@@ -2203,7 +2207,8 @@ static int scst_mode_select_checks(struct scst_cmd *cmd)
                if (unlikely((cmd->cdb[0] == MODE_SELECT) || 
                    (cmd->cdb[0] == MODE_SELECT_10) ||
                    (cmd->cdb[0] == LOG_SELECT))) {
-                       if (atomic && (cmd->dev->scsi_dev != NULL)) {
+                       struct scst_device *dev = cmd->dev;
+                       if (atomic && (dev->scsi_dev != NULL)) {
                                TRACE_DBG("%s", "MODE/LOG SELECT: thread "
                                        "context required");
                                res = SCST_CMD_STATE_RES_NEED_THREAD;
@@ -2214,7 +2219,8 @@ static int scst_mode_select_checks(struct scst_cmd *cmd)
                                "setting the SELECT UA (lun=%Ld)", 
                                (uint64_t)cmd->lun);
 
-                       spin_lock_bh(&scst_temp_UA_lock);
+                       spin_lock_bh(&dev->dev_lock);
+                       spin_lock(&scst_temp_UA_lock);
                        if (cmd->cdb[0] == LOG_SELECT) {
                                scst_set_sense(scst_temp_UA,
                                        sizeof(scst_temp_UA),
@@ -2224,12 +2230,13 @@ static int scst_mode_select_checks(struct scst_cmd *cmd)
                                        sizeof(scst_temp_UA),
                                        UNIT_ATTENTION, 0x2a, 0x01);
                        }
-                       scst_dev_check_set_local_UA(cmd->dev, cmd, scst_temp_UA,
+                       scst_dev_check_set_local_UA(dev, cmd, scst_temp_UA,
                                sizeof(scst_temp_UA));
-                       spin_unlock_bh(&scst_temp_UA_lock);
+                       spin_unlock(&scst_temp_UA_lock);
+                       spin_unlock_bh(&dev->dev_lock);
 
-                       if (cmd->dev->scsi_dev != NULL)
-                               scst_obtain_device_parameters(cmd->dev);
+                       if (dev->scsi_dev != NULL)
+                               scst_obtain_device_parameters(dev);
                }
        } else if ((cmd->status == SAM_STAT_CHECK_CONDITION) && 
                    SCST_SENSE_VALID(cmd->sense_buffer) &&
@@ -3499,9 +3506,10 @@ static int scst_clear_task_set(struct scst_mgmt_cmd *mcmd)
 
        __scst_abort_task_set(mcmd, mcmd->mcmd_tgt_dev, 0, 0);
 
+       mutex_lock(&scst_mutex);
+
        list_for_each_entry(tgt_dev, &dev->dev_tgt_dev_list, 
-               dev_tgt_dev_list_entry) 
-       {
+                       dev_tgt_dev_list_entry) {
                struct scst_session *sess = tgt_dev->sess;
                struct scst_cmd *cmd;
                int aborted = 0;
@@ -3528,6 +3536,8 @@ static int scst_clear_task_set(struct scst_mgmt_cmd *mcmd)
                                        &UA_tgt_devs);
        }
 
+       mutex_unlock(&scst_mutex);
+
        scst_unblock_aborted_cmds(0);
 
        if (!dev->tas) {
@@ -3686,8 +3696,7 @@ static int scst_target_reset(struct scst_mgmt_cmd *mcmd)
                cont = 0;
                c = 0;
                list_for_each_entry(tgt_dev, &dev->dev_tgt_dev_list,
-                       dev_tgt_dev_list_entry) 
-               {
+                               dev_tgt_dev_list_entry) {
                        cont = 1;
                        rc = scst_call_dev_task_mgmt_fn(mcmd, tgt_dev, 0);
                        if (rc == SCST_DEV_TM_NOT_COMPLETED) 
@@ -4378,7 +4387,6 @@ static int scst_init_session(struct scst_session *sess)
 
        TRACE_ENTRY();
 
-       scst_suspend_activity();        
        mutex_lock(&scst_mutex);
 
        if (sess->initiator_name)
@@ -4401,7 +4409,6 @@ static int scst_init_session(struct scst_session *sess)
        res = scst_sess_alloc_tgt_devs(sess);
 
        mutex_unlock(&scst_mutex);
-       scst_resume_activity();
 
        if (sess->init_result_fn) {
                TRACE_DBG("Calling init_result_fn(%p)", sess);