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.
This commit is contained in:
		
							parent
							
								
									4c156d8a32
								
							
						
					
					
						commit
						c540bcb001
					
				| @ -394,10 +394,7 @@ PUBLIC int dev_io( | |||||||
|   dp = &dmap[major_dev]; |   dp = &dmap[major_dev]; | ||||||
| 
 | 
 | ||||||
|   /* See if driver is roughly valid. */ |   /* See if driver is roughly valid. */ | ||||||
|   if (dp->dmap_driver == NONE) { |   if (dp->dmap_driver == NONE) return(ENXIO); | ||||||
| 	printf("VFS: dev_io: no driver for major %d\n", major_dev); |  | ||||||
| 	return(ENXIO); |  | ||||||
|   } |  | ||||||
| 
 | 
 | ||||||
|   if (suspend_reopen) { |   if (suspend_reopen) { | ||||||
| 	/* Suspend user. */ | 	/* Suspend user. */ | ||||||
|  | |||||||
| @ -30,7 +30,6 @@ | |||||||
| #include "scratchpad.h" | #include "scratchpad.h" | ||||||
| #include "dmap.h" | #include "dmap.h" | ||||||
| #include "param.h" | #include "param.h" | ||||||
| #include "select.h" |  | ||||||
| #include <minix/vfsif.h> | #include <minix/vfsif.h> | ||||||
| #include "vnode.h" | #include "vnode.h" | ||||||
| #include "vmnt.h" | #include "vmnt.h" | ||||||
|  | |||||||
| @ -14,7 +14,6 @@ | |||||||
| #include <string.h> | #include <string.h> | ||||||
| #include <assert.h> | #include <assert.h> | ||||||
| 
 | 
 | ||||||
| #include "select.h" |  | ||||||
| #include "file.h" | #include "file.h" | ||||||
| #include "fproc.h" | #include "fproc.h" | ||||||
| #include "dmap.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.,
 | 	/* Test filp for select operations if not already done so. e.g.,
 | ||||||
| 	 * processes sharing a filp and both doing a select on that filp. */ | 	 * processes sharing a filp and both doing a select on that filp. */ | ||||||
| 	f = se->filps[fd]; | 	f = se->filps[fd]; | ||||||
| 	select_lock_filp(f, f->filp_select_ops | ops); |  | ||||||
| 	if ((f->filp_select_ops & ops) != ops) { | 	if ((f->filp_select_ops & ops) != ops) { | ||||||
| 		int wantops; | 		int wantops; | ||||||
| 
 | 
 | ||||||
| 		wantops = (f->filp_select_ops |= ops); | 		wantops = (f->filp_select_ops |= ops); | ||||||
| 		r = do_select_request(se, fd, &wantops); | 		r = do_select_request(se, fd, &wantops); | ||||||
| 		unlock_filp(f); | 		if (r != OK) { | ||||||
| 		if (r != SEL_OK) { | 			if (r == SUSPEND) continue; | ||||||
| 			if (r == SEL_DEFERRED) continue; |  | ||||||
| 			else break; /* Error or bogus return code; abort */ | 			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 | 		 * Either way, we might have a result and we need to store them | ||||||
| 		 * in the select table entry. */ | 		 * in the select table entry. */ | ||||||
| 		if (wantops & ops) ops2tab(wantops, fd, se); | 		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))) { | 		if (!(rops & (SEL_RD|SEL_WR|SEL_ERR))) { | ||||||
| 			/* Nothing left to do */ | 			/* Nothing left to do */ | ||||||
| 			*ops = 0; | 			*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) |   if (f->filp_select_flags & FSF_BUSY) | ||||||
| 	return(SEL_DEFERRED); | 	return(SUSPEND); | ||||||
| 
 | 
 | ||||||
|   major = major(f->filp_vno->v_sdev); |   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]; |   dp = &dmap[major]; | ||||||
|   if (dp->dmap_sel_filp) |   if (dp->dmap_sel_filp) | ||||||
| 	return(SEL_DEFERRED); | 	return(SUSPEND); | ||||||
| 
 | 
 | ||||||
|   f->filp_select_flags &= ~FSF_UPDATE; |   f->filp_select_flags &= ~FSF_UPDATE; | ||||||
|   r = dev_io(VFS_DEV_SELECT, f->filp_vno->v_sdev, rops, NULL, |   r = dev_io(VFS_DEV_SELECT, f->filp_vno->v_sdev, rops, NULL, | ||||||
| 	     cvu64(0), 0, 0, FALSE); | 	     cvu64(0), 0, 0, FALSE); | ||||||
|   if (r < 0 && r != SUSPEND) |   if (r < 0 && r != SUSPEND) | ||||||
| 	return(SEL_ERROR); | 	return(r); | ||||||
| 
 | 
 | ||||||
|   if (r != SUSPEND) |   if (r != SUSPEND) | ||||||
| 	panic("select_request_asynch: expected SUSPEND got: %d", r); | 	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; |   dp->dmap_sel_filp = f; | ||||||
|   f->filp_select_flags |= FSF_BUSY; |   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)) |   int UNUSED(block)) | ||||||
| { | { | ||||||
|   /* Files are always ready, so output *ops is input *ops */ |   /* 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; |   int major, r; | ||||||
| 
 | 
 | ||||||
|   major = major(f->filp_vno->v_sdev); |   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 || |   if (dmap[major].dmap_style == STYLE_DEVA || | ||||||
|       dmap[major].dmap_style == STYLE_CLONE_A) |       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, |   *ops = dev_io(VFS_DEV_SELECT, f->filp_vno->v_sdev, rops, NULL, | ||||||
| 		cvu64(0), 0, 0, FALSE); | 		cvu64(0), 0, 0, FALSE); | ||||||
|   if (*ops < 0) |   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) |   if (!*ops && block) | ||||||
| 	f->filp_pipe_select_ops |= orig_ops; | 	f->filp_pipe_select_ops |= orig_ops; | ||||||
| 
 | 
 | ||||||
|   return(SEL_OK); |   return(OK); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /*===========================================================================*
 | /*===========================================================================*
 | ||||||
| @ -829,7 +824,7 @@ int status; | |||||||
| 		/* there may be operations pending */ | 		/* there may be operations pending */ | ||||||
| 		f->filp_select_ops &= ~status; | 		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 && (f->filp_select_flags & FSF_BLOCKED))) { | ||||||
| 		if (status > 0) {	/* operations ready */ | 		if (status > 0) {	/* operations ready */ | ||||||
| 			if (status & SEL_RD) | 			if (status & SEL_RD) | ||||||
| @ -842,12 +837,10 @@ int status; | |||||||
| 			/* Always unblock upon error */ | 			/* Always unblock upon error */ | ||||||
| 			f->filp_select_flags &= ~FSF_BLOCKED; | 			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(); |   select_restart_filps(); | ||||||
| @ -956,13 +949,11 @@ PRIVATE void select_restart_filps() | |||||||
| 			continue;			  /* 'update' state */ | 			continue;			  /* 'update' state */ | ||||||
| 
 | 
 | ||||||
| 		wantops = ops = f->filp_select_ops; | 		wantops = ops = f->filp_select_ops; | ||||||
| 		select_lock_filp(f, ops); |  | ||||||
| 		vp = f->filp_vno; | 		vp = f->filp_vno; | ||||||
| 		assert((vp->v_mode & I_TYPE) == I_CHAR_SPECIAL); | 		assert((vp->v_mode & I_TYPE) == I_CHAR_SPECIAL); | ||||||
| 		r = do_select_request(se, fd, &wantops); | 		r = do_select_request(se, fd, &wantops); | ||||||
| 		unlock_filp(f); | 		if (r != OK) { | ||||||
| 		if (r != SEL_OK) { | 			if (r == SUSPEND) continue; | ||||||
| 			if (r == SEL_DEFERRED) continue; |  | ||||||
| 			else break; /* Error or bogus return code; abort */ | 			else break; /* Error or bogus return code; abort */ | ||||||
| 		} | 		} | ||||||
| 		if (wantops & ops) ops2tab(wantops, fd, se); | 		if (wantops & ops) ops2tab(wantops, fd, se); | ||||||
| @ -985,8 +976,10 @@ int *ops; | |||||||
| 
 | 
 | ||||||
|   type = se->type[fd]; |   type = se->type[fd]; | ||||||
|   f = se->filps[fd]; |   f = se->filps[fd]; | ||||||
|  |   select_lock_filp(f, *ops); | ||||||
|   r = fdtypes[type].select_request(f, ops, se->block); |   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->error = EINTR; | ||||||
| 	se->block = 0;	/* Stop blocking to return asap */ | 	se->block = 0;	/* Stop blocking to return asap */ | ||||||
| 	if (!is_deferred(se)) select_cancel_all(se); | 	if (!is_deferred(se)) select_cancel_all(se); | ||||||
|  | |||||||
| @ -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 |  | ||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 Thomas Veerman
						Thomas Veerman