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: NNicholas Bellinger <nab@linux-iscsi.org>

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 @@ struct iscsi_thread_set *iscsi_get_thread_set(void)
 	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 @@ struct iscsi_conn *iscsi_rx_thread_pre_handler(struct iscsi_thread_set *ts)
 	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 @@ struct iscsi_conn *iscsi_tx_thread_pre_handler(struct iscsi_thread_set *ts)
 	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;
 }
 

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 ***/