Revise and improve ntfs_subr.c 1.30: read only a single cluster at a time

in ntfs_writentvattr_plain and ntfs_readntvattr_plain, and purge the boot
block from the buffer cache if isn't exactly one cluster long. These two
changes work around the same buffer cache bug that ntfs_subr.c 1.30 tried
to, but in a different way. This may decrease throughput by reading smaller
amounts of data from the disk at a time, but may increase it by avoiding
bogus writes of clean buffers.
Problem (re)reported by Karel J. Bosschaart on -current.

2 files changed, 28 insertions, 17 deletions

diff --git a/sys/fs/ntfs/ntfs_subr.c b/sys/fs/ntfs/ntfs_subr.c
index 814a91e..cf2879c 100644
--- a/sys/fs/ntfs/ntfs_subr.c
+++ b/sys/fs/ntfs/ntfs_subr.c
@@ -1442,9 +1442,16 @@ ntfs_writentvattr_plain(
 		off = ntfs_btocnoff(off);
 
 		while (left && ccl) {
-			tocopy = min(left,
-				  min(ntfs_cntob(ccl) - off, MAXBSIZE - off));
+			/*
+			 * Always read and write single clusters at a time -
+			 * we need to avoid requesting differently-sized
+			 * blocks at the same disk offsets to avoid
+			 * confusing the buffer cache.
+			 */
+			tocopy = min(left, ntfs_cntob(1) - off);
 			cl = ntfs_btocl(tocopy + off);
+			KASSERT(cl == 1 && tocopy <= ntfs_cntob(1),
+			    ("single cluster limit mistake"));
 			ddprintf(("ntfs_writentvattr_plain: write: " \
 				"cn: 0x%x cl: %d, off: %d len: %d, left: %d\n",
 				(u_int32_t) cn, (u_int32_t) cl, 
@@ -1540,23 +1547,19 @@ ntfs_readntvattr_plain(
 				off = ntfs_btocnoff(off);
 
 				while (left && ccl) {
-					tocopy = min(left,
-						  min(ntfs_cntob(ccl) - off,
-						      MAXBSIZE - off));
-					cl = ntfs_btocl(tocopy + off);
-
 					/*
-					 * If 'off' pushes us to next
-					 * block, don't attempt to read whole
-					 * 'tocopy' at once. This is to avoid
-					 * bread() with varying 'size' for
-					 * same 'blkno', which is not good.
+					 * Always read single clusters at a
+					 * time - we need to avoid reading
+					 * differently-sized blocks at the
+					 * same disk offsets to avoid
+					 * confusing the buffer cache.
 					 */
-					if (cl > ntfs_btocl(tocopy)) {
-						tocopy -=
-						    ntfs_btocnoff(tocopy + off);
-						cl--;
-					}
+					tocopy = min(left,
+					    ntfs_cntob(1) - off);
+					cl = ntfs_btocl(tocopy + off);
+					KASSERT(cl == 1 &&
+					    tocopy <= ntfs_cntob(1),
+					    ("single cluster limit mistake"));
 
 					ddprintf(("ntfs_readntvattr_plain: " \
 						"read: cn: 0x%x cl: %d, " \
diff --git a/sys/fs/ntfs/ntfs_vfsops.c b/sys/fs/ntfs/ntfs_vfsops.c
index 0f3e60e..f01c6ff 100644
--- a/sys/fs/ntfs/ntfs_vfsops.c
+++ b/sys/fs/ntfs/ntfs_vfsops.c
@@ -311,6 +311,14 @@ ntfs_mountfs(devvp, mp, argsp, td)
 		goto out;
 	ntmp = malloc( sizeof *ntmp, M_NTFSMNT, M_WAITOK | M_ZERO);
 	bcopy( bp->b_data, &ntmp->ntm_bootfile, sizeof(struct bootfile) );
+	/*
+	 * We must not cache the boot block if its size is not exactly
+	 * one cluster in order to avoid confusing the buffer cache when
+	 * the boot file is read later by ntfs_readntvattr_plain(), which
+	 * reads a cluster at a time.
+	 */
+	if (ntfs_cntob(1) != BBSIZE)
+		bp->b_flags |= B_NOCACHE;
 	brelse( bp );
 	bp = NULL;