[linux-cifs-client] [PATCH 3/4] CIFS fixes for 2.4.x

[Sergey Vlasov]

This patch sets the signal mask in SendReceive() to block all
signals during this function, and also makes the wait in this
function uninterruptible.

In theory, the ability to interrupt a long FS operation with a
signal would be good (especially for a network filesystem).
However, the way it was implemented in SendReceive() does not really
work: when I applied the previous 2 patches, I still got lots of -4
(EINTR) errors when writing large files.

The problem is that writing dirty pages from the page cache is
performed asynchronously, and can happen in context of any process.
Most writes are performed by kswapd, but any process which performs
memory allocations can trigger a call to ->writepage in its own
context.  If this process receives signals, and SendReceive() uses
an interruptible wait, these signals would then interrupt the
writepage handler and cause data loss.

The signal blocking code was stolen from smbfs; it is needed to
avoid -EINTR returns from sock_sendmsg().  The big-looking part of
the patch is mostly reindenting after "if" removal (also an expicit
return was replaced by "goto cifs_no_response_exit" to perform the
proper cleanup).


--- kernel-source-cifs-1.0.2/transport.c.send-sigmask	2004-04-08 14:02:24 +0400
+++ kernel-source-cifs-1.0.2/transport.c	2004-04-08 14:18:15 +0400
@@ -174,6 +174,7 @@ SendReceive(const unsigned int xid, stru
 	unsigned int receive_len;
 	long timeout = 10 * HZ;
 	struct mid_q_entry *midQ;
+	sigset_t old_set;
 
 	if ((ses == NULL) || (ses->server == NULL)) {
 		cERROR(1,("Null tcp session or smb session: %p",ses));
@@ -216,6 +217,12 @@ SendReceive(const unsigned int xid, stru
 	
 	rc = cifs_sign_smb(in_buf, ses, &midQ->sequence_number);
 
+	spin_lock_irq(&current->sigmask_lock);
+	old_set = current->blocked;
+	siginitsetinv(&current->blocked, sigmask(SIGKILL)|sigmask(SIGSTOP));
+	recalc_sigpending(current);
+	spin_unlock_irq(&current->sigmask_lock);
+
 	midQ->midState = MID_REQUEST_SUBMITTED;
 	rc = smb_send(ses->server->ssocket, in_buf, in_buf->smb_buf_length,
 		      (struct sockaddr *) &(ses->server->sockAddr));
@@ -234,39 +241,33 @@ SendReceive(const unsigned int xid, stru
 	/* wait for 15 seconds or until woken up due to response arriving or 
 	   due to last connection to this server being unmounted */
 
-	timeout = interruptible_sleep_on_timeout(&ses->server->response_q,timeout);
+	timeout = sleep_on_timeout(&ses->server->response_q,timeout);
 
-	if (signal_pending(current)) {
-		cFYI(1, ("CIFS: caught signal"));
-		DeleteMidQEntry(midQ);
-		return -EINTR;
-	} else {  /* BB spinlock protect this against races with demux thread */
-		spin_lock(&GlobalMid_Lock);
-		if (midQ->resp_buf) {
-			spin_unlock(&GlobalMid_Lock);
-			receive_len =
-			    be32_to_cpu(midQ->resp_buf->smb_buf_length);
-		} else {
-			cFYI(1,("No response buffer"));
-			if(midQ->midState == MID_REQUEST_SUBMITTED) {
-				if(ses->server->tcpStatus == CifsExiting)
-					rc = -EHOSTDOWN;
-				else {
-				ses->server->tcpStatus = CifsNeedReconnect;
-				midQ->midState = MID_RETRY_NEEDED;
-				}
+	/* BB spinlock protect this against races with demux thread */
+	spin_lock(&GlobalMid_Lock);
+	if (midQ->resp_buf) {
+		spin_unlock(&GlobalMid_Lock);
+		receive_len =
+		    be32_to_cpu(midQ->resp_buf->smb_buf_length);
+	} else {
+		cFYI(1,("No response buffer"));
+		if(midQ->midState == MID_REQUEST_SUBMITTED) {
+			if(ses->server->tcpStatus == CifsExiting)
+				rc = -EHOSTDOWN;
+			else {
+			ses->server->tcpStatus = CifsNeedReconnect;
+			midQ->midState = MID_RETRY_NEEDED;
 			}
+		}
 
-			if(midQ->midState == MID_RETRY_NEEDED) {
-				rc = -EAGAIN;
-				cFYI(1,("marking request for retry"));
-			} else {
-				rc = -EIO;
-			}
-			spin_unlock(&GlobalMid_Lock);
-			DeleteMidQEntry(midQ);
-			return rc;
+		if(midQ->midState == MID_RETRY_NEEDED) {
+			rc = -EAGAIN;
+			cFYI(1,("marking request for retry"));
+		} else {
+			rc = -EIO;
 		}
+		spin_unlock(&GlobalMid_Lock);
+		goto cifs_no_response_exit;
 	}
 
 	if (receive_len > CIFS_MAX_MSGSIZE + MAX_CIFS_HDR_SIZE) {
@@ -318,6 +319,11 @@ SendReceive(const unsigned int xid, stru
 		}
 	}
 cifs_no_response_exit:
+	spin_lock_irq(&current->sigmask_lock);
+	current->blocked = old_set;
+	recalc_sigpending(current);
+	spin_unlock_irq(&current->sigmask_lock);
+
 	DeleteMidQEntry(midQ);	/* BB what if process is killed?
 			 - BB add background daemon to clean up Mid entries from
 			 killed processes & test killing process with active mid */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.samba.org/archive/linux-cifs-client/attachments/20040408/4d50805b/attachment.bin