From: David van Moolenbroek <david@minix3.org>
Date: Sat, 24 Aug 2013 22:26:38 +0000 (+0200)
Subject: VFS: properly cancel select queries on unpause
X-Git-Tag: v3.3.0~609
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/index.css?a=commitdiff_plain;h=refs%2Fchanges%2F49%2F949%2F2;p=minix.git

VFS: properly cancel select queries on unpause

Change-Id: I16e71db3f5c1bcc7ba6045bc9f02b13d71dc31eb
---

diff --git a/servers/vfs/dmap.c b/servers/vfs/dmap.c
index c6491fde8..947f638f3 100644
--- a/servers/vfs/dmap.c
+++ b/servers/vfs/dmap.c
@@ -24,9 +24,6 @@
 
 struct dmap dmap[NR_DEVICES];
 
-#define DT_EMPTY { no_dev, no_dev_io, NONE, "", 0, STYLE_NDEV, NULL, NONE, \
-		   0, NULL, 0}
-
 /*===========================================================================*
  *				lock_dmap		 		     *
  *===========================================================================*/
@@ -281,10 +278,16 @@ void init_dmap()
 {
 /* Initialize the table with empty device <-> driver mappings. */
   int i;
-  struct dmap dmap_default = DT_EMPTY;
 
-  for (i = 0; i < NR_DEVICES; i++)
-	dmap[i] = dmap_default;
+  memset(dmap, 0, sizeof(dmap));
+
+  for (i = 0; i < NR_DEVICES; i++) {
+	dmap[i].dmap_opcl = no_dev;
+	dmap[i].dmap_io = no_dev_io;
+	dmap[i].dmap_driver = NONE;
+	dmap[i].dmap_style = STYLE_NDEV;
+	dmap[i].dmap_servicing = NONE;
+  }
 }
 
 /*===========================================================================*
diff --git a/servers/vfs/dmap.h b/servers/vfs/dmap.h
index cd15a1cc4..4520d577f 100644
--- a/servers/vfs/dmap.h
+++ b/servers/vfs/dmap.h
@@ -20,6 +20,7 @@ extern struct dmap {
   char dmap_label[LABEL_MAX];
   int dmap_flags;
   int dmap_style;
+  int dmap_sel_busy;
   struct filp *dmap_sel_filp;
   endpoint_t dmap_servicing;
   mutex_t dmap_lock;
diff --git a/servers/vfs/pipe.c b/servers/vfs/pipe.c
index 50f6ddb3e..f46111496 100644
--- a/servers/vfs/pipe.c
+++ b/servers/vfs/pipe.c
@@ -557,7 +557,7 @@ void unpause(endpoint_t proc_e)
 		break;
 
 	case FP_BLOCKED_ON_SELECT:/* process blocking on select() */
-		select_forget(proc_e);
+		select_forget();
 		break;
 
 	case FP_BLOCKED_ON_POPEN:	/* process trying to open a fifo */
diff --git a/servers/vfs/proto.h b/servers/vfs/proto.h
index 8f1d16089..300c6dc6c 100644
--- a/servers/vfs/proto.h
+++ b/servers/vfs/proto.h
@@ -360,7 +360,7 @@ int do_gcov_flush(void);
 int do_select(message *m_out);
 void init_select(void);
 void select_callback(struct filp *, int ops);
-void select_forget(endpoint_t proc_e);
+void select_forget(void);
 void select_reply1(endpoint_t driver_e, int minor, int status);
 void select_reply2(endpoint_t driver_e, int minor, int status);
 void select_timeout_check(timer_t *);
diff --git a/servers/vfs/select.c b/servers/vfs/select.c
index 81f20e076..46f769177 100644
--- a/servers/vfs/select.c
+++ b/servers/vfs/select.c
@@ -377,7 +377,7 @@ static int select_request_major(struct filp *f, int *ops, int block)
 	return(SUSPEND);
 
   dp = &dmap[major];
-  if (dp->dmap_sel_filp)
+  if (dp->dmap_sel_busy)
 	return(SUSPEND);
 
   f->filp_select_flags &= ~FSF_UPDATE;
@@ -385,6 +385,7 @@ static int select_request_major(struct filp *f, int *ops, int block)
   if (r != OK)
 	return(r);
 
+  dp->dmap_sel_busy = TRUE;
   dp->dmap_sel_filp = f;
   f->filp_select_flags |= FSF_BUSY;
 
@@ -573,11 +574,11 @@ static void select_cancel_all(struct selectentry *se)
 static void select_cancel_filp(struct filp *f)
 {
 /* Reduce number of select users of this filp */
+  dev_t major;
 
   assert(f);
-  assert(f->filp_selectors >= 0);
-  if (f->filp_selectors == 0) return;
-  if (f->filp_count == 0) return;
+  assert(f->filp_selectors > 0);
+  assert(f->filp_count > 0);
 
   select_lock_filp(f, f->filp_select_ops);
 
@@ -587,6 +588,17 @@ static void select_cancel_filp(struct filp *f)
 	f->filp_select_ops = 0;
 	f->filp_select_flags = 0;
 	f->filp_pipe_select_ops = 0;
+
+	/* If this filp is the subject of an ongoing select query to a
+	 * character device, mark the query as stale, so that this filp will
+	 * not be checked when the result arrives.
+	 */
+	if (is_supported_major(f)) {
+		major = major(f->filp_vno->v_sdev);
+		if (dmap[major].dmap_sel_busy &&
+			dmap[major].dmap_sel_filp == f)
+			dmap[major].dmap_sel_filp = NULL; /* leave _busy set */
+	}
   }
 
   unlock_filp(f);
@@ -638,24 +650,24 @@ void init_select(void)
 /*===========================================================================*
  *				select_forget			             *
  *===========================================================================*/
-void select_forget(endpoint_t proc_e)
+void select_forget(void)
 {
-/* Something has happened (e.g. signal delivered that interrupts select()).
- * Totally forget about the select(). */
-
+/* The calling thread's associated process is expected to be unpaused, due to
+ * a signal that is supposed to interrupt the current system call.
+ * Totally forget about the select().
+ */
   int slot;
   struct selectentry *se;
 
   for (slot = 0; slot < MAXSELECTS; slot++) {
 	se = &selecttab[slot];
-	if (se->requestor != NULL && se->req_endpt == proc_e)
+	if (se->requestor == fp)
 		break;
   }
 
   if (slot >= MAXSELECTS) return;	/* Entry not found */
-  se->error = EINTR;
-  if (is_deferred(se)) return;		/* Still awaiting initial reply */
 
+  /* Do NOT test on is_deferred here. We can safely cancel ongoing queries. */
   select_cancel_all(se);
 }
 
@@ -743,20 +755,25 @@ int status;
   dev = makedev(major, minor);
 
   /* Get filp belonging to character special file */
-  if ((f = dp->dmap_sel_filp) == NULL) {
+  if (!dp->dmap_sel_busy) {
 	printf("VFS (%s:%d): major %d was not expecting a DEV_SELECT reply\n",
 		__FILE__, __LINE__, major);
 	return;
   }
 
-  /* Is the filp still in use and busy waiting for a reply? The owner might
-   * have vanished before the driver was able to reply. */
-  if (f->filp_count >= 1 && (f->filp_select_flags & FSF_BUSY)) {
+  /* The select filp may have been set to NULL if the requestor has been
+   * unpaused in the meantime. In that case, we ignore the result, but we do
+   * look for other filps to restart later.
+   */
+  if ((f = dp->dmap_sel_filp) != NULL) {
 	/* Find vnode and check we got a reply from the device we expected */
 	vp = f->filp_vno;
 	assert(vp != NULL);
 	assert(S_ISCHR(vp->v_mode));
 	if (vp->v_sdev != dev) {
+		/* This should never happen. The driver may be misbehaving.
+		 * For now we assume that the reply we want will arrive later..
+		 */
 		printf("VFS (%s:%d): expected reply from dev %d not %d\n",
 			__FILE__, __LINE__, vp->v_sdev, dev);
 		return;
@@ -764,12 +781,14 @@ int status;
   }
 
   /* No longer waiting for a reply from this device */
+  dp->dmap_sel_busy = FALSE;
   dp->dmap_sel_filp = NULL;
 
-  /* Process select result only if requestor is still around. That is, the
-   * corresponding filp is still in use.
-   */
-  if (f->filp_count >= 1) {
+  /* Process the select result only if the filp is valid. */
+  if (f != NULL) {
+	assert(f->filp_count >= 1);
+	assert(f->filp_select_flags & FSF_BUSY);
+
 	select_lock_filp(f, f->filp_select_ops);
 	f->filp_select_flags &= ~FSF_BUSY;