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.
This commit is contained in:
Ben Gras 2009-05-08 13:56:41 +00:00
parent 113b1ec5f3
commit dc1238b7b9
9 changed files with 123 additions and 59 deletions

View File

@ -25,8 +25,8 @@ EXTERN struct fproc {
char *fp_buffer; /* place to save buffer if rd/wr can't finish*/ 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_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 */ int fp_cum_io_partial; /* partial byte count if rd/wr can't finish */
char fp_suspended; /* set to indicate process hanging */ int fp_suspended; /* set to indicate process hanging */
char fp_revived; /* set to indicate process being revived */ int fp_revived; /* set to indicate process being revived */
int fp_task; /* which task is proc suspended on */ int fp_task; /* which task is proc suspended on */
endpoint_t fp_ioproc; /* proc no. in suspended-on i/o message */ endpoint_t fp_ioproc; /* proc no. in suspended-on i/o message */
@ -47,10 +47,12 @@ EXTERN struct fproc {
*/ */
/* Field values. */ /* Field values. */
#define NOT_SUSPENDED 0 /* process is not suspended on pipe or task */ /* fp_suspended is one of these. */
#define SUSPENDED 1 /* process is suspended on pipe or task */ #define NOT_SUSPENDED 0xC0FFEE /* process is not suspended on pipe or task */
#define NOT_REVIVING 0 /* process is not being revived */ #define SUSPENDED 0xDEAD /* process is suspended on pipe or task */
#define REVIVING 1 /* process is being revived from suspension */
#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 */ #define PID_FREE 0 /* process slot free */
/* Check is process number is acceptable - includes system processes. */ /* Check is process number is acceptable - includes system processes. */

View File

@ -5,7 +5,19 @@
#define _MINIX 1 /* tell headers to include MINIX stuff */ #define _MINIX 1 /* tell headers to include MINIX stuff */
#define _SYSTEM 1 /* tell headers that this is the kernel */ #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. */ /* The following are so basic, all the *.c files get them automatically. */
#include <minix/config.h> /* MUST be first */ #include <minix/config.h> /* MUST be first */

View File

@ -57,8 +57,11 @@ PUBLIC int main()
fs_init(); fs_init();
SANITYCHECK;
/* This is the main loop that gets work, processes it, and sends replies. */ /* This is the main loop that gets work, processes it, and sends replies. */
while (TRUE) { while (TRUE) {
SANITYCHECK;
get_work(); /* sets who and call_nr */ get_work(); /* sets who and call_nr */
if (who_e == PM_PROC_NR && call_nr != PROC_EVENT) if (who_e == PM_PROC_NR && call_nr != PROC_EVENT)
@ -121,14 +124,7 @@ PUBLIC int main()
/* Device notifies us of an event. */ /* Device notifies us of an event. */
dev_status(&m_in); dev_status(&m_in);
} }
#if 0 SANITYCHECK;
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
continue; continue;
} }
@ -143,6 +139,14 @@ PUBLIC int main()
fp = &fproc[who_p]; /* pointer to proc table struct */ fp = &fproc[who_p]; /* pointer to proc table struct */
super_user = (fp->fp_effuid == SU_UID ? TRUE : FALSE); /* su? */ 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. */ /* Calls from VM. */
if(who_e == VM_PROC_NR) { if(who_e == VM_PROC_NR) {
int caught = 1; int caught = 1;
@ -167,6 +171,8 @@ PUBLIC int main()
} }
} }
SANITYCHECK;
/* Other calls. */ /* Other calls. */
switch(call_nr) switch(call_nr)
{ {
@ -196,21 +202,15 @@ PUBLIC int main()
#if ENABLE_SYSCALL_STATS #if ENABLE_SYSCALL_STATS
calls_stats[call_nr]++; calls_stats[call_nr]++;
#endif #endif
SANITYCHECK;
error = (*call_vec[call_nr])(); error = (*call_vec[call_nr])();
SANITYCHECK;
} }
/* Copy the results back to the user and send reply. */ /* Copy the results back to the user and send reply. */
if (error != SUSPEND) { reply(who_e, error); } if (error != SUSPEND) { reply(who_e, error); }
} }
#if 0 SANITYCHECK;
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
} }
return(OK); /* shouldn't come here */ return(OK); /* shouldn't come here */
} }
@ -352,6 +352,8 @@ PRIVATE void fs_init()
rfp->fp_effgid = (gid_t) SYS_GID; rfp->fp_effgid = (gid_t) SYS_GID;
rfp->fp_umask = ~0; rfp->fp_umask = ~0;
rfp->fp_grant = GRANT_INVALID; rfp->fp_grant = GRANT_INVALID;
rfp->fp_suspended = NOT_SUSPENDED;
rfp->fp_revived = NOT_REVIVING;
} while (TRUE); /* continue until process NONE */ } while (TRUE); /* continue until process NONE */
mess.m_type = OK; /* tell PM that we succeeded */ mess.m_type = OK; /* tell PM that we succeeded */

View File

@ -312,12 +312,12 @@ void unmount_all(void)
for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; vmp++) { for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; vmp++) {
if (vmp->m_dev != NO_DEV) { if (vmp->m_dev != NO_DEV) {
found++; found++;
CHECK_VREFS; SANITYCHECK;
if(unmount(vmp->m_dev) == OK) if(unmount(vmp->m_dev) == OK)
worked++; worked++;
else else
remain++; remain++;
CHECK_VREFS; SANITYCHECK;
} }
} }
} }
@ -335,7 +335,7 @@ PUBLIC void pm_reboot()
do_sync(); do_sync();
CHECK_VREFS; SANITYCHECK;
/* Do exit processing for all leftover processes and servers, /* Do exit processing for all leftover processes and servers,
* but don't actually exit them (if they were really gone, PM * 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); free_proc(&fproc[i], 0);
} }
CHECK_VREFS; SANITYCHECK;
unmount_all(); unmount_all();
CHECK_VREFS; SANITYCHECK;
} }
@ -436,6 +436,8 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
register struct vnode *vp; register struct vnode *vp;
dev_t dev; dev_t dev;
SANITYCHECK;
fp = exiter; /* get_filp() needs 'fp' */ fp = exiter; /* get_filp() needs 'fp' */
if(fp->fp_endpoint == NONE) { if(fp->fp_endpoint == NONE) {
@ -443,12 +445,14 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
} }
if (fp->fp_suspended == SUSPENDED) { if (fp->fp_suspended == SUSPENDED) {
SANITYCHECK;
task = -fp->fp_task; task = -fp->fp_task;
if (task == XPIPE || task == XPOPEN) susp_count--;
unpause(fp->fp_endpoint); unpause(fp->fp_endpoint);
fp->fp_suspended = NOT_SUSPENDED; SANITYCHECK;
} }
SANITYCHECK;
/* Loop on file descriptors, closing any that are open. */ /* Loop on file descriptors, closing any that are open. */
for (i = 0; i < OPEN_MAX; i++) { for (i = 0; i < OPEN_MAX; i++) {
(void) close_fd(fp, 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_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; } 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 /* The rest of these actions is only done when processes actually
* exit. * exit.
*/ */
if(!(flags & FP_EXITING)) if(!(flags & FP_EXITING)) {
SANITYCHECK;
return; return;
}
/* Invalidate endpoint number for error and sanity checks. */ /* Invalidate endpoint number for error and sanity checks. */
fp->fp_endpoint = NONE; fp->fp_endpoint = NONE;
@ -504,6 +508,8 @@ PRIVATE void free_proc(struct fproc *exiter, int flags)
/* Exit done. Mark slot as free. */ /* Exit done. Mark slot as free. */
fp->fp_pid = PID_FREE; fp->fp_pid = PID_FREE;
SANITYCHECK;
} }
/*===========================================================================* /*===========================================================================*

View File

@ -71,7 +71,7 @@ PUBLIC int do_mount()
{ {
endpoint_t fs_e; endpoint_t fs_e;
CHECK_VREFS; SANITYCHECK;
/* Only the super-user may do MOUNT. */ /* Only the super-user may do MOUNT. */
if (!super_user) return(EPERM); if (!super_user) return(EPERM);
@ -107,7 +107,7 @@ PRIVATE int mount_fs(endpoint_t fs_e)
char *label; char *label;
struct node_details res; struct node_details res;
CHECK_VREFS; SANITYCHECK;
/* Only the super-user may do MOUNT. */ /* Only the super-user may do MOUNT. */
if (!super_user) return(EPERM); 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); if(tfp->fp_wd) MAKEROOT(tfp->fp_wd);
} }
CHECK_VREFS; SANITYCHECK;
return(OK); 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); printf("VFSmount: moving opened block spec to new FS_e: %d...\n", fs_e);
bspec->v_bfs_e = fs_e; bspec->v_bfs_e = fs_e;
} }
CHECK_VREFS; SANITYCHECK;
return(OK); return(OK);
} }
@ -382,17 +382,17 @@ PUBLIC int do_umount()
{ {
/* Perform the umount(name) system call. */ /* Perform the umount(name) system call. */
dev_t dev; dev_t dev;
CHECK_VREFS; SANITYCHECK;
/* Only the super-user may do UMOUNT. */ /* Only the super-user may do UMOUNT. */
if (!super_user) return(EPERM); if (!super_user) return(EPERM);
CHECK_VREFS; SANITYCHECK;
/* If 'name' is not for a block special file, return error. */ /* 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); 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); if ( (dev = name_to_dev()) == NO_DEV) return(err_code);
CHECK_VREFS; SANITYCHECK;
return(unmount(dev)); return(unmount(dev));
} }
@ -410,7 +410,7 @@ Dev_t dev;
int count, r; int count, r;
int fs_e; int fs_e;
CHECK_VREFS; SANITYCHECK;
/* Find vmnt */ /* Find vmnt */
for (vmp_i = &vmnt[0]; vmp_i < &vmnt[NR_MNTS]; ++vmp_i) { for (vmp_i = &vmnt[0]; vmp_i < &vmnt[NR_MNTS]; ++vmp_i) {
@ -472,16 +472,16 @@ Dev_t dev;
count += vp->v_ref_count; count += vp->v_ref_count;
} }
} }
CHECK_VREFS; SANITYCHECK;
if (count > 1) { if (count > 1) {
return(EBUSY); /* can't umount a busy file system */ return(EBUSY); /* can't umount a busy file system */
} }
CHECK_VREFS; SANITYCHECK;
vnode_clean_refs(vmp->m_root_node); vnode_clean_refs(vmp->m_root_node);
CHECK_VREFS; SANITYCHECK;
if (vmp->m_mounted_on) { if (vmp->m_mounted_on) {
put_vnode(vmp->m_mounted_on); put_vnode(vmp->m_mounted_on);
@ -504,7 +504,7 @@ Dev_t dev;
vmp->m_dev = NO_DEV; vmp->m_dev = NO_DEV;
vmp->m_fs_e = NONE; vmp->m_fs_e = NONE;
CHECK_VREFS; SANITYCHECK;
/* Is there a block special file that was handled by that partition? */ /* Is there a block special file that was handled by that partition? */
for (vp = &vnode[0]; vp < &vnode[NR_VNODES]; vp++) { for (vp = &vnode[0]; vp < &vnode[NR_VNODES]; vp++) {
@ -531,7 +531,7 @@ Dev_t dev;
} }
} }
CHECK_VREFS; SANITYCHECK;
return(OK); return(OK);
} }

View File

@ -240,9 +240,14 @@ int task; /* who is proc waiting for? (PIPE = pipe) */
* The SUSPEND pseudo error should be returned after calling suspend(). * The SUSPEND pseudo error should be returned after calling suspend().
*/ */
#if DO_SANITYCHECKS
if (task == XPIPE) if (task == XPIPE)
panic(__FILE__, "suspend: called for XPIPE", NO_NUM); 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*/ if (task == XPOPEN) susp_count++;/* #procs susp'ed on pipe*/
fp->fp_suspended = SUSPENDED; fp->fp_suspended = SUSPENDED;
assert(fp->fp_grant == GRANT_INVALID || !GRANT_VALID(fp->fp_grant)); 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.) * but they are needed for pipes, and it is not worth making the distinction.)
* The SUSPEND pseudo error should be returned after calling suspend(). * 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*/ susp_count++; /* #procs susp'ed on pipe*/
fp->fp_suspended = SUSPENDED; fp->fp_suspended = SUSPENDED;
@ -463,6 +472,7 @@ int proc_nr_e;
struct filp *f; struct filp *f;
dev_t dev; dev_t dev;
message mess; message mess;
int wasreviving = 0;
if(isokendpt(proc_nr_e, &proc_nr_p) != OK) { if(isokendpt(proc_nr_e, &proc_nr_p) != OK) {
printf("VFS: ignoring unpause for bogus endpoint %d\n", proc_nr_e); 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; rfp->fp_revived = NOT_REVIVING;
reviving--; reviving--;
wasreviving = 1;
} }
switch (task) { switch (task) {
case XPIPE: /* process trying to read or write a pipe */ case XPIPE: /* process trying to read or write a pipe */
break; break;
@ -540,6 +552,11 @@ int proc_nr_e;
} }
rfp->fp_suspended = NOT_SUSPENDED; rfp->fp_suspended = NOT_SUSPENDED;
if ((task == XPIPE || task == XPOPEN) && !wasreviving) {
susp_count--;
}
reply(proc_nr_e, status); /* signal interrupted call */ reply(proc_nr_e, status); /* signal interrupted call */
} }
@ -587,6 +604,34 @@ PUBLIC int select_match_pipe(struct filp *f)
return 0; 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

View File

@ -138,6 +138,9 @@ _PROTOTYPE( int select_match_pipe, (struct filp *f) );
_PROTOTYPE( void unsuspend_by_endpt, (int) ); _PROTOTYPE( void unsuspend_by_endpt, (int) );
_PROTOTYPE( void select_reply1, (void) ); _PROTOTYPE( void select_reply1, (void) );
_PROTOTYPE( void select_reply2, (void) ); _PROTOTYPE( void select_reply2, (void) );
#if DO_SANITYCHECKS
_PROTOTYPE( int check_pipe, (void) );
#endif
/* protect.c */ /* protect.c */
_PROTOTYPE( int do_access, (void) ); _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 dup_vnode, (struct vnode *vp) );
_PROTOTYPE( void put_vnode, (struct vnode *vp) ); _PROTOTYPE( void put_vnode, (struct vnode *vp) );
_PROTOTYPE( void vnode_clean_refs, (struct vnode *vp) ); _PROTOTYPE( void vnode_clean_refs, (struct vnode *vp) );
#if 0 #if DO_SANITYCHECKS
_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)
_PROTOTYPE( int check_vrefs, (void) ); _PROTOTYPE( int check_vrefs, (void) );
#endif
/* write.c */ /* write.c */
_PROTOTYPE( int do_write, (void) ); _PROTOTYPE( int do_write, (void) );

View File

@ -169,6 +169,7 @@ int line;
#define REFVP(v) { vp = (v); CHECKVN(v); vp->v_ref_check++; } #define REFVP(v) { vp = (v); CHECKVN(v); vp->v_ref_check++; }
#if DO_SANITYCHECKS
/*===========================================================================* /*===========================================================================*
* check_vrefs * * check_vrefs *
*===========================================================================*/ *===========================================================================*/
@ -245,3 +246,4 @@ PUBLIC int check_vrefs()
} }
return !bad; return !bad;
} }
#endif

View File

@ -38,5 +38,3 @@ EXTERN struct vnode {
#define NO_SEEK 0 /* i_seek = NO_SEEK if last op was not SEEK */ #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 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)