From: Jeffrey Altman <jaltman@your-file-system.com>
Date: Sat, 17 Apr 2010 02:42:34 +0000 (-0400)
Subject: Rx: make conn_call_lock and conn_data_lock usage consistent
X-Git-Tag: openafs-devel-1_5_74~9
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=3ad9c5583845986979a25d615a9640770a3bf9f6;p=packages%2Fo%2Fopenafs.git

Rx: make conn_call_lock and conn_data_lock usage consistent

The rx_connection.flags field is protected by the conn_data_lock
but the conn_data_lock is not held everywhere the conn flags
field is altered.  This produces a race that can result in a
deadlock when waiter flags are inadvertently prevented from being
cleared.

The conn_call_lock usage in rx_EndCall which was removed in
Change e169708681eb1bbbb31951b95f68e861a4b01c7e must be restored.
If rx_EndCall never obtains the conn_call_lock it can't ensure
that the thread in rx_NewCall actively checking the calls will
not end up blocking when there is now a call channel that can
be reused.  This usage of conn_call_lock can be removed only
if a true producer/consumer model is implemented.

Change-Id: Id093fd6c4a42afb833c775411be0cd406abf4ef7
Reviewed-on: http://gerrit.openafs.org/1766
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
---

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 9ff827fa3..14d685cb5 100644
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -2076,6 +2076,9 @@ rx_EndCall(struct rx_call *call, afs_int32 rc)
 	 * have checked this call, found it active and by the time it
 	 * goes to sleep, will have missed the signal.
 	 */
+        MUTEX_EXIT(&call->lock);
+        MUTEX_ENTER(&conn->conn_call_lock);
+        MUTEX_ENTER(&call->lock);
 	MUTEX_ENTER(&conn->conn_data_lock);
 	conn->flags |= RX_CONN_BUSY;
 	if (conn->flags & RX_CONN_MAKECALL_WAITING) {
@@ -2116,7 +2119,10 @@ rx_EndCall(struct rx_call *call, afs_int32 rc)
     CALL_RELE(call, RX_CALL_REFCOUNT_BEGIN);
     MUTEX_EXIT(&call->lock);
     if (conn->type == RX_CLIENT_CONNECTION) {
+	MUTEX_ENTER(&conn->conn_data_lock);
 	conn->flags &= ~RX_CONN_BUSY;
+	MUTEX_EXIT(&conn->conn_data_lock);
+        MUTEX_EXIT(&conn->conn_call_lock);
     }
     USERPRI;
     /*
@@ -2385,8 +2391,8 @@ rxi_FreeCall(struct rx_call *call)
      * If someone else destroys a connection, they either have no
      * call lock held or are going through this section of code.
      */
+    MUTEX_ENTER(&conn->conn_data_lock);
     if (conn->flags & RX_CONN_DESTROY_ME && !(conn->flags & RX_CONN_MAKECALL_WAITING)) {
-	MUTEX_ENTER(&conn->conn_data_lock);
 	conn->refCount++;
 	MUTEX_EXIT(&conn->conn_data_lock);
 #ifdef RX_ENABLE_LOCKS
@@ -2397,6 +2403,8 @@ rxi_FreeCall(struct rx_call *call)
 #else /* RX_ENABLE_LOCKS */
 	rxi_DestroyConnection(conn);
 #endif /* RX_ENABLE_LOCKS */
+    } else {
+	MUTEX_EXIT(&conn->conn_data_lock);
     }
 }
 
@@ -3153,6 +3161,7 @@ rxi_IsConnInteresting(struct rx_connection *aconn)
 
     if (aconn->flags & (RX_CONN_MAKECALL_WAITING | RX_CONN_DESTROY_ME))
 	return 1;
+
     for (i = 0; i < RX_MAXCALLS; i++) {
 	tcall = aconn->call[i];
 	if (tcall) {
diff --git a/src/rx/rx.h b/src/rx/rx.h
index 343a18e29..b51822a2d 100644
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -254,7 +254,7 @@ struct rx_connection {
     struct rx_service *service;	/* used by servers only */
     u_short serviceId;		/* To stamp on requests (clients only) */
     afs_uint32 refCount;		/* Reference count */
-    u_char flags;		/* Defined below */
+    u_char flags;		/* Defined below - (conn_data_lock) */
     u_char type;		/* Type of connection, defined below */
     u_char secondsUntilPing;	/* how often to ping for each active call */
     u_char securityIndex;	/* corresponds to the security class of the */