From: Ben Gras Date: Fri, 8 May 2009 13:56:41 +0000 (+0000) Subject: make unpause() decrease susp_count, as it shouldn't be decreased X-Git-Tag: v3.1.4~53 X-Git-Url: http://zhaoyanbai.com/repos/doc/%20%22?a=commitdiff_plain;h=dc1238b7b9c92348523d07ab96bd43bfd2848075;p=minix.git make unpause() decrease susp_count, as it shouldn't be decreased 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. --- diff --git a/servers/vfs/fproc.h b/servers/vfs/fproc.h index d1052cd93..7537edf18 100644 --- a/servers/vfs/fproc.h +++ b/servers/vfs/fproc.h @@ -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. */ diff --git a/servers/vfs/fs.h b/servers/vfs/fs.h index 7d0328340..9bd2058ac 100644 --- a/servers/vfs/fs.h +++ b/servers/vfs/fs.h @@ -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 /* MUST be first */ diff --git a/servers/vfs/main.c b/servers/vfs/main.c index 024f4e92a..43912b6cc 100644 --- a/servers/vfs/main.c +++ b/servers/vfs/main.c @@ -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 */ diff --git a/servers/vfs/misc.c b/servers/vfs/misc.c index 54f54d8e3..effe96550 100644 --- a/servers/vfs/misc.c +++ b/servers/vfs/misc.c @@ -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; } /*===========================================================================* diff --git a/servers/vfs/mount.c b/servers/vfs/mount.c index 871c67266..d337a4b3f 100644 --- a/servers/vfs/mount.c +++ b/servers/vfs/mount.c @@ -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); } diff --git a/servers/vfs/pipe.c b/servers/vfs/pipe.c index 4e3e4dac7..f80f02832 100644 --- a/servers/vfs/pipe.c +++ b/servers/vfs/pipe.c @@ -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 diff --git a/servers/vfs/proto.h b/servers/vfs/proto.h index 7ad810094..d4d0a19a5 100644 --- a/servers/vfs/proto.h +++ b/servers/vfs/proto.h @@ -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) ); diff --git a/servers/vfs/vnode.c b/servers/vfs/vnode.c index 286f3689a..8c6da3706 100644 --- a/servers/vfs/vnode.c +++ b/servers/vfs/vnode.c @@ -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 diff --git a/servers/vfs/vnode.h b/servers/vfs/vnode.h index dddd98480..e514413be 100644 --- a/servers/vfs/vnode.h +++ b/servers/vfs/vnode.h @@ -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)