iscsi-target: Add thread_set->ts_activate_sem + use common deallocate

This patch removes the iscsi_thread_set->[rx,tx]_post_start_comp that
was originally used synchronize startup between rx and tx threads within
a single thread_set.

Instead, use a single ->ts_activate_sem in iscsi_activate_thread_set()
to wait for both processes to awake in the RX/TX pre handlers.

Also, go ahead and refactor thread_set deallocate code into a common
iscsi_deallocate_thread_one(), and update iscsi_deallocate_thread_sets()
and iscsi_deallocate_extra_thread_sets() use this code

v3 changes:
  - Make iscsi_deallocate_thread_one defined as static (Fengguang)

v2 changes:
  - Set ISCSI_THREAD_SET_ACTIVE before calling complete in
    iscsi_activate_thread_set
  - Protect ts->conn sanity checks with ->ts_state_lock in
    RX/TX pre handlers
  - Add ->ts_activate_sem to save extra context switches per
    iscsi_activate_thread_set() call.
  - Refactor thread_set shutdown into iscsi_deallocate_thread_one()

Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

diff --git a/drivers/target/iscsi/iscsi_target_tq.c b/drivers/target/iscsi/iscsi_target_tq.c
index 1a5bbec03f3e2389c764202a2ab1109b27a4a32f..ba0ac41889af32f44e22f800174d8312b3e6110a 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_tq.c
+++ b/drivers/target/iscsi/iscsi_target_tq.c
@@ -105,12 +105,11 @@ int iscsi_allocate_thread_sets(u32 thread_pair_count)
                ts->status = ISCSI_THREAD_SET_FREE;
                INIT_LIST_HEAD(&ts->ts_list);
                spin_lock_init(&ts->ts_state_lock);
-               init_completion(&ts->rx_post_start_comp);
-               init_completion(&ts->tx_post_start_comp);
                init_completion(&ts->rx_restart_comp);
                init_completion(&ts->tx_restart_comp);
                init_completion(&ts->rx_start_comp);
                init_completion(&ts->tx_start_comp);
+               sema_init(&ts->ts_activate_sem, 0);
 
                ts->create_threads = 1;
                ts->tx_thread = kthread_run(iscsi_target_tx_thread, ts, "%s",
@@ -139,35 +138,44 @@ int iscsi_allocate_thread_sets(u32 thread_pair_count)
        return allocated_thread_pair_count;
 }
 
-void iscsi_deallocate_thread_sets(void)
+static void iscsi_deallocate_thread_one(struct iscsi_thread_set *ts)
 {
-       u32 released_count = 0;
-       struct iscsi_thread_set *ts = NULL;
-
-       while ((ts = iscsi_get_ts_from_inactive_list())) {
+       spin_lock_bh(&ts->ts_state_lock);
+       ts->status = ISCSI_THREAD_SET_DIE;
 
+       if (ts->rx_thread) {
+               complete(&ts->rx_start_comp);
+               spin_unlock_bh(&ts->ts_state_lock);
+               kthread_stop(ts->rx_thread);
                spin_lock_bh(&ts->ts_state_lock);
-               ts->status = ISCSI_THREAD_SET_DIE;
+       }
+       if (ts->tx_thread) {
+               complete(&ts->tx_start_comp);
                spin_unlock_bh(&ts->ts_state_lock);
+               kthread_stop(ts->tx_thread);
+               spin_lock_bh(&ts->ts_state_lock);
+       }
+       spin_unlock_bh(&ts->ts_state_lock);
+       /*
+        * Release this thread_id in the thread_set_bitmap
+        */
+       spin_lock(&ts_bitmap_lock);
+       bitmap_release_region(iscsit_global->ts_bitmap,
+                       ts->thread_id, get_order(1));
+       spin_unlock(&ts_bitmap_lock);
 
-               if (ts->rx_thread) {
-                       send_sig(SIGINT, ts->rx_thread, 1);
-                       kthread_stop(ts->rx_thread);
-               }
-               if (ts->tx_thread) {
-                       send_sig(SIGINT, ts->tx_thread, 1);
-                       kthread_stop(ts->tx_thread);
-               }
-               /*
-                * Release this thread_id in the thread_set_bitmap
-                */
-               spin_lock(&ts_bitmap_lock);
-               bitmap_release_region(iscsit_global->ts_bitmap,
-                               ts->thread_id, get_order(1));
-               spin_unlock(&ts_bitmap_lock);
+       kfree(ts);
+}
+
+void iscsi_deallocate_thread_sets(void)
+{
+       struct iscsi_thread_set *ts = NULL;
+       u32 released_count = 0;
 
+       while ((ts = iscsi_get_ts_from_inactive_list())) {
+
+               iscsi_deallocate_thread_one(ts);
                released_count++;
-               kfree(ts);
        }
 
        if (released_count)
@@ -187,38 +195,13 @@ static void iscsi_deallocate_extra_thread_sets(void)
                if (!ts)
                        break;
 
-               spin_lock_bh(&ts->ts_state_lock);
-               ts->status = ISCSI_THREAD_SET_DIE;
-
-               if (ts->rx_thread) {
-                       complete(&ts->rx_start_comp);
-                       spin_unlock_bh(&ts->ts_state_lock);
-                       kthread_stop(ts->rx_thread);
-                       spin_lock_bh(&ts->ts_state_lock);
-               }
-               if (ts->tx_thread) {
-                       complete(&ts->tx_start_comp);
-                       spin_unlock_bh(&ts->ts_state_lock);
-                       kthread_stop(ts->tx_thread);
-                       spin_lock_bh(&ts->ts_state_lock);
-               }
-               spin_unlock_bh(&ts->ts_state_lock);
-               /*
-                * Release this thread_id in the thread_set_bitmap
-                */
-               spin_lock(&ts_bitmap_lock);
-               bitmap_release_region(iscsit_global->ts_bitmap,
-                               ts->thread_id, get_order(1));
-               spin_unlock(&ts_bitmap_lock);
-
+               iscsi_deallocate_thread_one(ts);
                released_count++;
-               kfree(ts);
        }
 
-       if (released_count) {
+       if (released_count)
                pr_debug("Stopped %d thread set(s) (%d total threads)."
                        "\n", released_count, released_count * 2);
-       }
 }
 
 void iscsi_activate_thread_set(struct iscsi_conn *conn, struct iscsi_thread_set *ts)
@@ -228,14 +211,13 @@ void iscsi_activate_thread_set(struct iscsi_conn *conn, struct iscsi_thread_set
        spin_lock_bh(&ts->ts_state_lock);
        conn->thread_set = ts;
        ts->conn = conn;
+       ts->status = ISCSI_THREAD_SET_ACTIVE;
        spin_unlock_bh(&ts->ts_state_lock);
-       /*
-        * Start up the RX thread and wait on rx_post_start_comp.  The RX
-        * Thread will then do the same for the TX Thread in
-        * iscsi_rx_thread_pre_handler().
-        */
+
        complete(&ts->rx_start_comp);
-       wait_for_completion(&ts->rx_post_start_comp);
+       complete(&ts->tx_start_comp);
+
+       down(&ts->ts_activate_sem);
 }
 
 struct iscsi_thread_set *iscsi_get_thread_set(void)
@@ -267,6 +249,7 @@ get_set:
        ts->thread_count = 2;
        init_completion(&ts->rx_restart_comp);
        init_completion(&ts->tx_restart_comp);
+       sema_init(&ts->ts_activate_sem, 0);
 
        return ts;
 }
@@ -452,18 +435,19 @@ sleep:
        if (iscsi_signal_thread_pre_handler(ts) < 0)
                return NULL;
 
+       iscsi_check_to_add_additional_sets();
+
+       spin_lock_bh(&ts->ts_state_lock);
        if (!ts->conn) {
                pr_err("struct iscsi_thread_set->conn is NULL for"
-                       " thread_id: %d, going back to sleep\n", ts->thread_id);
-               goto sleep;
+                       " RX thread_id: %s/%d\n", current->comm, current->pid);
+               spin_unlock_bh(&ts->ts_state_lock);
+               return NULL;
        }
-       iscsi_check_to_add_additional_sets();
-       /*
-        * The RX Thread starts up the TX Thread and sleeps.
-        */
        ts->thread_clear |= ISCSI_CLEAR_RX_THREAD;
-       complete(&ts->tx_start_comp);
-       wait_for_completion(&ts->tx_post_start_comp);
+       spin_unlock_bh(&ts->ts_state_lock);
+
+       up(&ts->ts_activate_sem);
 
        return ts->conn;
 }
@@ -505,27 +489,20 @@ sleep:
        if (iscsi_signal_thread_pre_handler(ts) < 0)
                return NULL;
 
-       if (!ts->conn) {
-               pr_err("struct iscsi_thread_set->conn is NULL for "
-                       " thread_id: %d, going back to sleep\n",
-                       ts->thread_id);
-               goto sleep;
-       }
-
        iscsi_check_to_add_additional_sets();
-       /*
-        * From the TX thread, up the tx_post_start_comp that the RX Thread is
-        * sleeping on in iscsi_rx_thread_pre_handler(), then up the
-        * rx_post_start_comp that iscsi_activate_thread_set() is sleeping on.
-        */
-       ts->thread_clear |= ISCSI_CLEAR_TX_THREAD;
-       complete(&ts->tx_post_start_comp);
-       complete(&ts->rx_post_start_comp);
 
        spin_lock_bh(&ts->ts_state_lock);
-       ts->status = ISCSI_THREAD_SET_ACTIVE;
+       if (!ts->conn) {
+               pr_err("struct iscsi_thread_set->conn is NULL for"
+                       " TX thread_id: %s/%d\n", current->comm, current->pid);
+               spin_unlock_bh(&ts->ts_state_lock);
+               return NULL;
+       }
+       ts->thread_clear |= ISCSI_CLEAR_TX_THREAD;
        spin_unlock_bh(&ts->ts_state_lock);
 
+       up(&ts->ts_activate_sem);
+
        return ts->conn;
 }
 

diff --git a/drivers/target/iscsi/iscsi_target_tq.h b/drivers/target/iscsi/iscsi_target_tq.h
index 547d118312820d2a39ba960b11844298810190ce..cc1eede5ab3a9cc4ad09fcbe3714eae77cdfb083 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_tq.h
+++ b/drivers/target/iscsi/iscsi_target_tq.h
@@ -64,10 +64,6 @@ struct iscsi_thread_set {
        struct iscsi_conn       *conn;
        /* used for controlling ts state accesses */
        spinlock_t      ts_state_lock;
-       /* Used for rx side post startup */
-       struct completion       rx_post_start_comp;
-       /* Used for tx side post startup */
-       struct completion       tx_post_start_comp;
        /* used for restarting thread queue */
        struct completion       rx_restart_comp;
        /* used for restarting thread queue */
@@ -82,6 +78,7 @@ struct iscsi_thread_set {
        struct task_struct      *tx_thread;
        /* struct iscsi_thread_set in list list head*/
        struct list_head        ts_list;
+       struct semaphore        ts_activate_sem;
 };
 
 #endif   /*** ISCSI_THREAD_QUEUE_H ***/