Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 29 Aug 2007 17:08:03 GMT
From:      Xin LI <delphij@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 125824 for review
Message-ID:  <200708291708.l7TH83W3092465@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=125824

Change 125824 by delphij@charlie on 2007/08/29 17:07:39

	We can only alter dirty flag while the device is read-write, so
	move it before g_access which would set the device as read-only,
	causing the write to fail.
	
	Restructure the flow in order to make it more obvious.
	
	Don't use vfs_flagopt() to obfuscate simple flag setting.
	
	Set pmp->pm_fmod appropriately to avoid panics.
	
	Submitted by:	bde

Affected files ...

.. //depot/projects/delphij_fork/sys/fs/msdosfs/msdosfs_vfsops.c#8 edit

Differences ...

==== //depot/projects/delphij_fork/sys/fs/msdosfs/msdosfs_vfsops.c#8 (text+ko) ====

@@ -250,8 +250,6 @@
 	 * read/write; if there is no device name, that's all we do.
 	 */
 	if (mp->mnt_flag & MNT_UPDATE) {
-		int ro_to_rw = 0;
-
 		pmp = VFSTOMSDOSFS(mp);
 		if (vfs_flagopt(mp->mnt_optnew, "export", NULL, 0)) {
 			/*
@@ -275,18 +273,38 @@
 			error = vflush(mp, 0, flags, td);
 			if (error)
 				return (error);
+
+			/*
+			 * Now the volume is clean.  We have to mark it so
+			 * when the device is still r/w.
+			 */
+			error = markvoldirty(pmp, 0);
+			if (error) {
+				(void)markvoldirty(pmp, 1);
+				return (error);
+			}
+
+			/* Downgrade the device from rw to ro. */ 
 			DROP_GIANT();
 			g_topology_lock();
 			error = g_access(pmp->pm_cp, 0, -1, 0);
 			g_topology_unlock();
 			PICKUP_GIANT();
-			if (error)
+			if (error) {
+				(void)markvoldirty(pmp, 1);
 				return (error);
+			}
+
+			/*
+			 * Backing out after an error was painful in the
+			 * above.  Now we are committed to succeeding.
+			 */
+			pmp->pm_fmod = 0; 
+			pmp->pm_flags |= MSDOSFSMNT_RONLY;
 
-			/* Now the volume is clean. Mark it. */
-			error = markvoldirty(pmp, 0);
-			if (error && (flags & FORCECLOSE) == 0)
-				return (error);
+			MNT_ILOCK(mp);
+			mp->mnt_flag |= MNT_RDONLY;
+			MNT_IUNLOCK(mp);
 		} else if ((pmp->pm_flags & MSDOSFSMNT_RONLY) &&
 		    !vfs_flagopt(mp->mnt_optnew, "ro", NULL, 0)) {
 			/*
@@ -311,20 +329,22 @@
 			PICKUP_GIANT();
 			if (error)
 				return (error);
+		}
+		/*
+		 * XXX: pm_fmod is only used to implement a panic
+		 * if we forget to set it here.
+		 */
+		pmp->pm_fmod = 1; 
 
-			ro_to_rw = 1;
-		}
-		vfs_flagopt(mp->mnt_optnew, "ro",
-		    &pmp->pm_flags, MSDOSFSMNT_RONLY);
-		vfs_flagopt(mp->mnt_optnew, "ro",
-		    &mp->mnt_flag, MNT_RDONLY);
+		pmp->pm_flags &= ~MSDOSFSMNT_RONLY;
+		MNT_ILOCK(mp);
+		mp->mnt_flag &= ~MNT_RDONLY;
+		MNT_IUNLOCK(mp); 
 
-		if (ro_to_rw) {
-			/* Now that the volume is modifiable, mark it dirty. */
-			error = markvoldirty(pmp, 1);
-			if (error)
-				return (error);
-		}
+		/* Now that the volume is modifiable, mark it dirty. */
+		error = markvoldirty(pmp, 1);
+		if (error)
+			return (error);
 	}
 	/*
 	 * Not an update, or updating the name: look up the name
@@ -714,9 +734,10 @@
 	if (ronly)
 		pmp->pm_flags |= MSDOSFSMNT_RONLY;
 	else {
-		/* Mark the volume dirty while it is mounted read/write. */
-		if ((error = markvoldirty(pmp, 1)) != 0)
+		if ((error = markvoldirty(pmp, 1)) != 0) {
+			(void)markvoldirty(pmp, 0); 
 			goto error_exit;
+		}
 		pmp->pm_fmod = 1;
 	}
 	mp->mnt_data = (qaddr_t) pmp;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200708291708.l7TH83W3092465>