[Iscsitarget-devel] [patch] replace nthread_lock with test_and_set/clear [r2]

[Ross S. W. W. <RW...@me...>]

Revision2
 
Removes nthread_lock through use of test_and_set_bit() in
nthread_wakeup() combined with set_current_state() in istd()
both perform memory barriers test_and_set_bit() performs a
general while set_current_state() performs a write, should
prevent a race without a spin_lock

Renames nthread_lock wspace_lock which is more apt now.

Fixed bug in revision 1, thanks Lars.
- istd was suppose to issue test_and_clear_bit()

Compile tested.

Signed-off-by: Ross Walker

Index: kernel/iscsi.h
===================================================================
--- kernel/iscsi.h	(revision 328)
+++ kernel/iscsi.h	(working copy)
@@ -69,7 +69,7 @@ struct network_thread_info {
 	unsigned long flags;
 	struct list_head active_conns;
 
-	spinlock_t nthread_lock;
+	spinlock_t wspace_lock;
 
 	void (*old_state_change)(struct sock *);
 	void (*old_data_ready)(struct sock *, int);
Index: kernel/nthread.c
===================================================================
--- kernel/nthread.c	(revision 328)
+++ kernel/nthread.c	(working copy)
@@ -20,19 +20,12 @@ enum daemon_state_bit {
 	D_DATA_READY,
 };
 
-void __nthread_wakeup(struct network_thread_info *info)
-{
-	set_bit(D_DATA_READY, &info->flags);
-	wake_up_process(info->task);
-}
-
 void nthread_wakeup(struct iscsi_target *target)
 {
 	struct network_thread_info *info = &target->nthread_info;
 
-	spin_lock_bh(&info->nthread_lock);
-	__nthread_wakeup(info);
-	spin_unlock_bh(&info->nthread_lock);
+	if (!test_and_set_bit(D_DATA_READY, &info->flags))
+		wake_up_process(info->task);
 }
 
 static inline void iscsi_conn_init_read(struct iscsi_conn *conn, void *data, size_t len)
@@ -292,7 +285,7 @@ next_state:
 }
 
 /*
- * @locking: grabs the target's nthread_lock to protect it from races with
+ * @locking: grabs the target's wspace_lock to protect it from races with
  * iet_write_space()
  */
 static void set_conn_wspace_wait(struct iscsi_conn *conn)
@@ -300,12 +293,12 @@ static void set_conn_wspace_wait(struct 
 	struct network_thread_info *info = &conn->session->target->nthread_info;
 	struct sock *sk = conn->sock->sk;
 
-	spin_lock_bh(&info->nthread_lock);
+	spin_lock_bh(&info->wspace_lock);
 
 	if (sk_stream_wspace(sk) < sk_stream_min_wspace(sk))
 		set_bit(CONN_WSPACE_WAIT, &conn->state);
 
-	spin_unlock_bh(&info->nthread_lock);
+	spin_unlock_bh(&info->wspace_lock);
 }
 
 /* This is taken from the Ardis code. */
@@ -706,17 +699,10 @@ static int istd(void *arg)
 
 	__set_current_state(TASK_RUNNING);
 	do {
-		spin_lock_bh(&info->nthread_lock);
-		__set_current_state(TASK_INTERRUPTIBLE);
-
-		if (!test_bit(D_DATA_READY, &info->flags)) {
-			spin_unlock_bh(&info->nthread_lock);
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (!test_and_clear_bit(D_DATA_READY, &info->flags))
 			schedule();
-			spin_lock_bh(&info->nthread_lock);
-		}
 		__set_current_state(TASK_RUNNING);
-		clear_bit(D_DATA_READY, &info->flags);
-		spin_unlock_bh(&info->nthread_lock);
 
 		target_lock(target, 0);
 		list_for_each_entry_safe(conn, tmp, &info->active_conns, poll_list) {
@@ -745,7 +731,7 @@ int nthread_init(struct iscsi_target *ta
 
 	INIT_LIST_HEAD(&info->active_conns);
 
-	spin_lock_init(&info->nthread_lock);
+	spin_lock_init(&info->wspace_lock);
 
 	return 0;
 }
Index: kernel/conn.c
===================================================================
--- kernel/conn.c	(revision 328)
+++ kernel/conn.c	(working copy)
@@ -97,23 +97,24 @@ static void iet_data_ready(struct sock *
 }
 
 /*
- * @locking: grabs the target's nthread_lock to protect it from races with
+ * @locking: grabs the target's wspace_lock to protect it from races with
  * set_conn_wspace_wait()
  */
 static void iet_write_space(struct sock *sk)
 {
 	struct iscsi_conn *conn = sk->sk_user_data;
-	struct network_thread_info *info = &conn->session->target->nthread_info;
+	struct iscsi_target *target = conn->session->target;
+	struct network_thread_info *info = &target->nthread_info;
 
-	spin_lock_bh(&info->nthread_lock);
+	spin_lock_bh(&info->wspace_lock);
 
 	if (sk_stream_wspace(sk) >= sk_stream_min_wspace(sk) &&
 	    test_bit(CONN_WSPACE_WAIT, &conn->state)) {
 		clear_bit(CONN_WSPACE_WAIT, &conn->state);
-		__nthread_wakeup(info);
+		nthread_wakeup(target);
 	}
 
-	spin_unlock_bh(&info->nthread_lock);
+	spin_unlock_bh(&info->wspace_lock);
 
 	info->old_write_space(sk);
 }

______________________________________________________________________
This e-mail, and any attachments thereto, is intended only for use by
the addressee(s) named herein and may contain legally privileged
and/or confidential information. If you are not the intended recipient
of this e-mail, you are hereby notified that any dissemination,
distribution or copying of this e-mail, and any attachments thereto,
is strictly prohibited. If you have received this e-mail in error,
please immediately notify the sender and permanently delete the
original and any copy or printout thereof.