From: David van Moolenbroek <david@minix3.org>
Date: Fri, 19 Jun 2015 22:06:19 +0000 (+0000)
Subject: VFS: fix pipe resumption delay bug
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/zpipe.c?a=commitdiff_plain;h=refs%2Fchanges%2F03%2F3003%2F1;p=minix.git

VFS: fix pipe resumption delay bug

Commit 723e513 erroneously removed a yield() call from VFS which was
necessary to get resumed pipe read/write threads to run before VFS
blocks on receive().  The removal caused those threads to run only
once VFS received another message, effectively slowing down activity
on pipes to a crawl in some cases.

Instead of readding the yield() call, this patch restructures the
get_work() code to go back through the main message loop even when no
new work is received, thus ensuring that newly started threads are
always activated without requiring a special case.

This fixes #65.

Change-Id: I59b7fb9e403d87dba1a5deecb04539cc37517742
---

diff --git a/minix/servers/vfs/main.c b/minix/servers/vfs/main.c
index 0c1c79288..4dac592c2 100644
--- a/minix/servers/vfs/main.c
+++ b/minix/servers/vfs/main.c
@@ -41,7 +41,7 @@ static void do_init_root(void);
 static void handle_work(void (*func)(void));
 static void reply(message *m_out, endpoint_t whom, int result);
 
-static void get_work(void);
+static int get_work(void);
 static void service_pm(void);
 static int unblock(struct fproc *rfp);
 
@@ -74,7 +74,12 @@ int main(void)
 	yield_all();	/* let other threads run */
 	self = NULL;
 	send_work();
-	get_work();
+
+	/* The get_work() function returns TRUE if we have a new message to
+	 * process. It returns FALSE if it spawned other thread activities.
+	 */
+	if (!get_work())
+		continue;
 
 	transid = TRNS_GET_ID(m_in.m_type);
 	if (IS_VFS_FS_TRANSID(transid)) {
@@ -476,28 +481,23 @@ void thread_cleanup(void)
 /*===========================================================================*
  *				get_work				     *
  *===========================================================================*/
-static void get_work()
+static int get_work(void)
 {
-  /* Normally wait for new input.  However, if 'reviving' is
-   * nonzero, a suspended process must be awakened.
+  /* Normally wait for new input.  However, if 'reviving' is nonzero, a
+   * suspended process must be awakened.  Return TRUE if there is a message to
+   * process (usually newly received, but possibly a resumed request), or FALSE
+   * if a thread for other activities has been spawned instead.
    */
-  int r, found_one, proc_p;
+  int r, proc_p;
   register struct fproc *rp;
 
-  while (reviving != 0) {
-	found_one = FALSE;
-
+  if (reviving != 0) {
 	/* Find a suspended process. */
 	for (rp = &fproc[0]; rp < &fproc[NR_PROCS]; rp++)
-		if (rp->fp_pid != PID_FREE && (rp->fp_flags & FP_REVIVED)) {
-			found_one = TRUE; /* Found a suspended process */
-			if (unblock(rp))
-				return;	/* So main loop can process job */
-			send_work();
-		}
+		if (rp->fp_pid != PID_FREE && (rp->fp_flags & FP_REVIVED))
+			return unblock(rp); /* So main loop can process job */
 
-	if (!found_one)	/* Consistency error */
-		panic("VFS: get_work couldn't revive anyone");
+	panic("VFS: get_work couldn't revive anyone");
   }
 
   for(;;) {
@@ -510,11 +510,6 @@ static void get_work()
 	if (proc_p < 0 || proc_p >= NR_PROCS) fp = NULL;
 	else fp = &fproc[proc_p];
 
-	if (m_in.m_type == EDEADSRCDST) {
-		printf("VFS: failed ipc_sendrec\n");
-		return;	/* Failed 'ipc_sendrec' */
-	}
-
 	/* Negative who_p is never used to access the fproc array. Negative
 	 * numbers (kernel tasks) are treated in a special way.
 	 */
@@ -536,8 +531,9 @@ static void get_work()
 			fproc[who_p].fp_endpoint, who_e);
 	}
 
-	return;
+	return TRUE;
   }
+  /* NOTREACHED */
 }
 
 /*===========================================================================*