VFS: various select fixes

- Fix locking bug when unable to send DEV_SELECT request. Upon failure
  VFS tried to cancel the select operation, but this failed due to trying
  to lock a filp that was already locked to send the request in the first
  place. Do_select_request now handles locking of filps itself instead of
  relying on the caller to do it.  This fixes a crash when killing INET.
- Fix failure to revive a process after a non-blocking select operation
  yielded no ready select operations when replying DEV_SEL_REPL1.
- Improve readability by using OK, SUSPEND, and standard error values as
  results instead of having separate macros in select.
- Don't print not having a driver for a major device; after killing a driver
  select will trigger this printf.

diff --git a/servers/vfs/device.c b/servers/vfs/device.c
index 926cf322a00e22a1e2eef71cba563d27b4816ddf..4700701b1be7ab843230e2c7a42c2781f25a3178 100644 (file)
--- a/servers/vfs/device.c
+++ b/servers/vfs/device.c
@@ -394,10 +394,7 @@ PUBLIC int dev_io(
   dp = &dmap[major_dev];
 
   /* See if driver is roughly valid. */
-  if (dp->dmap_driver == NONE) {
-       printf("VFS: dev_io: no driver for major %d\n", major_dev);
-       return(ENXIO);
-  }
+  if (dp->dmap_driver == NONE) return(ENXIO);
 
   if (suspend_reopen) {
        /* Suspend user. */

diff --git a/servers/vfs/pipe.c b/servers/vfs/pipe.c
index 0a6d6f49d528136026d2fabc15ee3b5672fc3288..b412cf4633e791fb2fb486f83634bedb31e7bbf3 100644 (file)
--- a/servers/vfs/pipe.c
+++ b/servers/vfs/pipe.c
@@ -30,7 +30,6 @@
 #include "scratchpad.h"
 #include "dmap.h"
 #include "param.h"
-#include "select.h"
 #include <minix/vfsif.h>
 #include "vnode.h"
 #include "vmnt.h"

diff --git a/servers/vfs/select.c b/servers/vfs/select.c
index 90e23ab0608f6ed304d417b96b645718eeb060bc..c2d729b9cc543c73d3fd2e9380c6373112029ec0 100644 (file)
--- a/servers/vfs/select.c
+++ b/servers/vfs/select.c
@@ -14,7 +14,6 @@
 #include <string.h>
 #include <assert.h>
 
-#include "select.h"
 #include "file.h"
 #include "fproc.h"
 #include "dmap.h"
@@ -211,15 +210,13 @@ PUBLIC int do_select(void)
        /* Test filp for select operations if not already done so. e.g.,
         * processes sharing a filp and both doing a select on that filp. */
        f = se->filps[fd];
-       select_lock_filp(f, f->filp_select_ops | ops);
        if ((f->filp_select_ops & ops) != ops) {
                int wantops;
 
                wantops = (f->filp_select_ops |= ops);
                r = do_select_request(se, fd, &wantops);
-               unlock_filp(f);
-               if (r != SEL_OK) {
-                       if (r == SEL_DEFERRED) continue;
+               if (r != OK) {
+                       if (r == SUSPEND) continue;
                        else break; /* Error or bogus return code; abort */
                }
 
@@ -228,8 +225,6 @@ PUBLIC int do_select(void)
                 * Either way, we might have a result and we need to store them
                 * in the select table entry. */
                if (wantops & ops) ops2tab(wantops, fd, se);
-       } else {
-               unlock_filp(f);
        }
   }
 
@@ -358,7 +353,7 @@ PRIVATE int select_request_async(struct filp *f, int *ops, int block)
                if (!(rops & (SEL_RD|SEL_WR|SEL_ERR))) {
                        /* Nothing left to do */
                        *ops = 0;
-                       return(SEL_OK);
+                       return(OK);
                }
        }
   }
@@ -372,19 +367,19 @@ PRIVATE int select_request_async(struct filp *f, int *ops, int block)
   }
 
   if (f->filp_select_flags & FSF_BUSY)
-       return(SEL_DEFERRED);
+       return(SUSPEND);
 
   major = major(f->filp_vno->v_sdev);
-  if (major < 0 || major >= NR_DEVICES) return(SEL_ERROR);
+  if (major < 0 || major >= NR_DEVICES) return(ENXIO);
   dp = &dmap[major];
   if (dp->dmap_sel_filp)
-       return(SEL_DEFERRED);
+       return(SUSPEND);
 
   f->filp_select_flags &= ~FSF_UPDATE;
   r = dev_io(VFS_DEV_SELECT, f->filp_vno->v_sdev, rops, NULL,
             cvu64(0), 0, 0, FALSE);
   if (r < 0 && r != SUSPEND)
-       return(SEL_ERROR);
+       return(r);
 
   if (r != SUSPEND)
        panic("select_request_asynch: expected SUSPEND got: %d", r);
@@ -392,7 +387,7 @@ PRIVATE int select_request_async(struct filp *f, int *ops, int block)
   dp->dmap_sel_filp = f;
   f->filp_select_flags |= FSF_BUSY;
 
-  return(SEL_DEFERRED);
+  return(SUSPEND);
 }
 
 /*===========================================================================*
@@ -402,7 +397,7 @@ PRIVATE int select_request_file(struct filp *UNUSED(f), int *UNUSED(ops),
   int UNUSED(block))
 {
   /* Files are always ready, so output *ops is input *ops */
-  return(SEL_OK);
+  return(OK);
 }
 
 /*===========================================================================*
@@ -413,7 +408,7 @@ PRIVATE int select_request_major(struct filp *f, int *ops, int block)
   int major, r;
 
   major = major(f->filp_vno->v_sdev);
-  if (major < 0 || major >= NR_DEVICES) return(SEL_ERROR);
+  if (major < 0 || major >= NR_DEVICES) return(ENXIO);
 
   if (dmap[major].dmap_style == STYLE_DEVA ||
       dmap[major].dmap_style == STYLE_CLONE_A)
@@ -436,9 +431,9 @@ PRIVATE int select_request_sync(struct filp *f, int *ops, int block)
   *ops = dev_io(VFS_DEV_SELECT, f->filp_vno->v_sdev, rops, NULL,
                cvu64(0), 0, 0, FALSE);
   if (*ops < 0)
-       return(SEL_ERROR);
+       return(*ops);
 
-  return(SEL_OK);
+  return(OK);
 }
 
 /*===========================================================================*
@@ -486,7 +481,7 @@ PRIVATE int select_request_pipe(struct filp *f, int *ops, int block)
   if (!*ops && block)
        f->filp_pipe_select_ops |= orig_ops;
 
-  return(SEL_OK);
+  return(OK);
 }
 
 /*===========================================================================*
@@ -829,7 +824,7 @@ int status;
                /* there may be operations pending */
                f->filp_select_ops &= ~status;
 
-       /* Tell filp owners about result unless we need to wait longer */
+       /* Record new filp status */
        if (!(status == 0 && (f->filp_select_flags & FSF_BLOCKED))) {
                if (status > 0) {       /* operations ready */
                        if (status & SEL_RD)
@@ -842,12 +837,10 @@ int status;
                        /* Always unblock upon error */
                        f->filp_select_flags &= ~FSF_BLOCKED;
                }
-
-               unlock_filp(f);
-               filp_status(f, status); /* Tell filp owners about the results */
-       } else {
-               unlock_filp(f);
        }
+
+       unlock_filp(f);
+       filp_status(f, status); /* Tell filp owners about the results */
   }
 
   select_restart_filps();
@@ -956,13 +949,11 @@ PRIVATE void select_restart_filps()
                        continue;                         /* 'update' state */
 
                wantops = ops = f->filp_select_ops;
-               select_lock_filp(f, ops);
                vp = f->filp_vno;
                assert((vp->v_mode & I_TYPE) == I_CHAR_SPECIAL);
                r = do_select_request(se, fd, &wantops);
-               unlock_filp(f);
-               if (r != SEL_OK) {
-                       if (r == SEL_DEFERRED) continue;
+               if (r != OK) {
+                       if (r == SUSPEND) continue;
                        else break; /* Error or bogus return code; abort */
                }
                if (wantops & ops) ops2tab(wantops, fd, se);
@@ -985,8 +976,10 @@ int *ops;
 
   type = se->type[fd];
   f = se->filps[fd];
+  select_lock_filp(f, *ops);
   r = fdtypes[type].select_request(f, ops, se->block);
-  if (r != SEL_OK && r != SEL_DEFERRED) {
+  unlock_filp(f);
+  if (r != OK && r != SUSPEND) {
        se->error = EINTR;
        se->block = 0;  /* Stop blocking to return asap */
        if (!is_deferred(se)) select_cancel_all(se);

diff --git a/servers/vfs/select.h b/servers/vfs/select.h
deleted file mode 100644 (file)
index 5215b1a..0000000
--- a/servers/vfs/select.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __VFS_SELECT_H__
-#define __VFS_SELECT_H__
-
-/* return codes for select_request_* and select_cancel_* */
-#define SEL_OK         0       /* ready */
-#define SEL_ERROR      1       /* failed */
-#define SEL_DEFERRED   2       /* request is sent to driver */
-
-#endif