From: Thomas Veerman <thomas@minix3.org>
Date: Fri, 19 Aug 2011 14:12:07 +0000 (+0000)
Subject: Fix locking issues with back calls from FSes
X-Git-Tag: v3.2.0~343
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/Bv9ARM.ch09.html?a=commitdiff_plain;h=ae2159c371b2472e2bc26a297bc8cf888b76b5a8;p=minix.git

Fix locking issues with back calls from FSes
---

diff --git a/servers/avfs/link.c b/servers/avfs/link.c
index 90bf4a148..d0e1c762f 100644
--- a/servers/avfs/link.c
+++ b/servers/avfs/link.c
@@ -417,7 +417,7 @@ struct fproc *rfp;
 }
 
 /*===========================================================================*
- *                             do_rdlink                                    *
+ *                             do_rdlink				     *
  *===========================================================================*/
 PUBLIC int do_rdlink()
 {
diff --git a/servers/avfs/main.c b/servers/avfs/main.c
index 2e548b80b..0f70e41dd 100644
--- a/servers/avfs/main.c
+++ b/servers/avfs/main.c
@@ -385,6 +385,10 @@ PRIVATE void *do_work(void *arg)
 
   lock_proc(fp, 0); /* This proc is busy */
 
+  if (verbose) {
+	printf("Doing call_nr = %d for %d\n", call_nr, who_e);
+  }
+
   if (call_nr == MAPDRIVER) {
 	error = do_mapdriver();
   } else if (call_nr == COMMON_GETSYSINFO) {
diff --git a/servers/avfs/misc.c b/servers/avfs/misc.c
index 0f0cba7cd..53b19b533 100644
--- a/servers/avfs/misc.c
+++ b/servers/avfs/misc.c
@@ -284,14 +284,19 @@ PUBLIC int do_fcntl()
 PUBLIC int do_sync()
 {
   struct vmnt *vmp;
+  int r = OK;
+
   for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; ++vmp) {
-	lock_vmnt(vmp, VMNT_EXCL);
-	if (vmp->m_dev != NO_DEV && vmp->m_fs_e != NONE)
+	if (vmp->m_dev != NO_DEV && vmp->m_fs_e != NONE &&
+		 vmp->m_root_node != NULL) {
+		if ((r = lock_vmnt(vmp, VMNT_EXCL)) != OK)
+			break;
 		req_sync(vmp->m_fs_e);
-	unlock_vmnt(vmp);
+		unlock_vmnt(vmp);
+	}
   }
 
-  return(OK);
+  return(r);
 }
 
 /*===========================================================================*
@@ -299,23 +304,28 @@ PUBLIC int do_sync()
  *===========================================================================*/
 PUBLIC int do_fsync()
 {
-/* Perform the fsync() system call. For now, don't be unnecessarily smart. */
+/* Perform the fsync() system call. */
   struct filp *rfilp;
   struct vmnt *vmp;
   dev_t dev;
+  int r = OK;
 
   if ((rfilp = get_filp(m_in.m1_i1, VNODE_READ)) == NULL) return(err_code);
   dev = rfilp->filp_vno->v_dev;
   for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; ++vmp) {
-	lock_vmnt(vmp, VMNT_EXCL);
-	if (vmp->m_dev != NO_DEV && vmp->m_dev == dev && vmp->m_fs_e != NONE)
+	if (vmp->m_dev != NO_DEV && vmp->m_dev == dev &&
+		vmp->m_fs_e != NONE && vmp->m_root_node != NULL) {
+
+		if ((r = lock_vmnt(vmp, VMNT_EXCL)) != OK)
+			break;
 		req_sync(vmp->m_fs_e);
-	unlock_vmnt(vmp);
+		unlock_vmnt(vmp);
+	}
   }
 
   unlock_filp(rfilp);
 
-  return(OK);
+  return(r);
 }
 
 /*===========================================================================*
@@ -344,6 +354,7 @@ PUBLIC void pm_reboot()
 	}
   }
 
+  do_sync();
   unmount_all();
 }
 
diff --git a/servers/avfs/mount.c b/servers/avfs/mount.c
index 00f8c8258..9842fd0e8 100644
--- a/servers/avfs/mount.c
+++ b/servers/avfs/mount.c
@@ -43,7 +43,6 @@ PRIVATE bitchunk_t nonedev[BITMAP_CHUNKS(NR_NONEDEVS)] = { 0 };
 
 FORWARD _PROTOTYPE( dev_t name_to_dev, (int allow_mountpt,
 					char path[PATH_MAX+1])		);
-FORWARD _PROTOTYPE( int is_nonedev, (dev_t dev)				);
 FORWARD _PROTOTYPE( dev_t find_free_nonedev, (void)			);
 FORWARD _PROTOTYPE( void update_bspec, (dev_t dev, endpoint_t fs_e,
 				      int send_drv_e)			);
@@ -153,7 +152,8 @@ PUBLIC int do_mount()
 	return(err_code);
 
   /* Do the actual job */
-  return mount_fs(dev, fullpath, fs_e, rdonly, mount_label);
+  r = mount_fs(dev, fullpath, fs_e, rdonly, mount_label);
+  return(r);
 }
 
 
@@ -191,32 +191,18 @@ char mount_label[LABEL_MAX] )
 	assert(strlen(label) > 0);
   }
 
-  lock_bsf();
-
-  /* Check whether there is a block special file open which uses the
-   * same device (partition) */
-  for (bspec = &vnode[0]; bspec < &vnode[NR_VNODES]; ++bspec) {
-	if (bspec->v_ref_count > 0 && bspec->v_sdev == dev) {
-		/* Found, flush and invalidate any blocks for this device. */
-		req_flush(bspec->v_fs_e, dev);
-		break;
-	}
-  }
-
   /* Scan vmnt table to see if dev already mounted. If not, find a free slot.*/
   found = FALSE;
   for (i = 0; i < NR_MNTS; ++i) {
 	if (vmnt[i].m_dev == dev) found = TRUE;
   }
   if (found) {
-	unlock_bsf();
 	return(EBUSY);
   } else if ((new_vmp = get_free_vmnt()) == NULL) {
-	unlock_bsf();
 	return(ENOMEM);
   }
 
-  lock_vmnt(new_vmp, VMNT_EXCL);
+  if ((r = lock_vmnt(new_vmp, VMNT_EXCL)) != OK) return(r);
 
   isroot = (strcmp(mountpoint, "/") == 0);
   mount_root = (isroot && have_root < 2); /* Root can be mounted twice:
@@ -237,14 +223,15 @@ char mount_label[LABEL_MAX] )
 	} else
 		r = EBUSY;
 
+	if (vp != NULL)
+		unlock_vmnt(parent_vmp);
+
 	if (r != OK) {
 		if (vp != NULL) {
 			unlock_vnode(vp);
-			unlock_vmnt(parent_vmp);
 			put_vnode(vp);
 		}
 		unlock_vmnt(new_vmp);
-		unlock_bsf();
 		return(r);
 	}
   }
@@ -254,11 +241,9 @@ char mount_label[LABEL_MAX] )
   if ((root_node = get_free_vnode()) == NULL || dev == 266) {
 	if (vp != NULL) {
 		unlock_vnode(vp);
-		unlock_vmnt(parent_vmp);
 		put_vnode(vp);
 	}
 	unlock_vmnt(new_vmp);
-	unlock_bsf();
 	return(err_code);
   }
 
@@ -271,19 +256,21 @@ char mount_label[LABEL_MAX] )
   else new_vmp->m_flags &= ~VMNT_READONLY;
 
   /* Tell FS which device to mount */
+  if (verbose)
+	printf("Tell FS %d to mount device %s %d\n", fs_e, label, dev);
   if ((r = req_readsuper(fs_e, label, dev, rdonly, isroot, &res)) != OK) {
+	if (verbose) printf("Failed: %d\n", r);
 	if (vp != NULL) {
 		unlock_vnode(vp);
-		unlock_vmnt(parent_vmp);
 		put_vnode(vp);
 	}
 	new_vmp->m_fs_e = NONE;
 	new_vmp->m_dev = NO_DEV;
 	unlock_vnode(root_node);
 	unlock_vmnt(new_vmp);
-	unlock_bsf();
 	return(r);
   }
+  if (verbose) printf("Ok done: r=%d\n", r);
 
   /* Fill in root node's fields */
   root_node->v_fs_e = res.fs_e;
@@ -300,7 +287,9 @@ char mount_label[LABEL_MAX] )
   root_node->v_vmnt = new_vmp;
   root_node->v_dev = new_vmp->m_dev;
 
-  if(mount_root) {
+  lock_bsf();
+
+  if (mount_root) {
 	/* Superblock and root node already read.
 	 * Nothing else can go wrong. Perform the mount. */
 	new_vmp->m_root_node = root_node;
@@ -342,7 +331,6 @@ char mount_label[LABEL_MAX] )
   /* If error, return the super block and both inodes; release the vmnt. */
   if (r != OK) {
 	unlock_vnode(vp);
-	unlock_vmnt(parent_vmp);
 	unlock_vnode(root_node);
 	unlock_vmnt(new_vmp);
 	put_vnode(vp);
@@ -364,7 +352,6 @@ char mount_label[LABEL_MAX] )
   update_bspec(dev, fs_e, 0 /* Don't send new driver endpoint */);
 
   unlock_vnode(vp);
-  unlock_vmnt(parent_vmp);
   unlock_vnode(root_node);
   unlock_vmnt(new_vmp);
   unlock_bsf();
@@ -454,7 +441,10 @@ PUBLIC int unmount(
 
   lock_bsf();
 
-  assert(lock_vmnt(vmp, VMNT_EXCL) == OK);
+  if ((r = lock_vmnt(vmp, VMNT_EXCL)) != OK) {
+	unlock_bsf();
+	return(r);
+  }
 
   /* See if the mounted device is busy.  Only 1 vnode using it should be
    * open -- the root vnode -- and that inode only 1 time. */
@@ -577,7 +567,7 @@ PRIVATE dev_t name_to_dev(int allow_mountpt, char path[PATH_MAX+1])
 /*===========================================================================*
  *                              is_nonedev				     *
  *===========================================================================*/
-PRIVATE int is_nonedev(dev_t dev)
+PUBLIC int is_nonedev(dev_t dev)
 {
 /* Return whether the given device is a "none" pseudo device.
  */
diff --git a/servers/avfs/open.c b/servers/avfs/open.c
index 54b69a43b..23e8cfb5f 100644
--- a/servers/avfs/open.c
+++ b/servers/avfs/open.c
@@ -180,7 +180,8 @@ PRIVATE int common_open(char path[PATH_MAX+1], int oflags, mode_t omode)
 			 * we default to ROOT_FS. */
 			vp->v_bfs_e = ROOT_FS_E; /* By default */
 			for (vmp = &vmnt[0]; vmp < &vmnt[NR_MNTS]; ++vmp)
-				if (vmp->m_dev == vp->v_sdev)
+				if (vmp->m_dev == vp->v_sdev &&
+				    !is_nonedev(vmp->m_dev))
 					vp->v_bfs_e = vmp->m_fs_e;
 
 			/* Get the driver endpoint of the block spec device */
diff --git a/servers/avfs/path.c b/servers/avfs/path.c
index 1be54d9ef..213ae27cf 100644
--- a/servers/avfs/path.c
+++ b/servers/avfs/path.c
@@ -169,6 +169,10 @@ struct fproc *rfp;
   char *cp;
   char dir_entry[PATH_MAX+1];
   struct vnode *start_dir, *res;
+  int r;
+
+  *resolve->l_vnode = NULL;
+  *resolve->l_vmp = NULL;
 
   /* Is the path absolute or relative? Initialize 'start_dir' accordingly. */
   start_dir = (resolve->l_path[0] == '/' ? rfp->fp_rd : rfp->fp_wd);
@@ -195,8 +199,12 @@ struct fproc *rfp;
 	struct vmnt *vmp;
 
 	vmp = find_vmnt(start_dir->v_fs_e);
-	if (lock_vmnt(vmp, resolve->l_vmnt_lock) != EBUSY)
+	r = lock_vmnt(vmp, resolve->l_vmnt_lock);
+	if (r == EDEADLK)
+		return(NULL);
+	else if (r == OK)
 		*resolve->l_vmp = vmp;
+
 	lock_vnode(start_dir, resolve->l_vnode_lock);
 	*resolve->l_vnode = start_dir;
 	dup_vnode(start_dir);
@@ -285,6 +293,8 @@ struct fproc *rfp;
   if ((r = lock_vmnt(vmpres, resolve->l_vmnt_lock)) != OK) {
 	if (r == EBUSY) /* vmnt already locked */
 		vmpres = NULL;
+	else
+		return(r);
   }
   *(resolve->l_vmp) = vmpres;
 
@@ -320,7 +330,7 @@ struct fproc *rfp;
 		vmp = NULL;
 	} else if (r == EENTERMOUNT) {
 		/* Entering a new partition */
-		dir_vp = 0;
+		dir_vp = NULL;
 		/* Start node is now the mounted partition's root node */
 		for (vmp = &vmnt[0]; vmp != &vmnt[NR_MNTS]; ++vmp) {
 			if (vmp->m_dev != NO_DEV && vmp->m_mounted_on) {
@@ -374,6 +384,8 @@ struct fproc *rfp;
 	if ((r = lock_vmnt(vmpres, resolve->l_vmnt_lock)) != OK) {
 		if (r == EBUSY)
 			vmpres = NULL;	/* Already locked */
+		else
+			return(r);
 	}
 	*(resolve->l_vmp) = vmpres;
 
diff --git a/servers/avfs/pipe.c b/servers/avfs/pipe.c
index de2128069..bfc918153 100644
--- a/servers/avfs/pipe.c
+++ b/servers/avfs/pipe.c
@@ -50,14 +50,14 @@ PUBLIC int do_pipe()
   struct vmnt *vmp;
   struct node_details res;
 
+  /* Get a lock on PFS */
+  if ((vmp = find_vmnt(PFS_PROC_NR)) == NULL) panic("PFS gone");
+  if ((r = lock_vmnt(vmp, VMNT_WRITE)) != OK) return(r);
+
   /* See if a free vnode is available */
   if ((vp = get_free_vnode()) == NULL) return(err_code);
   lock_vnode(vp, VNODE_OPCL);
 
-  /* Get a lock on PFS */
-  if ((vmp = find_vmnt(PFS_PROC_NR)) == NULL) panic("PFS gone");
-  lock_vmnt(vmp, VMNT_WRITE);
-
   /* Acquire two file descriptors. */
   rfp = fp;
   if ((r = get_fd(0, R_BIT, &fil_des[0], &fil_ptr0)) != OK) {
@@ -147,8 +147,13 @@ endpoint_t map_to_fs_e;
 
   if ((vmp = find_vmnt(map_to_fs_e)) == NULL)
 	panic("Can't map to unknown endpoint");
-  if (lock_vmnt(vmp, VMNT_WRITE) == EBUSY)
-	vmp = NULL;	/* Already locked, do not unlock */
+  if ((r = lock_vmnt(vmp, VMNT_WRITE)) != OK) {
+	if (r == EBUSY)
+		vmp = NULL;	/* Already locked, do not unlock */
+	else
+		return(r);
+
+  }
 
   /* Create a temporary mapping of this inode to another FS. Read and write
    * operations on data will be handled by that FS. The rest by the 'original'
diff --git a/servers/avfs/proto.h b/servers/avfs/proto.h
index 31f7ca2b9..0fa5761bd 100644
--- a/servers/avfs/proto.h
+++ b/servers/avfs/proto.h
@@ -141,6 +141,7 @@ _PROTOTYPE( void ds_event, (void)					);
 _PROTOTYPE( int do_fsready, (void)                                      );
 _PROTOTYPE( int do_mount, (void)					);
 _PROTOTYPE( int do_umount, (void)					);
+_PROTOTYPE( int is_nonedev, (dev_t dev)					);
 _PROTOTYPE( void mount_pfs, (void)					);
 _PROTOTYPE( int mount_fs, (dev_t dev, char fullpath[PATH_MAX+1],
 				   endpoint_t fs_e, int rdonly,
diff --git a/servers/avfs/read.c b/servers/avfs/read.c
index ffde1f9db..8de5eeb86 100644
--- a/servers/avfs/read.c
+++ b/servers/avfs/read.c
@@ -137,12 +137,17 @@ int rw_flag;			/* READING or WRITING */
   } else if (block_spec) {		/* Block special files. */
 	lock_bsf();
 
+	printf("Doing block read_write(%d) from dev %d/ep=%d\n",
+		rw_flag == READING,
+		vp->v_sdev, vp->v_bfs_e);
 	r = req_breadwrite(vp->v_bfs_e, who_e, vp->v_sdev, position,
 		m_in.nbytes, m_in.buffer, rw_flag, &res_pos, &res_cum_io);
 	if (r == OK) {
+		printf("OK res_cum_io = %d\n", res_cum_io);
 		position = res_pos;
 		cum_io += res_cum_io;
-	}
+	} else
+		printf("Failed with %d\n", r);
 
 	unlock_bsf();
   } else {				/* Regular files */
diff --git a/servers/avfs/vmnt.c b/servers/avfs/vmnt.c
index cbb517f7f..f391aa963 100644
--- a/servers/avfs/vmnt.c
+++ b/servers/avfs/vmnt.c
@@ -129,6 +129,8 @@ PUBLIC int lock_vmnt(struct vmnt *vmp, tll_access_t locktype)
 
   initial_locktype = (locktype == VMNT_EXCL) ? VMNT_WRITE : locktype;
 
+  if (vmp->m_fs_e == who_e) return(EDEADLK);
+
   r = tll_lock(&vmp->m_lock, initial_locktype);
 
   if (r == EBUSY) return(r);