]> Zhao Yanbai Git Server - minix.git/commitdiff
make unpause() decrease susp_count, as it shouldn't be decreased
authorBen Gras <ben@minix3.org>
Fri, 8 May 2009 13:56:41 +0000 (13:56 +0000)
committerBen Gras <ben@minix3.org>
Fri, 8 May 2009 13:56:41 +0000 (13:56 +0000)
 if the process was REVIVING. (susp_count doesn't count those
 processes.) this together with dev_io SELECT suspend side effect
 for asynch. character devices solves the hanging pipe bug. or
 at last vastly improves it.

 added sanity checks, turned off by default.

 made the {NOT_,}{SUSPENDING,REVIVING} constants weirder to
 help sanity checking.

servers/vfs/fproc.h
servers/vfs/fs.h
servers/vfs/main.c
servers/vfs/misc.c
servers/vfs/mount.c
servers/vfs/pipe.c
servers/vfs/proto.h
servers/vfs/vnode.c
servers/vfs/vnode.h

index d1052cd93a43124744ac8084b6df2a5fb37ae2f1..7537edf18ab60498f591e3ace52e74621ed5341a 100644 (file)
@@ -25,8 +25,8 @@ EXTERN struct fproc {
   char *fp_buffer;             /* place to save buffer if rd/wr can't finish*/
   int  fp_nbytes;              /* place to save bytes if rd/wr can't finish */
   int  fp_cum_io_partial;      /* partial byte count if rd/wr can't finish */
-  char fp_suspended;           /* set to indicate process hanging */
-  char fp_revived;             /* set to indicate process being revived */
+  int fp_suspended;            /* set to indicate process hanging */
+  int fp_revived;              /* set to indicate process being revived */
   int fp_task;                 /* which task is proc suspended on */
   
   endpoint_t fp_ioproc;                /* proc no. in suspended-on i/o message */
@@ -47,10 +47,12 @@ EXTERN struct fproc {
                                 */
 
 /* Field values. */
-#define NOT_SUSPENDED      0   /* process is not suspended on pipe or task */
-#define SUSPENDED          1   /* process is suspended on pipe or task */
-#define NOT_REVIVING       0   /* process is not being revived */
-#define REVIVING           1   /* process is being revived from suspension */
+/* fp_suspended is one of these. */
+#define NOT_SUSPENDED      0xC0FFEE    /* process is not suspended on pipe or task */
+#define SUSPENDED          0xDEAD      /* process is suspended on pipe or task */
+
+#define NOT_REVIVING       0xC0FFEEE   /* process is not being revived */
+#define REVIVING           0xDEEAD     /* process is being revived from suspension */
 #define PID_FREE          0    /* process slot free */
 
 /* Check is process number is acceptable - includes system processes. */
index 7d032834026e4af98fd40af52803233fab003097..9bd2058ace4751c704422d47e4aefeceaa74f97b 100644 (file)
@@ -5,7 +5,19 @@
 #define _MINIX             1   /* tell headers to include MINIX stuff */
 #define _SYSTEM            1   /* tell headers that this is the kernel */
 
-#define VERBOSE                   0    /* show messages during initialization? */
+#define DO_SANITYCHECKS           0
+
+#if DO_SANITYCHECKS
+#define SANITYCHECK do {                       \
+       if(!check_vrefs() || !check_pipe()) {                           \
+          printf("VFS:%s:%d: call_nr %d who_e %d\n", \
+                       __FILE__, __LINE__, call_nr, who_e);    \
+          panic(__FILE__, "sanity check failed", NO_NUM);      \
+       }                                                       \
+} while(0)
+#else
+#define SANITYCHECK
+#endif
 
 /* The following are so basic, all the *.c files get them automatically. */
 #include <minix/config.h>      /* MUST be first */
index 024f4e92acab9bf493637b9edf8ea1a8b48fa454..43912b6cce1b7fd4d1a47a64d02bb2ab1857b809 100644 (file)
@@ -57,8 +57,11 @@ PUBLIC int main()
 
   fs_init();
 
+  SANITYCHECK;
+
   /* This is the main loop that gets work, processes it, and sends replies. */
   while (TRUE) {
+       SANITYCHECK;
        get_work();             /* sets who and call_nr */
 
        if (who_e == PM_PROC_NR && call_nr != PROC_EVENT)
@@ -121,14 +124,7 @@ PUBLIC int main()
                        /* Device notifies us of an event. */
                        dev_status(&m_in);
                }
-#if 0
-               if (!check_vrefs())
-               {
-                       printf("after call %d from %d/%d\n",
-                               call_nr, who_p, who_e);
-                       panic(__FILE__, "check_vrefs failed at line", __LINE__);
-               }
-#endif
+               SANITYCHECK;
                continue;
        }
 
@@ -143,6 +139,14 @@ PUBLIC int main()
        fp = &fproc[who_p];     /* pointer to proc table struct */
        super_user = (fp->fp_effuid == SU_UID ? TRUE : FALSE);   /* su? */
 
+#if DO_SANITYCHECKS
+       if(fp->fp_suspended != NOT_SUSPENDED) {
+               printf("VFS: requester %d call %d: not not suspended\n",
+                       who_e, call_nr);
+               panic(__FILE__, "requester suspended", NO_NUM);
+       }
+#endif
+
        /* Calls from VM. */
        if(who_e == VM_PROC_NR) {
            int caught = 1;
@@ -167,6 +171,8 @@ PUBLIC int main()
           }
        }
 
+               SANITYCHECK;
+
          /* Other calls. */
          switch(call_nr)
          {
@@ -196,21 +202,15 @@ PUBLIC int main()
 #if ENABLE_SYSCALL_STATS
                        calls_stats[call_nr]++;
 #endif
+                       SANITYCHECK;
                        error = (*call_vec[call_nr])();
+                       SANITYCHECK;
                }
 
                /* Copy the results back to the user and send reply. */
                if (error != SUSPEND) { reply(who_e, error); }
        }
-#if 0
-       if (!check_vrefs())
-       {
-               printf("after call %d from %d/%d\n", call_nr, who_p, who_e);
-               panic(__FILE__, "check_vrefs failed at line", __LINE__);
-       }
-#endif
-       
-       
+       SANITYCHECK;
   }
   return(OK);                          /* shouldn't come here */
 }
@@ -352,6 +352,8 @@ PRIVATE void fs_init()
        rfp->fp_effgid = (gid_t) SYS_GID;
        rfp->fp_umask = ~0;
        rfp->fp_grant = GRANT_INVALID;
+       rfp->fp_suspended = NOT_SUSPENDED;
+       rfp->fp_revived = NOT_REVIVING;
    
   } while (TRUE);                      /* continue until process NONE */
   mess.m_type = OK;                    /* tell PM that we succeeded */
index 54f54d8e390555fb59462c61ec7649d95bbad64d..effe965508b856442f0efa784a69c4132944162f 100644 (file)
@@ -312,12 +312,12 @@ void unmount_all(void)
        for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; vmp++) {
                if (vmp->m_dev != NO_DEV) {
                        found++;
-                       CHECK_VREFS;
+                       SANITYCHECK;
                        if(unmount(vmp->m_dev) == OK)
                                worked++;
                        else
                                remain++;
-                       CHECK_VREFS;
+                       SANITYCHECK;
                }
        }
   }
@@ -335,7 +335,7 @@ PUBLIC void pm_reboot()
 
   do_sync();
 
-  CHECK_VREFS;
+  SANITYCHECK;
 
   /* Do exit processing for all leftover processes and servers,
    * but don't actually exit them (if they were really gone, PM
@@ -350,11 +350,11 @@ PUBLIC void pm_reboot()
                 */
                free_proc(&fproc[i], 0);
        }
-  CHECK_VREFS;
+  SANITYCHECK;
 
   unmount_all();
 
-  CHECK_VREFS;
+  SANITYCHECK;
 
 }
 
@@ -436,6 +436,8 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
   register struct vnode *vp;
   dev_t dev;
 
+ SANITYCHECK;
+
   fp = exiter;         /* get_filp() needs 'fp' */
 
   if(fp->fp_endpoint == NONE) {
@@ -443,12 +445,14 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
   }
 
   if (fp->fp_suspended == SUSPENDED) {
+       SANITYCHECK;
        task = -fp->fp_task;
-       if (task == XPIPE || task == XPOPEN) susp_count--;
        unpause(fp->fp_endpoint);
-       fp->fp_suspended = NOT_SUSPENDED;
+       SANITYCHECK;
   }
 
+ SANITYCHECK;
+
   /* Loop on file descriptors, closing any that are open. */
   for (i = 0; i < OPEN_MAX; i++) {
        (void) close_fd(fp, i);
@@ -465,13 +469,13 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
   if(fp->fp_rd) { put_vnode(fp->fp_rd); fp->fp_rd = NIL_VNODE; }
   if(fp->fp_wd) { put_vnode(fp->fp_wd); fp->fp_wd = NIL_VNODE; }
 
- CHECK_VREFS;
-
   /* The rest of these actions is only done when processes actually
    * exit.
    */
-  if(!(flags & FP_EXITING))
+  if(!(flags & FP_EXITING)) {
+       SANITYCHECK;
        return;
+  }
 
   /* Invalidate endpoint number for error and sanity checks. */
   fp->fp_endpoint = NONE;
@@ -504,6 +508,8 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
 
   /* Exit done. Mark slot as free. */
   fp->fp_pid = PID_FREE;
+
+  SANITYCHECK;
 }
 
 /*===========================================================================*
index 871c67266e464d44860a4e0a7ee0c4bbadc6c2d9..d337a4b3f3e0703584eb3b4b447f1c809addb99c 100644 (file)
@@ -71,7 +71,7 @@ PUBLIC int do_mount()
 {
   endpoint_t fs_e; 
 
-  CHECK_VREFS;
+  SANITYCHECK;
 
   /* Only the super-user may do MOUNT. */
   if (!super_user) return(EPERM);
@@ -107,7 +107,7 @@ PRIVATE int mount_fs(endpoint_t fs_e)
   char *label;
   struct node_details res;
 
-  CHECK_VREFS;
+  SANITYCHECK;
   
   /* Only the super-user may do MOUNT. */
   if (!super_user) return(EPERM);
@@ -337,7 +337,7 @@ PRIVATE int mount_fs(endpoint_t fs_e)
          if(tfp->fp_wd) MAKEROOT(tfp->fp_wd);
       }
 
-       CHECK_VREFS;
+       SANITYCHECK;
 
       return(OK);
   }
@@ -371,7 +371,7 @@ PRIVATE int mount_fs(endpoint_t fs_e)
       printf("VFSmount: moving opened block spec to new FS_e: %d...\n", fs_e);
       bspec->v_bfs_e = fs_e; 
   }
-       CHECK_VREFS;
+       SANITYCHECK;
   return(OK);
 }
 
@@ -382,17 +382,17 @@ PUBLIC int do_umount()
 {
 /* Perform the umount(name) system call. */
   dev_t dev;
-       CHECK_VREFS;
+       SANITYCHECK;
 
   /* Only the super-user may do UMOUNT. */
   if (!super_user) return(EPERM);
-       CHECK_VREFS;
+       SANITYCHECK;
 
   /* If 'name' is not for a block special file, return error. */
   if (fetch_name(m_in.name, m_in.name_length, M3) != OK) return(err_code);
-       CHECK_VREFS;
+       SANITYCHECK;
   if ( (dev = name_to_dev()) == NO_DEV) return(err_code);
-       CHECK_VREFS;
+       SANITYCHECK;
 
   return(unmount(dev));
 }
@@ -410,7 +410,7 @@ Dev_t dev;
   int count, r;
   int fs_e;
   
-       CHECK_VREFS;
+       SANITYCHECK;
 
   /* Find vmnt */
   for (vmp_i = &vmnt[0]; vmp_i < &vmnt[NR_MNTS]; ++vmp_i) {
@@ -472,16 +472,16 @@ Dev_t dev;
           count += vp->v_ref_count;
       }
   }
-       CHECK_VREFS;
+       SANITYCHECK;
 
   if (count > 1) {
       return(EBUSY);    /* can't umount a busy file system */
   }
 
-       CHECK_VREFS;
+       SANITYCHECK;
 
   vnode_clean_refs(vmp->m_root_node);
-       CHECK_VREFS;
+       SANITYCHECK;
 
   if (vmp->m_mounted_on) {
       put_vnode(vmp->m_mounted_on);
@@ -504,7 +504,7 @@ Dev_t dev;
   vmp->m_dev = NO_DEV;
   vmp->m_fs_e = NONE;
 
-       CHECK_VREFS;
+       SANITYCHECK;
 
   /* Is there a block special file that was handled by that partition? */
   for (vp = &vnode[0]; vp < &vnode[NR_VNODES]; vp++) {
@@ -531,7 +531,7 @@ Dev_t dev;
       }
   }
 
-       CHECK_VREFS;
+       SANITYCHECK;
 
   return(OK);
 }
index 4e3e4dac7d1d6d5f0d7405a62d34af20bb4f7308..f80f028321c74a024f3179e6d5246f774b09ac99 100644 (file)
@@ -240,9 +240,14 @@ int task;                  /* who is proc waiting for? (PIPE = pipe) */
  * The SUSPEND pseudo error should be returned after calling suspend().
  */
 
+#if DO_SANITYCHECKS
   if (task == XPIPE)
        panic(__FILE__, "suspend: called for XPIPE", NO_NUM);
 
+  if(fp->fp_suspended == SUSPENDED)
+       panic(__FILE__, "suspend: called for suspended process", NO_NUM);
+#endif
+
   if (task == XPOPEN) susp_count++;/* #procs susp'ed on pipe*/
   fp->fp_suspended = SUSPENDED;
   assert(fp->fp_grant == GRANT_INVALID || !GRANT_VALID(fp->fp_grant));
@@ -277,6 +282,10 @@ size_t size;
  * but they are needed for pipes, and it is not worth making the distinction.)
  * The SUSPEND pseudo error should be returned after calling suspend().
  */
+#if DO_SANITYCHECKS
+  if(fp->fp_suspended == SUSPENDED)
+       panic(__FILE__, "pipe_suspend: called for suspended process", NO_NUM);
+#endif
 
   susp_count++;                                        /* #procs susp'ed on pipe*/
   fp->fp_suspended = SUSPENDED;
@@ -463,6 +472,7 @@ int proc_nr_e;
   struct filp *f;
   dev_t dev;
   message mess;
+  int wasreviving = 0;
 
   if(isokendpt(proc_nr_e, &proc_nr_p) != OK) {
        printf("VFS: ignoring unpause for bogus endpoint %d\n", proc_nr_e);
@@ -478,8 +488,10 @@ int proc_nr_e;
   {
        rfp->fp_revived = NOT_REVIVING;
        reviving--;
+       wasreviving = 1;
   }
 
+
   switch (task) {
        case XPIPE:             /* process trying to read or write a pipe */
                break;
@@ -540,6 +552,11 @@ int proc_nr_e;
   }
 
   rfp->fp_suspended = NOT_SUSPENDED;
+
+  if ((task == XPIPE || task == XPOPEN) && !wasreviving) {
+       susp_count--;
+  }
+
   reply(proc_nr_e, status);    /* signal interrupted call */
 }
 
@@ -587,6 +604,34 @@ PUBLIC int select_match_pipe(struct filp *f)
        return 0;
 }
 
+#if DO_SANITYCHECKS
+/*===========================================================================*
+ *                             check_pipe                           *
+ *===========================================================================*/
+PUBLIC int check_pipe(void)
+{
+       struct fproc *rfp;
+       int mycount = 0;
+        for (rfp=&fproc[0]; rfp < &fproc[NR_PROCS]; rfp++) {
+                if (rfp->fp_pid == PID_FREE)
+                        continue;
+               if(rfp->fp_suspended != SUSPENDED &&
+                       rfp->fp_suspended != NOT_SUSPENDED) {
+                       printf("check_pipe: %d invalid suspended value 0x%x\n",
+                               rfp->fp_endpoint, rfp->fp_suspended);
+                       return 0;
+               }
+               if(rfp->fp_suspended == SUSPENDED && rfp->fp_revived != REVIVING && (-rfp->fp_task == XPIPE || -rfp->fp_task == XPOPEN)) {
+                       mycount++;
+               }
+        }
 
+       if(mycount != susp_count) {
+               printf("check_pipe: mycount %d susp_count %d\n",
+                       mycount, susp_count);
+               return 0;
+       }
 
-
+       return 1;
+}
+#endif
index 7ad8100942d329b010b09fa901c2fc28503eeac3..d4d0a19a54efa4b02e38ff56321c067b79a0aa8a 100644 (file)
@@ -138,6 +138,9 @@ _PROTOTYPE( int select_match_pipe, (struct filp *f)                 );
 _PROTOTYPE( void unsuspend_by_endpt, (int)                             );
 _PROTOTYPE( void select_reply1, (void)                                 );
 _PROTOTYPE( void select_reply2, (void)                                 );
+#if DO_SANITYCHECKS
+_PROTOTYPE( int check_pipe, (void)                                     );
+#endif
 
 /* protect.c */
 _PROTOTYPE( int do_access, (void)                                      );
@@ -251,15 +254,9 @@ _PROTOTYPE( struct vnode *find_vnode, (int fs_e, int numb)              );
 _PROTOTYPE( void dup_vnode, (struct vnode *vp)                          );
 _PROTOTYPE( void put_vnode, (struct vnode *vp)                          );
 _PROTOTYPE( void vnode_clean_refs, (struct vnode *vp)                   );
-#if 0
-_PROTOTYPE( struct vnode *get_vnode, (int fs_e, int inode_nr)           );
-_PROTOTYPE( struct vnode *get_vnode_x, (int fs_e, int inode_nr)                );
-_PROTOTYPE( void mark_vn, (struct vnode *vp, char *file, int line)     );
-#endif
-#define CHECK_VREFS do { if(!check_vrefs()) { \
-       printf("VFS:%s:%d: check_vrefs failed\n", __FILE__, __LINE__);  \
-       panic("VFS", "check_vrefs failed", NO_NUM);} } while(0)
+#if DO_SANITYCHECKS
 _PROTOTYPE( int check_vrefs, (void)                    );
+#endif
 
 /* write.c */
 _PROTOTYPE( int do_write, (void)                                       );
index 286f3689a5e3e6d2ed784bb14caae9266fe120a2..8c6da370609ce35f2448d54ef71481882a272382 100644 (file)
@@ -169,6 +169,7 @@ int line;
 
 #define REFVP(v) { vp = (v); CHECKVN(v); vp->v_ref_check++; }
 
+#if DO_SANITYCHECKS
 /*===========================================================================*
  *                             check_vrefs                                  *
  *===========================================================================*/
@@ -245,3 +246,4 @@ PUBLIC int check_vrefs()
        }
        return !bad;
 }
+#endif
index dddd98480fac638b74380376239cbb7c75d60289..e514413beb361153b26c35eaa0eb3e05df4ea38c 100644 (file)
@@ -38,5 +38,3 @@ EXTERN struct vnode {
 #define NO_SEEK            0   /* i_seek = NO_SEEK if last op was not SEEK */
 #define ISEEK              1   /* i_seek = ISEEK if last op was SEEK */
 
-
-#define SANITYCHECK do { if(!check_vrefs()) { printf("VFS:%s:%d: vref check failed\n", __FILE__, __LINE__); util_stacktrace(); } } while(0)