[linux-cifs-client] [PATCH] cifs: fix wait_for_response to time out sleeping processes correctly

[Jeff Layton]

The current scheme that CIFS uses to sleep and wait for a response is
not quite what we want. After sending a request, wait_for_response puts
the task to sleep with wait_event(). One of the conditions for
wait_event is a timeout (using time_after()).

The problem with this is that there is no guarantee that the process
will ever be woken back up. If the server stops sending data, then
cifs_demultiplex_thread will leave its response queue sleeping.

I think the only thing that saves us here is the fact that
cifs_dnotify_thread periodically (every 15s) wakes up sleeping processes
on all response_q's that have calls in flight.  This makes for
unnecessary wakeups of some processes. It also means large variability
in the timeouts since they're all woken up at once.

Instead of this, put the tasks to sleep with wait_event_timeout. This
makes them wake up on their own if they time out. With this change,
cifs_dnotify_thread should no longer be needed.

I've been testing this in conjunction with some other patches that I'm
working on. It doesn't seem to affect performance at all with with heavy
I/O. Identical iozone -ac runs complete in almost exactly the same time
(<1% difference in times).

Thanks to Wasrshi Nimara for initially pointing this out. Wasrshi, it
would be nice to know whether this patch also helps your testcase.

Signed-off-by: Jeff Layton <jlayton at redhat.com>
Cc: Wasrshi Nimara <warshinimara at gmail.com>
---
 fs/cifs/cifsfs.c    |   37 -------------------------------------
 fs/cifs/transport.c |    7 ++-----
 2 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7f87066..1bd03af 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -66,7 +66,6 @@ unsigned int sign_CIFS_PDUs = 1;
 extern struct task_struct *oplockThread; /* remove sparse warning */
 struct task_struct *oplockThread = NULL;
 /* extern struct task_struct * dnotifyThread; remove sparse warning */
-static struct task_struct *dnotifyThread = NULL;
 static const struct super_operations cifs_super_ops;
 unsigned int CIFSMaxBufSize = CIFS_MAX_MSGSIZE;
 module_param(CIFSMaxBufSize, int, 0);
@@ -1049,32 +1048,6 @@ static int cifs_oplock_thread(void *dummyarg)
 	return 0;
 }
 
-static int cifs_dnotify_thread(void *dummyarg)
-{
-	struct list_head *tmp;
-	struct TCP_Server_Info *server;
-
-	do {
-		if (try_to_freeze())
-			continue;
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(15*HZ);
-		/* check if any stuck requests that need
-		   to be woken up and wakeq so the
-		   thread can wake up and error out */
-		read_lock(&cifs_tcp_ses_lock);
-		list_for_each(tmp, &cifs_tcp_ses_list) {
-			server = list_entry(tmp, struct TCP_Server_Info,
-					 tcp_ses_list);
-			if (atomic_read(&server->inFlight))
-				wake_up_all(&server->response_q);
-		}
-		read_unlock(&cifs_tcp_ses_lock);
-	} while (!kthread_should_stop());
-
-	return 0;
-}
-
 static int __init
 init_cifs(void)
 {
@@ -1151,17 +1124,8 @@ init_cifs(void)
 		goto out_unregister_dfs_key_type;
 	}
 
-	dnotifyThread = kthread_run(cifs_dnotify_thread, NULL, "cifsdnotifyd");
-	if (IS_ERR(dnotifyThread)) {
-		rc = PTR_ERR(dnotifyThread);
-		cERROR(1, ("error %d create dnotify thread", rc));
-		goto out_stop_oplock_thread;
-	}
-
 	return 0;
 
- out_stop_oplock_thread:
-	kthread_stop(oplockThread);
  out_unregister_dfs_key_type:
 #ifdef CONFIG_CIFS_DFS_UPCALL
 	unregister_key_type(&key_type_dns_resolver);
@@ -1200,7 +1164,6 @@ exit_cifs(void)
 	cifs_destroy_mids();
 	cifs_destroy_request_bufs();
 	kthread_stop(oplockThread);
-	kthread_stop(dnotifyThread);
 }
 
 MODULE_AUTHOR("Steve French <sfrench at us.ibm.com>");
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 8e0d1c3..6c1e10c 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -410,11 +410,8 @@ static int wait_for_response(struct cifsSesInfo *ses,
 
 	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)));
+		wait_event_timeout(ses->server->response_q,
+			midQ->midState != MID_REQUEST_SUBMITTED, timeout);
 
 		if (time_after(jiffies, curr_timeout) &&
 			(midQ->midState == MID_REQUEST_SUBMITTED) &&
-- 
1.5.5.1