[linux-cifs-client] Re: Bug in cifsfs POSIX locking client code.

Jeremy Allison jra at samba.org
Fri Jul 14 03:56:52 GMT 2006


On Wed, Jul 12, 2006 at 12:13:53AM -0700, Jeremy Allison wrote:
> 
> I'm guessing the waiting for blocking calls still isn't
> working correctly....

Steve - here's a possible fix as we discussed earlier
today - it keeps a "last received time" in the session
struct. I'm going to test this with my tdbtorture overnight
and see if it fixes the problem.

Let me know what you think of the patch (it applies to the
linux-converged-for-old-kernels branch).

Jeremy.
-------------- next part --------------
Index: connect.c
===================================================================
--- connect.c	(revision 55)
+++ connect.c	(working copy)
@@ -609,6 +609,10 @@
 
 		task_to_wake = NULL;
 		spin_lock(&GlobalMid_Lock);
+
+		/* Note when we last received a reply - needed for timeout purposes. */
+		server->last_receive_time = jiffies;
+
 		list_for_each(tmp, &server->pending_mid_q) {
 			mid_entry = list_entry(tmp, struct mid_q_entry, qhead);
 
Index: transport.c
===================================================================
--- transport.c	(revision 55)
+++ transport.c	(working copy)
@@ -476,12 +476,38 @@
 
 	/* No user interrupts in wait - wreaks havoc with performance */
 	if(timeout != MAX_SCHEDULE_TIMEOUT) {
-		timeout += jiffies;
-		wait_event(ses->server->response_q,
-			(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
-			time_after(jiffies, timeout) || 
-			((ses->server->tcpStatus != CifsGood) &&
-			 (ses->server->tcpStatus != CifsNew)));
+		unsigned long curr_timeout;
+
+		for (;;) {
+			curr_timeout = timeout + jiffies;
+			wait_event(ses->server->response_q,
+				(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
+				time_after(jiffies, curr_timeout) || 
+				((ses->server->tcpStatus != CifsGood) &&
+				 (ses->server->tcpStatus != CifsNew)));
+
+			if (time_after(jiffies, curr_timeout) &&
+				(midQ->midState & MID_REQUEST_SUBMITTED) &&
+				((ses->server->tcpStatus == CifsGood) ||
+				 (ses->server->tcpStatus == CifsNew))) {
+
+				unsigned long lrt;
+
+				/* We timed out. Is the server still
+				   sending replies ? */
+				spin_lock(&GlobalMid_Lock);
+				lrt = ses->server->last_receive_time;
+				spin_unlock(&GlobalMid_Lock);
+
+				/* Calculate 15 seconds past last received time. */
+				lrt += (15 * HZ);
+				if (time_after(jiffies, lrt)) {
+					/* Server sent no replies for 15 seconds. */	
+					cERROR(1,("Server idle for 15 seconds. Timing out"));
+					break;
+				}
+			}
+		}
 	} else {
 		wait_event(ses->server->response_q,
 			(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
@@ -743,12 +769,38 @@
 
 	/* No user interrupts in wait - wreaks havoc with performance */
 	if(timeout != MAX_SCHEDULE_TIMEOUT) {
-		timeout += jiffies;
-		wait_event(ses->server->response_q,
-			(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
-			time_after(jiffies, timeout) || 
-			((ses->server->tcpStatus != CifsGood) &&
-			 (ses->server->tcpStatus != CifsNew)));
+		unsigned long curr_timeout;
+
+		for (;;) {
+			curr_timeout = timeout + jiffies;
+			wait_event(ses->server->response_q,
+				(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
+				time_after(jiffies, curr_timeout) || 
+				((ses->server->tcpStatus != CifsGood) &&
+				 (ses->server->tcpStatus != CifsNew)));
+
+			if (time_after(jiffies, curr_timeout) &&
+				(midQ->midState & MID_REQUEST_SUBMITTED) &&
+				((ses->server->tcpStatus == CifsGood) ||
+				 (ses->server->tcpStatus == CifsNew))) {
+
+				unsigned long lrt;
+
+				/* We timed out. Is the server still
+				   sending replies ? */
+				spin_lock(&GlobalMid_Lock);
+				lrt = ses->server->last_receive_time;
+				spin_unlock(&GlobalMid_Lock);
+
+				/* Calculate 15 seconds past last received time. */
+				lrt += (15 * HZ);
+				if (time_after(jiffies, lrt)) {
+					/* Server sent no replies for 15 seconds. */	
+					cERROR(1,("Server idle for 15 seconds. Timing out"));
+					break;
+				}
+			}
+		}
 	} else {
 		wait_event(ses->server->response_q,
 			(!(midQ->midState & MID_REQUEST_SUBMITTED)) || 
Index: cifsglob.h
===================================================================
--- cifsglob.h	(revision 55)
+++ cifsglob.h	(working copy)
@@ -155,6 +155,7 @@
 	int capabilities; /* allow selective disabling of caps by smb sess */
 	__u16 timeZone;
 	__u16 CurrentMid;         /* multiplex id - rotating counter */
+	unsigned long last_receive_time;
 	char cryptKey[CIFS_CRYPTO_KEY_SIZE];
 	/* 16th byte of RFC1001 workstation name is always null */
 	char workstation_RFC1001_name[SERVER_NAME_LEN_WITH_NULL];


More information about the linux-cifs-client mailing list