]> Zhao Yanbai Git Server - minix.git/commitdiff
VFS: properly cancel select queries on unpause 49/949/2
authorDavid van Moolenbroek <david@minix3.org>
Sat, 24 Aug 2013 22:26:38 +0000 (00:26 +0200)
committerLionel Sambuc <lionel@minix3.org>
Tue, 18 Feb 2014 10:25:03 +0000 (11:25 +0100)
Change-Id: I16e71db3f5c1bcc7ba6045bc9f02b13d71dc31eb

servers/vfs/dmap.c
servers/vfs/dmap.h
servers/vfs/pipe.c
servers/vfs/proto.h
servers/vfs/select.c

index c6491fde8465f58ad8861d660714dba74be77d1f..947f638f3ae48bae57de3711f299a5ff9300c09e 100644 (file)
@@ -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;
+  }
 }
 
 /*===========================================================================*
index cd15a1cc43d5262769fc4bc368b3107de31d17b4..4520d577fd4e4568a13d507d0727c315fd7a1e25 100644 (file)
@@ -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;
index 50f6ddb3e368f82ce03f10b9e1c4ea3b985d9d72..f46111496a4067a469a783365c5fc23bf16a5724 100644 (file)
@@ -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 */
index 8f1d16089987b9456331d9ad758ceba6fc0bfaea..300c6dc6c8126977a5a4b2e3f3bd06e82c1ba483 100644 (file)
@@ -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 *);
index 81f20e0769dbcb867b1daedd5caa0b4c12b316ba..46f769177542cf19aaac9fbe34bbd98c0b4e99b0 100644 (file)
@@ -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;